From d7e2ff62203ca5f09cc074bd8b55ca2e09706e03 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 14 Jul 2023 11:47:22 +0200 Subject: [PATCH] Introduce `RouteParameters::max_total_routing_fee_msat` 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 | 9 +-- lightning/src/ln/blinded_payment_tests.rs | 16 ++--- lightning/src/ln/channelmanager.rs | 3 +- lightning/src/ln/outbound_payment.rs | 77 ++++++++++++++++------- lightning/src/ln/payment_tests.rs | 2 +- lightning/src/routing/router.rs | 17 ++++- 6 files changed, 85 insertions(+), 39 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index f5b20d87..08be3d54 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -116,10 +116,7 @@ fn pay_invoice_using_amount( 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( 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( 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) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 826eaa86..1c9563db 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -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); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0cadbd41..c76ca58f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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)); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 023412e1..2f1e3b0d 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -54,10 +54,14 @@ pub(crate) enum PendingOutboundPayment { AwaitingInvoice { timer_ticks_without_response: u8, retry_strategy: Retry, + max_total_routing_fee_msat: Option, }, 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, }, Retryable { retry_strategy: Option, @@ -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, }, /// 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 ) -> 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!( diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 3def4e36..f74fab25 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -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. diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 1758cbbf..dacc137a 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -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, } 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(&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(reader: &mut R) -> Result { _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, }) } } -- 2.30.2