Introduce `RouteParameters::max_total_routing_fee_msat`
authorElias Rohrer <dev@tnull.de>
Fri, 14 Jul 2023 09:47:22 +0000 (11:47 +0200)
committerElias Rohrer <dev@tnull.de>
Tue, 26 Sep 2023 07:44:04 +0000 (09:44 +0200)
Currently, users have no means to upper-bound the total fees accruing
when finding a route. Here, we add a corresponding field to
`RouteParameters` which will be used to limit the candidate set during
path finding in the following commits.

lightning-invoice/src/payment.rs
lightning/src/ln/blinded_payment_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/routing/router.rs

index f5b20d87fa192f25f2ee709749d9cd903d71792d..08be3d54ba502a97d0ae36b80407a290c846c10d 100644 (file)
@@ -116,10 +116,7 @@ fn pay_invoice_using_amount<P: Deref>(
        if let Some(features) = invoice.features() {
                payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
        }
-       let route_params = RouteParameters {
-               payment_params,
-               final_value_msat: amount_msats,
-       };
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msats);
 
        payer.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
 }
@@ -148,7 +145,7 @@ pub fn preflight_probe_invoice<C: AChannelManager>(
        if let Some(features) = invoice.features() {
                payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
        }
-       let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
        channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
                .map_err(ProbingError::Sending)
@@ -178,7 +175,7 @@ pub fn preflight_probe_zero_value_invoice<C: AChannelManager>(
        if let Some(features) = invoice.features() {
                payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
        }
-       let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
        channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
                .map_err(ProbingError::Sending)
index 826eaa86f48f1dd073a57d11dbb0831129b99e27..1c9563db0fa1311d4e159fa842b6a4463dc680e6 100644 (file)
@@ -47,10 +47,10 @@ fn do_one_hop_blinded_path(success: bool) {
                nodes[1].node.get_our_node_id(), payee_tlvs, &chanmon_cfgs[1].keys_manager, &secp_ctx
        ).unwrap();
 
-       let route_params = RouteParameters {
-               payment_params: PaymentParameters::blinded(vec![blinded_path]),
-               final_value_msat: amt_msat
-       };
+       let route_params = RouteParameters::from_payment_params_and_value(
+               PaymentParameters::blinded(vec![blinded_path]),
+               amt_msat,
+       );
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(),
        PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
        check_added_monitors(&nodes[0], 1);
@@ -90,11 +90,11 @@ fn mpp_to_one_hop_blinded_path() {
 
        let bolt12_features: Bolt12InvoiceFeatures =
                channelmanager::provided_invoice_features(&UserConfig::default()).to_context();
-       let route_params = RouteParameters {
-               payment_params: PaymentParameters::blinded(vec![blinded_path])
+       let route_params = RouteParameters::from_payment_params_and_value(
+               PaymentParameters::blinded(vec![blinded_path])
                        .with_bolt12_features(bolt12_features).unwrap(),
-               final_value_msat: amt_msat,
-       };
+               amt_msat,
+       );
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
        check_added_monitors(&nodes[0], 2);
 
index 0cadbd41a29d21ccb99de1e0d7bf84319e33971c..c76ca58f59fb9cecd319dcad9d49ddf67649e139 100644 (file)
@@ -3570,7 +3570,7 @@ where
                let payment_params =
                        PaymentParameters::from_node_id(node_id, final_cltv_expiry_delta);
 
-               let route_params = RouteParameters { payment_params, final_value_msat: amount_msat };
+               let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
                self.send_preflight_probes(route_params, liquidity_limit_multiplier)
        }
@@ -9559,6 +9559,7 @@ where
                                                                                pending_fee_msat: Some(path_fee),
                                                                                total_msat: path_amt,
                                                                                starting_block_height: best_block_height,
+                                                                               remaining_max_total_routing_fee_msat: None, // only used for retries, and we'll never retry on startup
                                                                        });
                                                                        log_info!(args.logger, "Added a pending payment for {} msat with payment hash {} for path with session priv {}",
                                                                                path_amt, &htlc.payment_hash,  log_bytes!(session_priv_bytes));
index 023412e1afb56cdcdd0a2d9195064e5744ccab6c..2f1e3b0dd0e3d4358f7b43b5e01d1706f3ec005f 100644 (file)
@@ -54,10 +54,14 @@ pub(crate) enum PendingOutboundPayment {
        AwaitingInvoice {
                timer_ticks_without_response: u8,
                retry_strategy: Retry,
+               max_total_routing_fee_msat: Option<u64>,
        },
        InvoiceReceived {
                payment_hash: PaymentHash,
                retry_strategy: Retry,
+               // Note this field is currently just replicated from AwaitingInvoice but not actually
+               // used anywhere.
+               max_total_routing_fee_msat: Option<u64>,
        },
        Retryable {
                retry_strategy: Option<Retry>,
@@ -76,6 +80,7 @@ pub(crate) enum PendingOutboundPayment {
                total_msat: u64,
                /// Our best known block height at the time this payment was initiated.
                starting_block_height: u32,
+               remaining_max_total_routing_fee_msat: Option<u64>,
        },
        /// When a pending payment is fulfilled, we continue tracking it until all pending HTLCs have
        /// been resolved. This ensures we don't look up pending payments in ChannelMonitors on restart
@@ -731,12 +736,15 @@ impl OutboundPayments {
                SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
        {
                let payment_hash = invoice.payment_hash();
+               let mut max_total_routing_fee_msat = None;
                match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
                        hash_map::Entry::Occupied(entry) => match entry.get() {
-                               PendingOutboundPayment::AwaitingInvoice { retry_strategy, .. } => {
+                               PendingOutboundPayment::AwaitingInvoice { retry_strategy, max_total_routing_fee_msat: max_total_fee, .. } => {
+                                       max_total_routing_fee_msat = *max_total_fee;
                                        *entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
                                                payment_hash,
                                                retry_strategy: *retry_strategy,
+                                               max_total_routing_fee_msat,
                                        };
                                },
                                _ => return Err(Bolt12PaymentError::DuplicateInvoice),
@@ -747,6 +755,7 @@ impl OutboundPayments {
                let route_params = RouteParameters {
                        payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
                        final_value_msat: invoice.amount_msats(),
+                       max_total_routing_fee_msat,
                };
 
                self.find_route_and_send_payment(
@@ -779,11 +788,12 @@ impl OutboundPayments {
                        let mut retry_id_route_params = None;
                        for (pmt_id, pmt) in outbounds.iter_mut() {
                                if pmt.is_auto_retryable_now() {
-                                       if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), payment_hash, .. } = pmt {
+                                       if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), payment_hash, remaining_max_total_routing_fee_msat, .. } = pmt {
                                                if pending_amt_msat < total_msat {
                                                        retry_id_route_params = Some((*payment_hash, *pmt_id, RouteParameters {
                                                                final_value_msat: *total_msat - *pending_amt_msat,
                                                                payment_params: params.clone(),
+                                                               max_total_routing_fee_msat: *remaining_max_total_routing_fee_msat,
                                                        }));
                                                        break
                                                }
@@ -987,7 +997,7 @@ impl OutboundPayments {
                                                        log_error!(logger, "Payment not yet sent");
                                                        return
                                                },
-                                               PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy } => {
+                                               PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy, .. } => {
                                                        let total_amount = route_params.final_value_msat;
                                                        let recipient_onion = RecipientOnionFields {
                                                                payment_secret: None,
@@ -1207,6 +1217,8 @@ impl OutboundPayments {
                        custom_tlvs: recipient_onion.custom_tlvs,
                        starting_block_height: best_block_height,
                        total_msat: route.get_total_amount(),
+                       remaining_max_total_routing_fee_msat:
+                               route.route_params.as_ref().and_then(|p| p.max_total_routing_fee_msat),
                };
 
                for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
@@ -1218,7 +1230,7 @@ impl OutboundPayments {
 
        #[allow(unused)]
        pub(super) fn add_new_awaiting_invoice(
-               &self, payment_id: PaymentId, retry_strategy: Retry
+               &self, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option<u64>
        ) -> Result<(), ()> {
                let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
                match pending_outbounds.entry(payment_id) {
@@ -1227,6 +1239,7 @@ impl OutboundPayments {
                                entry.insert(PendingOutboundPayment::AwaitingInvoice {
                                        timer_ticks_without_response: 0,
                                        retry_strategy,
+                                       max_total_routing_fee_msat,
                                });
 
                                Ok(())
@@ -1328,8 +1341,9 @@ impl OutboundPayments {
                                failed_paths_retry: if pending_amt_unsent != 0 {
                                        if let Some(payment_params) = route.route_params.as_ref().map(|p| p.payment_params.clone()) {
                                                Some(RouteParameters {
-                                                       payment_params: payment_params,
+                                                       payment_params,
                                                        final_value_msat: pending_amt_unsent,
+                                                       max_total_routing_fee_msat: None,
                                                })
                                        } else { None }
                                } else { None },
@@ -1689,6 +1703,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
                (8, pending_amt_msat, required),
                (9, custom_tlvs, optional_vec),
                (10, starting_block_height, required),
+               (11, remaining_max_total_routing_fee_msat, option),
                (not_written, retry_strategy, (static_value, None)),
                (not_written, attempts, (static_value, PaymentAttempts::new())),
        },
@@ -1700,10 +1715,12 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
        (5, AwaitingInvoice) => {
                (0, timer_ticks_without_response, required),
                (2, retry_strategy, required),
+               (4, max_total_routing_fee_msat, option),
        },
        (7, InvoiceReceived) => {
                (0, payment_hash, required),
                (2, retry_strategy, required),
+               (4, max_total_routing_fee_msat, option),
        },
 );
 
@@ -1926,7 +1943,9 @@ mod tests {
                let payment_id = PaymentId([0; 32]);
 
                assert!(!outbound_payments.has_pending_payments());
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS {
@@ -1944,10 +1963,15 @@ mod tests {
                );
                assert!(pending_events.lock().unwrap().is_empty());
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_err());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None)
+                               .is_err()
+               );
        }
 
        #[test]
@@ -1957,7 +1981,9 @@ mod tests {
                let payment_id = PaymentId([0; 32]);
 
                assert!(!outbound_payments.has_pending_payments());
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                outbound_payments.abandon_payment(
@@ -1985,7 +2011,9 @@ mod tests {
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                let created_at = now() - DEFAULT_RELATIVE_EXPIRY;
@@ -2031,7 +2059,9 @@ mod tests {
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
@@ -2045,10 +2075,10 @@ mod tests {
                        .sign(recipient_sign).unwrap();
 
                router.expect_find_route(
-                       RouteParameters {
-                               payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
-                               final_value_msat: invoice.amount_msats(),
-                       },
+                       RouteParameters::from_payment_params_and_value(
+                               PaymentParameters::from_bolt12_invoice(&invoice),
+                               invoice.amount_msats(),
+                       ),
                        Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }),
                );
 
@@ -2084,7 +2114,9 @@ mod tests {
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
@@ -2097,10 +2129,10 @@ mod tests {
                        .build().unwrap()
                        .sign(recipient_sign).unwrap();
 
-               let route_params = RouteParameters {
-                       payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
-                       final_value_msat: invoice.amount_msats(),
-               };
+               let route_params = RouteParameters::from_payment_params_and_value(
+                       PaymentParameters::from_bolt12_invoice(&invoice),
+                       invoice.amount_msats(),
+               );
                router.expect_find_route(
                        route_params.clone(), Ok(Route { paths: vec![], route_params: Some(route_params) })
                );
@@ -2150,6 +2182,7 @@ mod tests {
                let route_params = RouteParameters {
                        payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
                        final_value_msat: invoice.amount_msats(),
+                       max_total_routing_fee_msat: Some(1234),
                };
                router.expect_find_route(
                        route_params.clone(),
@@ -2185,7 +2218,9 @@ mod tests {
                assert!(!outbound_payments.has_pending_payments());
                assert!(pending_events.lock().unwrap().is_empty());
 
-               assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), Some(1234)).is_ok()
+               );
                assert!(outbound_payments.has_pending_payments());
 
                assert_eq!(
index 3def4e3629bf2fb98d227d9a95e5b1faf6493bc9..f74fab255e6e7916a2fb334c1a518c51bf23c8b1 100644 (file)
@@ -1337,7 +1337,7 @@ fn preflight_probes_yield_event_and_skip() {
        let mut payment_params = PaymentParameters::from_node_id(nodes[4].node.get_our_node_id(), TEST_FINAL_CLTV)
                .with_bolt11_features(invoice_features).unwrap();
 
-       let route_params = RouteParameters { payment_params, final_value_msat: 80_000_000 };
+       let route_params = RouteParameters::from_payment_params_and_value(payment_params, 80_000_000);
        let res = nodes[0].node.send_preflight_probes(route_params, None).unwrap();
 
        // We check that only one probe was sent, the other one was skipped due to limited liquidity.
index 1758cbbf9c3eaae24151050f488c2524f43109da..dacc137ae8f536b6282f52576c7427b53908d825 100644 (file)
@@ -409,6 +409,7 @@ impl Writeable for Route {
                        (1, self.route_params.as_ref().map(|p| &p.payment_params), option),
                        (2, blinded_tails, optional_vec),
                        (3, self.route_params.as_ref().map(|p| p.final_value_msat), option),
+                       (5, self.route_params.as_ref().map(|p| p.max_total_routing_fee_msat), option),
                });
                Ok(())
        }
@@ -436,6 +437,7 @@ impl Readable for Route {
                        (1, payment_params, (option: ReadableArgs, min_final_cltv_expiry_delta)),
                        (2, blinded_tails, optional_vec),
                        (3, final_value_msat, option),
+                       (5, max_total_routing_fee_msat, option)
                });
                let blinded_tails = blinded_tails.unwrap_or(Vec::new());
                if blinded_tails.len() != 0 {
@@ -448,7 +450,7 @@ impl Readable for Route {
                // If we previously wrote the corresponding fields, reconstruct RouteParameters.
                let route_params = match (payment_params, final_value_msat) {
                        (Some(payment_params), Some(final_value_msat)) => {
-                               Some(RouteParameters { payment_params, final_value_msat })
+                               Some(RouteParameters { payment_params, final_value_msat, max_total_routing_fee_msat })
                        }
                        _ => None,
                };
@@ -467,12 +469,20 @@ pub struct RouteParameters {
 
        /// The amount in msats sent on the failed payment path.
        pub final_value_msat: u64,
+
+       /// The maximum total fees, in millisatoshi, that may accrue during route finding.
+       ///
+       /// This limit also applies to the total fees that may arise while retrying failed payment
+       /// paths.
+       ///
+       /// Default value: `None`
+       pub max_total_routing_fee_msat: Option<u64>,
 }
 
 impl RouteParameters {
        /// Constructs [`RouteParameters`] from the given [`PaymentParameters`] and a payment amount.
        pub fn from_payment_params_and_value(payment_params: PaymentParameters, final_value_msat: u64) -> Self {
-               Self { payment_params, final_value_msat }
+               Self { payment_params, final_value_msat, max_total_routing_fee_msat: None }
        }
 }
 
@@ -480,6 +490,7 @@ impl Writeable for RouteParameters {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
                write_tlv_fields!(writer, {
                        (0, self.payment_params, required),
+                       (1, self.max_total_routing_fee_msat, option),
                        (2, self.final_value_msat, required),
                        // LDK versions prior to 0.0.114 had the `final_cltv_expiry_delta` parameter in
                        // `RouteParameters` directly. For compatibility, we write it here.
@@ -493,6 +504,7 @@ impl Readable for RouteParameters {
        fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
                _init_and_read_len_prefixed_tlv_fields!(reader, {
                        (0, payment_params, (required: ReadableArgs, 0)),
+                       (1, max_total_routing_fee_msat, option),
                        (2, final_value_msat, required),
                        (4, final_cltv_delta, option),
                });
@@ -505,6 +517,7 @@ impl Readable for RouteParameters {
                Ok(Self {
                        payment_params,
                        final_value_msat: final_value_msat.0.unwrap(),
+                       max_total_routing_fee_msat,
                })
        }
 }