diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index fb2740c1de9..145b7c8350d 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -512,7 +512,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { }, 4 => { let final_value_msat = slice_to_be24(get_slice!(3)) as u64; - let payment_params = PaymentParameters::from_node_id(get_pubkey!()); + let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42); let params = RouteParameters { payment_params, final_value_msat, @@ -536,7 +536,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { }, 15 => { let final_value_msat = slice_to_be24(get_slice!(3)) as u64; - let payment_params = PaymentParameters::from_node_id(get_pubkey!()); + let payment_params = PaymentParameters::from_node_id(get_pubkey!(), 42); let params = RouteParameters { payment_params, final_value_msat, diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 3e51d0c6a6d..86538bbb662 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -263,10 +263,13 @@ pub fn do_test(data: &[u8], out: Out) { let scorer = FixedPenaltyScorer::with_penalty(0); let random_seed_bytes: [u8; 32] = [get_slice!(1)[0]; 32]; for target in node_pks.iter() { + let final_value_msat = slice_to_be64(get_slice!(8)); + let final_cltv_expiry_delta = slice_to_be32(get_slice!(4)); let route_params = RouteParameters { - payment_params: PaymentParameters::from_node_id(*target).with_route_hints(last_hops.clone()), - final_value_msat: slice_to_be64(get_slice!(8)), - final_cltv_expiry_delta: slice_to_be32(get_slice!(4)), + payment_params: PaymentParameters::from_node_id(*target, final_cltv_expiry_delta) + .with_route_hints(last_hops.clone()), + final_value_msat, + final_cltv_expiry_delta, }; let _ = find_route(&our_pubkey, &route_params, &net_graph, first_hops.map(|c| c.iter().collect::>()).as_ref().map(|a| a.as_slice()), diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index a3517264b4a..89ff242a268 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -422,7 +422,8 @@ where }; let payment_secret = Some(invoice.payment_secret().clone()); - let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key()) + let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(), + invoice.min_final_cltv_expiry_delta() as u32) .with_expiry_time(expiry_time_from_unix_epoch(&invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { @@ -486,7 +487,7 @@ where }; let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(pubkey), + payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta), final_value_msat: amount_msats, final_cltv_expiry_delta, }; @@ -1377,7 +1378,7 @@ mod tests { assert_eq!(*payer.attempts.borrow(), 1); let retry = RouteParameters { - payment_params: PaymentParameters::for_keysend(pubkey), + payment_params: PaymentParameters::for_keysend(pubkey, final_cltv_expiry_delta), final_value_msat, final_cltv_expiry_delta, }; @@ -1655,7 +1656,8 @@ mod tests { } fn retry_for_invoice(invoice: &Invoice) -> RouteParameters { - let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key()) + let mut payment_params = PaymentParameters::from_node_id( + invoice.recover_payee_pub_key(), invoice.min_final_cltv_expiry_delta() as u32) .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs()) .with_route_hints(invoice.route_hints()); if let Some(features) = invoice.features() { @@ -2071,7 +2073,7 @@ mod tests { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)), }; let router = ManualRouter(RefCell::new(VecDeque::new())); router.expect_find_route(Ok(route.clone())); @@ -2114,7 +2116,7 @@ mod tests { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 100)), }; let router = ManualRouter(RefCell::new(VecDeque::new())); router.expect_find_route(Ok(route.clone())); @@ -2194,7 +2196,7 @@ mod tests { cltv_expiry_delta: 100, }] ], - payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())), + payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 100)), }; let router = ManualRouter(RefCell::new(VecDeque::new())); router.expect_find_route(Ok(route.clone())); diff --git a/lightning-invoice/src/utils.rs b/lightning-invoice/src/utils.rs index 0d6a1f11667..cee1a6b7a88 100644 --- a/lightning-invoice/src/utils.rs +++ b/lightning-invoice/src/utils.rs @@ -724,7 +724,8 @@ mod test { assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat); assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat); - let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key()) + let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(), + invoice.min_final_cltv_expiry_delta() as u32) .with_features(invoice.features().unwrap().clone()) .with_route_hints(invoice.route_hints()); let route_params = RouteParameters { @@ -1087,7 +1088,8 @@ mod test { assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into())); assert!(!invoice.features().unwrap().supports_basic_mpp()); - let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key()) + let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(), + invoice.min_final_cltv_expiry_delta() as u32) .with_features(invoice.features().unwrap().clone()) .with_route_hints(invoice.route_hints()); let params = RouteParameters { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b2a23c7c1d4..e9b7fb94930 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -245,6 +245,10 @@ pub(crate) enum HTLCSource { first_hop_htlc_msat: u64, payment_id: PaymentId, payment_secret: Option, + /// Note that this is now "deprecated" - we write it for forwards (and read it for + /// backwards) compatibility reasons, but prefer to use the data in the + /// [`super::outbound_payment`] module, which stores per-payment data once instead of in + /// each HTLC. payment_params: Option, }, } @@ -2473,7 +2477,7 @@ where self.pending_outbound_payments .send_payment(payment_hash, payment_secret, payment_id, retry_strategy, route_params, &self.router, self.list_usable_channels(), self.compute_inflight_htlcs(), - &self.entropy_source, &self.node_signer, best_block_height, + &self.entropy_source, &self.node_signer, best_block_height, &self.logger, |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } @@ -2489,7 +2493,7 @@ where #[cfg(test)] pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, route: &Route) -> Result, PaymentSendFailure> { let best_block_height = self.best_block.read().unwrap().height(); - self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, Retry::Attempts(0), &self.entropy_source, best_block_height) + self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, None, &self.entropy_source, best_block_height) } @@ -7357,9 +7361,9 @@ where hash_map::Entry::Vacant(entry) => { let path_fee = path.get_path_fees(); entry.insert(PendingOutboundPayment::Retryable { - retry_strategy: Retry::Attempts(0), + retry_strategy: None, attempts: PaymentAttempts::new(), - route_params: None, + payment_params: None, session_privs: [session_priv_bytes].iter().map(|a| *a).collect(), payment_hash: htlc.payment_hash, payment_secret, @@ -7871,7 +7875,7 @@ mod tests { // Next, attempt a keysend payment and make sure it fails. let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id()), + payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV), final_value_msat: 100_000, final_cltv_expiry_delta: TEST_FINAL_CLTV, }; @@ -7964,7 +7968,7 @@ mod tests { let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]); let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(payee_pubkey), + payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10_000, final_cltv_expiry_delta: 40, }; @@ -8009,7 +8013,7 @@ mod tests { let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]); let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(payee_pubkey), + payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10_000, final_cltv_expiry_delta: 40, }; @@ -8574,7 +8578,7 @@ pub mod bench { macro_rules! send_payment { ($node_a: expr, $node_b: expr) => { let usable_channels = $node_a.list_usable_channels(); - let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV) .with_features($node_b.invoice_features()); let scorer = test_utils::TestScorer::with_penalty(0); let seed = [3u8; 32]; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 868b38e3fda..c2ce158f897 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1344,157 +1344,6 @@ impl SendEvent { } } -#[macro_export] -/// Performs the "commitment signed dance" - the series of message exchanges which occur after a -/// commitment update. -macro_rules! commitment_signed_dance { - ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */) => { - { - check_added_monitors!($node_a, 0); - assert!($node_a.node.get_and_clear_pending_msg_events().is_empty()); - $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed); - check_added_monitors!($node_a, 1); - commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, false); - } - }; - ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */, true /* return last RAA */) => { - { - let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!($node_a, $node_b.node.get_our_node_id()); - check_added_monitors!($node_b, 0); - assert!($node_b.node.get_and_clear_pending_msg_events().is_empty()); - $node_b.node.handle_revoke_and_ack(&$node_a.node.get_our_node_id(), &as_revoke_and_ack); - assert!($node_b.node.get_and_clear_pending_msg_events().is_empty()); - check_added_monitors!($node_b, 1); - $node_b.node.handle_commitment_signed(&$node_a.node.get_our_node_id(), &as_commitment_signed); - let (bs_revoke_and_ack, extra_msg_option) = { - let events = $node_b.node.get_and_clear_pending_msg_events(); - assert!(events.len() <= 2); - let (node_a_event, events) = remove_first_msg_event_to_node(&$node_a.node.get_our_node_id(), &events); - (match node_a_event { - MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { - assert_eq!(*node_id, $node_a.node.get_our_node_id()); - (*msg).clone() - }, - _ => panic!("Unexpected event"), - }, events.get(0).map(|e| e.clone())) - }; - check_added_monitors!($node_b, 1); - if $fail_backwards { - assert!($node_a.node.get_and_clear_pending_events().is_empty()); - assert!($node_a.node.get_and_clear_pending_msg_events().is_empty()); - } - (extra_msg_option, bs_revoke_and_ack) - } - }; - ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => { - { - check_added_monitors!($node_a, 0); - assert!($node_a.node.get_and_clear_pending_msg_events().is_empty()); - $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed); - check_added_monitors!($node_a, 1); - let (extra_msg_option, bs_revoke_and_ack) = commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true, true); - assert!(extra_msg_option.is_none()); - bs_revoke_and_ack - } - }; - ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => { - { - let (extra_msg_option, bs_revoke_and_ack) = commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true, true); - $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack); - check_added_monitors!($node_a, 1); - extra_msg_option - } - }; - ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => { - { - assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none()); - } - }; - ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => { - { - commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true); - if $fail_backwards { - expect_pending_htlcs_forwardable_and_htlc_handling_failed!($node_a, vec![$crate::util::events::HTLCDestination::NextHopChannel{ node_id: Some($node_b.node.get_our_node_id()), channel_id: $commitment_signed.channel_id }]); - check_added_monitors!($node_a, 1); - - let node_a_per_peer_state = $node_a.node.per_peer_state.read().unwrap(); - let mut number_of_msg_events = 0; - for (cp_id, peer_state_mutex) in node_a_per_peer_state.iter() { - let peer_state = peer_state_mutex.lock().unwrap(); - let cp_pending_msg_events = &peer_state.pending_msg_events; - number_of_msg_events += cp_pending_msg_events.len(); - if cp_pending_msg_events.len() == 1 { - if let MessageSendEvent::UpdateHTLCs { .. } = cp_pending_msg_events[0] { - assert_ne!(*cp_id, $node_b.node.get_our_node_id()); - } else { panic!("Unexpected event"); } - } - } - // Expecting the failure backwards event to the previous hop (not `node_b`) - assert_eq!(number_of_msg_events, 1); - } else { - assert!($node_a.node.get_and_clear_pending_msg_events().is_empty()); - } - } - } -} - -/// Get a payment preimage and hash. -#[macro_export] -macro_rules! get_payment_preimage_hash { - ($dest_node: expr) => { - { - get_payment_preimage_hash!($dest_node, None) - } - }; - ($dest_node: expr, $min_value_msat: expr) => { - { - crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None) - } - }; - ($dest_node: expr, $min_value_msat: expr, $min_final_cltv_expiry_delta: expr) => { - { - use bitcoin::hashes::Hash as _; - let mut payment_count = $dest_node.network_payment_count.borrow_mut(); - let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]); - *payment_count += 1; - let payment_hash = $crate::ln::PaymentHash( - bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner()); - let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, $min_final_cltv_expiry_delta).unwrap(); - (payment_preimage, payment_hash, payment_secret) - } - }; -} - -#[macro_export] -macro_rules! get_route { - ($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{ - use $crate::chain::keysinterface::EntropySource; - let scorer = $crate::util::test_utils::TestScorer::with_penalty(0); - let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet); - let random_seed_bytes = keys_manager.get_secure_random_bytes(); - $crate::routing::router::get_route( - &$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(), - Some(&$send_node.node.list_usable_channels().iter().collect::>()), - $recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes - ) - }} -} - -#[cfg(test)] -#[macro_export] -macro_rules! get_route_and_payment_hash { - ($send_node: expr, $recv_node: expr, $recv_value: expr) => {{ - let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id()) - .with_features($recv_node.node.invoice_features()); - $crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV) - }}; - ($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{ - let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value)); - let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv); - (route.unwrap(), payment_hash, payment_preimage, payment_secret) - }} -} - #[macro_export] macro_rules! expect_pending_htlcs_forwardable_conditions { ($node: expr, $expected_failures: expr) => {{ @@ -1585,6 +1434,164 @@ macro_rules! expect_pending_htlcs_forwardable_from_events { } }} } + +#[macro_export] +/// Performs the "commitment signed dance" - the series of message exchanges which occur after a +/// commitment update. +macro_rules! commitment_signed_dance { + ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */) => { + $crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, true); + }; + ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */, true /* return last RAA */) => { + $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards) + }; + ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => { + { + check_added_monitors!($node_a, 0); + assert!($node_a.node.get_and_clear_pending_msg_events().is_empty()); + $node_a.node.handle_commitment_signed(&$node_b.node.get_our_node_id(), &$commitment_signed); + check_added_monitors!($node_a, 1); + let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards); + assert!(extra_msg_option.is_none()); + bs_revoke_and_ack + } + }; + ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => { + { + let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards); + $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack); + check_added_monitors!($node_a, 1); + extra_msg_option + } + }; + ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => { + assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none()); + }; + ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => { + $crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, false); + } +} + + +pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> (Option, msgs::RevokeAndACK) { + let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(node_a, node_b.node.get_our_node_id()); + check_added_monitors!(node_b, 0); + assert!(node_b.node.get_and_clear_pending_msg_events().is_empty()); + node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack); + assert!(node_b.node.get_and_clear_pending_msg_events().is_empty()); + check_added_monitors!(node_b, 1); + node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &as_commitment_signed); + let (bs_revoke_and_ack, extra_msg_option) = { + let events = node_b.node.get_and_clear_pending_msg_events(); + assert!(events.len() <= 2); + let (node_a_event, events) = remove_first_msg_event_to_node(&node_a.node.get_our_node_id(), &events); + (match node_a_event { + MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { + assert_eq!(*node_id, node_a.node.get_our_node_id()); + (*msg).clone() + }, + _ => panic!("Unexpected event"), + }, events.get(0).map(|e| e.clone())) + }; + check_added_monitors!(node_b, 1); + if fail_backwards { + assert!(node_a.node.get_and_clear_pending_events().is_empty()); + assert!(node_a.node.get_and_clear_pending_msg_events().is_empty()); + } + (extra_msg_option, bs_revoke_and_ack) +} + +pub fn do_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, commitment_signed: &msgs::CommitmentSigned, fail_backwards: bool, skip_last_step: bool) { + check_added_monitors!(node_a, 0); + assert!(node_a.node.get_and_clear_pending_msg_events().is_empty()); + node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), commitment_signed); + check_added_monitors!(node_a, 1); + + commitment_signed_dance!(node_a, node_b, (), fail_backwards, true, false); + + if skip_last_step { return; } + + if fail_backwards { + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(node_a, + vec![crate::util::events::HTLCDestination::NextHopChannel{ node_id: Some(node_b.node.get_our_node_id()), channel_id: commitment_signed.channel_id }]); + check_added_monitors!(node_a, 1); + + let node_a_per_peer_state = node_a.node.per_peer_state.read().unwrap(); + let mut number_of_msg_events = 0; + for (cp_id, peer_state_mutex) in node_a_per_peer_state.iter() { + let peer_state = peer_state_mutex.lock().unwrap(); + let cp_pending_msg_events = &peer_state.pending_msg_events; + number_of_msg_events += cp_pending_msg_events.len(); + if cp_pending_msg_events.len() == 1 { + if let MessageSendEvent::UpdateHTLCs { .. } = cp_pending_msg_events[0] { + assert_ne!(*cp_id, node_b.node.get_our_node_id()); + } else { panic!("Unexpected event"); } + } + } + // Expecting the failure backwards event to the previous hop (not `node_b`) + assert_eq!(number_of_msg_events, 1); + } else { + assert!(node_a.node.get_and_clear_pending_msg_events().is_empty()); + } +} + +/// Get a payment preimage and hash. +#[macro_export] +macro_rules! get_payment_preimage_hash { + ($dest_node: expr) => { + { + get_payment_preimage_hash!($dest_node, None) + } + }; + ($dest_node: expr, $min_value_msat: expr) => { + { + crate::get_payment_preimage_hash!($dest_node, $min_value_msat, None) + } + }; + ($dest_node: expr, $min_value_msat: expr, $min_final_cltv_expiry_delta: expr) => { + { + use bitcoin::hashes::Hash as _; + let mut payment_count = $dest_node.network_payment_count.borrow_mut(); + let payment_preimage = $crate::ln::PaymentPreimage([*payment_count; 32]); + *payment_count += 1; + let payment_hash = $crate::ln::PaymentHash( + bitcoin::hashes::sha256::Hash::hash(&payment_preimage.0[..]).into_inner()); + let payment_secret = $dest_node.node.create_inbound_payment_for_hash(payment_hash, $min_value_msat, 7200, $min_final_cltv_expiry_delta).unwrap(); + (payment_preimage, payment_hash, payment_secret) + } + }; +} + +#[macro_export] +macro_rules! get_route { + ($send_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{ + use $crate::chain::keysinterface::EntropySource; + let scorer = $crate::util::test_utils::TestScorer::with_penalty(0); + let keys_manager = $crate::util::test_utils::TestKeysInterface::new(&[0u8; 32], bitcoin::network::constants::Network::Testnet); + let random_seed_bytes = keys_manager.get_secure_random_bytes(); + $crate::routing::router::get_route( + &$send_node.node.get_our_node_id(), &$payment_params, &$send_node.network_graph.read_only(), + Some(&$send_node.node.list_usable_channels().iter().collect::>()), + $recv_value, $cltv, $send_node.logger, &scorer, &random_seed_bytes + ) + }} +} + +#[cfg(test)] +#[macro_export] +macro_rules! get_route_and_payment_hash { + ($send_node: expr, $recv_node: expr, $recv_value: expr) => {{ + let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id(), TEST_FINAL_CLTV) + .with_features($recv_node.node.invoice_features()); + $crate::get_route_and_payment_hash!($send_node, $recv_node, payment_params, $recv_value, TEST_FINAL_CLTV) + }}; + ($send_node: expr, $recv_node: expr, $payment_params: expr, $recv_value: expr, $cltv: expr) => {{ + let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value)); + let route = $crate::get_route!($send_node, $payment_params, $recv_value, $cltv); + (route.unwrap(), payment_hash, payment_preimage, payment_secret) + }} +} + #[macro_export] #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] macro_rules! expect_payment_claimable { @@ -2096,7 +2103,7 @@ pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: pub const TEST_FINAL_CLTV: u32 = 70; pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) { - let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(expected_route.last().unwrap().node.invoice_features()); let route = get_route!(origin_node, payment_params, recv_value, TEST_FINAL_CLTV).unwrap(); assert_eq!(route.paths.len(), 1); @@ -2110,7 +2117,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: } pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) { - let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(expected_route.last().unwrap().node.invoice_features()); let network_graph = origin_node.network_graph.read_only(); let scorer = test_utils::TestScorer::with_penalty(0); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 371fcfa7764..bf0c3396ced 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1802,7 +1802,7 @@ fn test_channel_reserve_holding_cell_htlcs() { // attempt to send amt_msat > their_max_htlc_value_in_flight_msat { - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[2].node.invoice_features()).with_max_channel_saturation_power_of_half(0); let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, recv_value_0, TEST_FINAL_CLTV); route.paths[0].last_mut().unwrap().fee_msat += 1; @@ -1827,7 +1827,7 @@ fn test_channel_reserve_holding_cell_htlcs() { break; } - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[2].node.invoice_features()).with_max_channel_saturation_power_of_half(0); let route = get_route!(nodes[0], payment_params, recv_value_0, TEST_FINAL_CLTV).unwrap(); let (payment_preimage, ..) = send_along_route(&nodes[0], route, &[&nodes[1], &nodes[2]], recv_value_0); @@ -4716,7 +4716,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { // We reduce the final CLTV here by a somewhat arbitrary constant to keep it under the one-byte // script push size limit so that the below script length checks match // ACCEPTED_HTLC_SCRIPT_WEIGHT. - let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV - 40) .with_features(nodes[3].node.invoice_features()); let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[3], payment_params, 800_000, TEST_FINAL_CLTV - 40); send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1], &nodes[2], &nodes[3]]], 800_000, duplicate_payment_hash, payment_secret); @@ -6009,7 +6009,7 @@ fn test_update_add_htlc_bolt2_sender_cltv_expiry_too_high() { let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 0); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 0) .with_features(nodes[1].node.invoice_features()); let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, 100000000, 0); route.paths[0].last_mut().unwrap().cltv_expiry_delta = 500000001; @@ -6927,7 +6927,7 @@ fn test_check_htlc_underpaying() { let scorer = test_utils::TestScorer::with_penalty(0); let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features()); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features()); let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 10_000, TEST_FINAL_CLTV, nodes[0].logger, &scorer, &random_seed_bytes).unwrap(); let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]); let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None).unwrap(); @@ -7068,7 +7068,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() { let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000); let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0; - let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 30) .with_features(nodes[0].node.invoice_features()); let (route,_, _, _) = get_route_and_payment_hash!(nodes[1], nodes[0], payment_params, 3000000, 30); send_along_route(&nodes[1], route, &vec!(&nodes[0])[..], 3000000); @@ -7174,13 +7174,13 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 59000000); // Lock HTLC in both directions (using a slightly lower CLTV delay to provide timely RBF bumps) - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features()); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 50).with_features(nodes[1].node.invoice_features()); let scorer = test_utils::TestScorer::with_penalty(0); let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes(); let route = get_route(&nodes[0].node.get_our_node_id(), &payment_params, &nodes[0].network_graph.read_only(), None, 3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap(); let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0; - let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id()).with_features(nodes[0].node.invoice_features()); + let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 50).with_features(nodes[0].node.invoice_features()); let route = get_route(&nodes[1].node.get_our_node_id(), &payment_params, &nodes[1].network_graph.read_only(), None, 3_000_000, 50, nodes[0].logger, &scorer, &random_seed_bytes).unwrap(); send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000); @@ -8976,7 +8976,7 @@ fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) { let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[1].node.invoice_features()); let route = get_route!(nodes[0], payment_params, 10_000, TEST_FINAL_CLTV).unwrap(); @@ -9081,7 +9081,7 @@ fn test_inconsistent_mpp_params() { create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 100_000, 0); let chan_2_3 =create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 100_000, 0); - let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[3].node.invoice_features()); let mut route = get_route!(nodes[0], payment_params, 15_000_000, TEST_FINAL_CLTV).unwrap(); assert_eq!(route.paths.len(), 2); @@ -9182,7 +9182,7 @@ fn test_keysend_payments_to_public_node() { let payer_pubkey = nodes[0].node.get_our_node_id(); let payee_pubkey = nodes[1].node.get_our_node_id(); let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(payee_pubkey), + payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10000, final_cltv_expiry_delta: 40, }; @@ -9215,7 +9215,7 @@ fn test_keysend_payments_to_private_node() { let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]); let route_params = RouteParameters { - payment_params: PaymentParameters::for_keysend(payee_pubkey), + payment_params: PaymentParameters::for_keysend(payee_pubkey, 40), final_value_msat: 10000, final_cltv_expiry_delta: 40, }; @@ -9603,7 +9603,7 @@ fn do_payment_with_custom_min_final_cltv_expiry(valid_delta: bool, use_user_hash create_chan_between_nodes(&nodes[0], &nodes[1]); - let payment_parameters = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()); + let payment_parameters = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), final_cltv_expiry_delta as u32); let (payment_hash, payment_preimage, payment_secret) = if use_user_hash { let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[1], Some(recv_value), Some(min_final_cltv_expiry_delta)); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 1ecdb5a87fc..1ed10a4a61d 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -655,7 +655,7 @@ fn do_test_onion_failure_stale_channel_update(announced_channel: bool) { htlc_maximum_msat: None, htlc_minimum_msat: None, }])]; - let payment_params = PaymentParameters::from_node_id(*channel_to_update_counterparty) + let payment_params = PaymentParameters::from_node_id(*channel_to_update_counterparty, TEST_FINAL_CLTV) .with_features(nodes[2].node.invoice_features()) .with_route_hints(hop_hints); get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, PAYMENT_AMT, TEST_FINAL_CLTV) @@ -802,7 +802,7 @@ fn test_always_create_tlv_format_onion_payloads() { create_announced_chan_between_nodes(&nodes, 0, 1); create_announced_chan_between_nodes(&nodes, 1, 2); - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(InvoiceFeatures::empty()); let (route, _payment_hash, _payment_preimage, _payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 40000, TEST_FINAL_CLTV); @@ -902,7 +902,7 @@ macro_rules! get_phantom_route { ($nodes: expr, $amt: expr, $channel: expr) => {{ let phantom_pubkey = $nodes[1].keys_manager.get_node_id(Recipient::PhantomNode).unwrap(); let phantom_route_hint = $nodes[1].node.get_phantom_route_hints(); - let payment_params = PaymentParameters::from_node_id(phantom_pubkey) + let payment_params = PaymentParameters::from_node_id(phantom_pubkey, TEST_FINAL_CLTV) .with_features($nodes[1].node.invoice_features()) .with_route_hints(vec![RouteHint(vec![ RouteHintHop { diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 8867b2e96ae..f745363036b 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -16,6 +16,7 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::chain::keysinterface::{EntropySource, NodeSigner, Recipient}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channelmanager::{ChannelDetails, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, MIN_HTLC_RELAY_HOLDING_CELL_MILLIS, PaymentId}; +use crate::ln::channelmanager::MIN_FINAL_CLTV_EXPIRY_DELTA as LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA; use crate::ln::msgs::DecodeError; use crate::ln::onion_utils::HTLCFailReason; use crate::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router}; @@ -41,9 +42,9 @@ pub(crate) enum PendingOutboundPayment { session_privs: HashSet<[u8; 32]>, }, Retryable { - retry_strategy: Retry, + retry_strategy: Option, attempts: PaymentAttempts, - route_params: Option, + payment_params: Option, session_privs: HashSet<[u8; 32]>, payment_hash: PaymentHash, payment_secret: Option, @@ -82,15 +83,37 @@ impl PendingOutboundPayment { attempts.count += 1; } } + fn is_auto_retryable_now(&self) -> bool { + match self { + PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => { + strategy.is_retryable_now(&attempts) + }, + _ => false, + } + } fn is_retryable_now(&self) -> bool { - if let PendingOutboundPayment::Retryable { retry_strategy, attempts, .. } = self { - return retry_strategy.is_retryable_now(&attempts) + match self { + PendingOutboundPayment::Retryable { retry_strategy: None, .. } => { + // We're handling retries manually, we can always retry. + true + }, + PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => { + strategy.is_retryable_now(&attempts) + }, + _ => false, + } + } + fn payment_parameters(&mut self) -> Option<&mut PaymentParameters> { + match self { + PendingOutboundPayment::Retryable { payment_params: Some(ref mut params), .. } => { + Some(params) + }, + _ => None, } - false } pub fn insert_previously_failed_scid(&mut self, scid: u64) { - if let PendingOutboundPayment::Retryable { route_params: Some(params), .. } = self { - params.payment_params.previously_failed_channels.push(scid); + if let PendingOutboundPayment::Retryable { payment_params: Some(params), .. } = self { + params.previously_failed_channels.push(scid); } } pub(super) fn is_fulfilled(&self) -> bool { @@ -212,9 +235,9 @@ impl PendingOutboundPayment { pub enum Retry { /// Max number of attempts to retry payment. /// - /// Note that this is the number of *path* failures, not full payment retries. For multi-path - /// payments, if this is less than the total number of paths, we will never even retry all of the - /// payment's paths. + /// Each attempt may be multiple HTLCs along multiple paths if the router decides to split up a + /// retry, and may retry multiple failed HTLCs at once if they failed around the same time and + /// were retried along a route from a single call to [`Router::find_route`]. Attempts(usize), #[cfg(not(feature = "no-std"))] /// Time elapsed before abandoning retries for a payment. @@ -378,22 +401,23 @@ impl OutboundPayments { } } - pub(super) fn send_payment( + pub(super) fn send_payment( &self, payment_hash: PaymentHash, payment_secret: &Option, payment_id: PaymentId, retry_strategy: Retry, route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: InFlightHtlcs, entropy_source: &ES, - node_signer: &NS, best_block_height: u32, send_payment_along_path: F + node_signer: &NS, best_block_height: u32, logger: &L, send_payment_along_path: F, ) -> Result<(), PaymentSendFailure> where R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, + L::Target: Logger, F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, { self.pay_internal(payment_id, Some((payment_hash, payment_secret, retry_strategy)), route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, - best_block_height, &send_payment_along_path) + best_block_height, logger, &send_payment_along_path) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) } @@ -408,7 +432,7 @@ impl OutboundPayments { F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { - let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, None, None, entropy_source, best_block_height)?; self.pay_route_internal(route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) .map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e }) @@ -429,7 +453,7 @@ impl OutboundPayments { None => PaymentPreimage(entropy_source.get_secure_random_bytes()), }; let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner()); - let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?; match self.pay_route_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok(payment_hash), @@ -458,11 +482,19 @@ impl OutboundPayments { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); let mut retry_id_route_params = None; for (pmt_id, pmt) in outbounds.iter_mut() { - if pmt.is_retryable_now() { - if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, route_params: Some(params), .. } = pmt { + if pmt.is_auto_retryable_now() { + if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, payment_params: Some(params), .. } = pmt { if pending_amt_msat < total_msat { - retry_id_route_params = Some((*pmt_id, params.clone())); - pmt.increment_attempts(); + retry_id_route_params = Some((*pmt_id, RouteParameters { + final_value_msat: *total_msat - *pending_amt_msat, + final_cltv_expiry_delta: + if let Some(delta) = params.final_cltv_expiry_delta { delta } + else { + debug_assert!(false, "We always set the final_cltv_expiry_delta when a path fails"); + LDK_DEFAULT_MIN_FINAL_CLTV_EXPIRY_DELTA.into() + }, + payment_params: params.clone(), + })); break } } @@ -470,24 +502,25 @@ impl OutboundPayments { } if let Some((payment_id, route_params)) = retry_id_route_params { core::mem::drop(outbounds); - if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, &send_payment_along_path) { - log_trace!(logger, "Errored retrying payment: {:?}", e); + if let Err(e) = self.pay_internal(payment_id, None, route_params, router, first_hops(), inflight_htlcs(), entropy_source, node_signer, best_block_height, logger, &send_payment_along_path) { + log_info!(logger, "Errored retrying payment: {:?}", e); } } else { break } } } - fn pay_internal( + fn pay_internal( &self, payment_id: PaymentId, initial_send_info: Option<(PaymentHash, &Option, Retry)>, route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: InFlightHtlcs, entropy_source: &ES, node_signer: &NS, best_block_height: u32, - send_payment_along_path: &F + logger: &L, send_payment_along_path: &F, ) -> Result<(), PaymentSendFailure> where R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, + L::Target: Logger, F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> { @@ -507,37 +540,26 @@ impl OutboundPayments { }))?; let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info { - let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy, Some(route_params.clone()), entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.payment_params.clone()), entropy_source, best_block_height)?; self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path) } else { self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path) }; match res { Err(PaymentSendFailure::AllFailedResendSafe(_)) => { - let mut outbounds = self.pending_outbound_payments.lock().unwrap(); - if let Some(payment) = outbounds.get_mut(&payment_id) { - let retryable = payment.is_retryable_now(); - if retryable { - payment.increment_attempts(); - } else { return res } - } else { return res } - core::mem::drop(outbounds); - self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path) + let retry_res = self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path); + log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res); + if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = &retry_res { + if err.starts_with("Retries exhausted ") { return res; } + } + retry_res }, - Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => { - let mut outbounds = self.pending_outbound_payments.lock().unwrap(); - if let Some(payment) = outbounds.get_mut(&payment_id) { - let retryable = payment.is_retryable_now(); - if retryable { - payment.increment_attempts(); - } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) } - } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) } - core::mem::drop(outbounds); - + Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), .. }) => { // Some paths were sent, even if we failed to send the full MPP value our recipient may // misbehave and claim the funds, at which point we have to consider the payment sent, so // return `Ok()` here, ignoring any retry errors. - let _ = self.pay_internal(payment_id, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, send_payment_along_path); + let retry_res = self.pay_internal(payment_id, None, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path); + log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res); Ok(()) }, Err(PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. }) => { @@ -606,6 +628,12 @@ impl OutboundPayments { })); }, }; + if !payment.is_retryable_now() { + return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { + err: format!("Retries exhausted for payment id {}", log_bytes!(payment_id.0)), + })) + } + payment.increment_attempts(); for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) { assert!(payment.insert(*session_priv_bytes, path)); } @@ -641,7 +669,7 @@ impl OutboundPayments { } let route = Route { paths: vec![hops], payment_params: None }; - let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?; + let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?; match self.pay_route_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) { Ok(()) => Ok((payment_hash, payment_id)), @@ -655,14 +683,14 @@ impl OutboundPayments { #[cfg(test)] pub(super) fn test_add_new_pending_payment( &self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, - route: &Route, retry_strategy: Retry, entropy_source: &ES, best_block_height: u32 + route: &Route, retry_strategy: Option, entropy_source: &ES, best_block_height: u32 ) -> Result, PaymentSendFailure> where ES::Target: EntropySource { self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, retry_strategy, None, entropy_source, best_block_height) } pub(super) fn add_new_pending_payment( &self, payment_hash: PaymentHash, payment_secret: Option, payment_id: PaymentId, - route: &Route, retry_strategy: Retry, route_params: Option, + route: &Route, retry_strategy: Option, payment_params: Option, entropy_source: &ES, best_block_height: u32 ) -> Result, PaymentSendFailure> where ES::Target: EntropySource { let mut onion_session_privs = Vec::with_capacity(route.paths.len()); @@ -677,7 +705,7 @@ impl OutboundPayments { let payment = entry.insert(PendingOutboundPayment::Retryable { retry_strategy, attempts: PaymentAttempts::new(), - route_params, + payment_params, session_privs: HashSet::new(), pending_amt_msat: 0, pending_fee_msat: Some(0), @@ -789,7 +817,9 @@ impl OutboundPayments { Some(RouteParameters { payment_params: payment_params.clone(), final_value_msat: pending_amt_unsent, - final_cltv_expiry_delta: max_unsent_cltv_delta, + final_cltv_expiry_delta: + if let Some(delta) = payment_params.final_cltv_expiry_delta { delta } + else { max_unsent_cltv_delta }, }) } else { None } } else { None }, @@ -953,6 +983,7 @@ impl OutboundPayments { let mut all_paths_failed = false; let mut full_failure_ev = None; let mut pending_retry_ev = None; + let mut retry = None; let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) { if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); @@ -962,10 +993,37 @@ impl OutboundPayments { log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0)); return } - let is_retryable_now = payment.get().is_retryable_now(); + let is_retryable_now = payment.get().is_auto_retryable_now(); if let Some(scid) = short_channel_id { payment.get_mut().insert_previously_failed_scid(scid); } + + // We want to move towards only using the `PaymentParameters` in the outbound payments + // map. However, for backwards-compatibility, we still need to support passing the + // `PaymentParameters` data that was shoved in the HTLC (and given to us via + // `payment_params`) back to the user. + let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); + if let Some(params) = payment.get_mut().payment_parameters() { + if params.final_cltv_expiry_delta.is_none() { + // This should be rare, but a user could provide None for the payment data, and + // we need it when we go to retry the payment, so fill it in. + params.final_cltv_expiry_delta = Some(path_last_hop.cltv_expiry_delta); + } + retry = Some(RouteParameters { + payment_params: params.clone(), + final_value_msat: path_last_hop.fee_msat, + final_cltv_expiry_delta: params.final_cltv_expiry_delta.unwrap(), + }); + } else if let Some(params) = payment_params { + retry = Some(RouteParameters { + payment_params: params.clone(), + final_value_msat: path_last_hop.fee_msat, + final_cltv_expiry_delta: + if let Some(delta) = params.final_cltv_expiry_delta { delta } + else { path_last_hop.cltv_expiry_delta }, + }); + } + if payment.get().remaining_parts() == 0 { all_paths_failed = true; if payment.get().abandoned() { @@ -982,14 +1040,6 @@ impl OutboundPayments { return }; core::mem::drop(outbounds); - let mut retry = if let Some(payment_params_data) = payment_params { - let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); - Some(RouteParameters { - payment_params: payment_params_data.clone(), - final_value_msat: path_last_hop.fee_msat, - final_cltv_expiry_delta: path_last_hop.cltv_expiry_delta, - }) - } else { None }; log_trace!(logger, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0)); let path_failure = { @@ -1101,13 +1151,13 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (0, session_privs, required), (1, pending_fee_msat, option), (2, payment_hash, required), - (not_written, retry_strategy, (static_value, Retry::Attempts(0))), + (3, payment_params, option), (4, payment_secret, option), - (not_written, attempts, (static_value, PaymentAttempts::new())), (6, total_msat, required), - (not_written, route_params, (static_value, None)), (8, pending_amt_msat, required), (10, starting_block_height, required), + (not_written, retry_strategy, (static_value, None)), + (not_written, attempts, (static_value, PaymentAttempts::new())), }, (3, Abandoned) => { (0, session_privs, required), @@ -1149,8 +1199,9 @@ mod tests { let past_expiry_time = std::time::SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() - 2; let payment_params = PaymentParameters::from_node_id( - PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap())) - .with_expiry_time(past_expiry_time); + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), + 0 + ).with_expiry_time(past_expiry_time); let expired_route_params = RouteParameters { payment_params, final_value_msat: 0, @@ -1159,11 +1210,12 @@ mod tests { let err = if on_retry { outbound_payments.pay_internal( PaymentId([0; 32]), None, expired_route_params, &&router, vec![], InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() } else { outbound_payments.send_payment( PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params, - &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, + |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() }; if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err { assert!(err.contains("Invoice expired")); @@ -1184,26 +1236,28 @@ mod tests { let secp_ctx = Secp256k1::new(); let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); - router.expect_find_route(Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })); - let payment_params = PaymentParameters::from_node_id( - PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap())); + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0); let route_params = RouteParameters { payment_params, final_value_msat: 0, final_cltv_expiry_delta: 0, }; + router.expect_find_route(route_params.clone(), + Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError })); + let err = if on_retry { outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]), - &Route { paths: vec![], payment_params: None }, Retry::Attempts(1), Some(route_params.clone()), + &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap(); outbound_payments.pay_internal( PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(), - &&keys_manager, &&keys_manager, 0, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + &&keys_manager, &&keys_manager, 0, &&logger, &|_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() } else { outbound_payments.send_payment( PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params, - &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() + &&router, vec![], InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, + |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err() }; if let PaymentSendFailure::ParameterError(APIError::APIMisuseError { err }) = err { assert!(err.contains("Failed to find a route")); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 726c5b89e54..68074f1b59b 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -919,7 +919,7 @@ fn get_ldk_payment_preimage() { let expiry_secs = 60 * 60; let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(amt_msat), expiry_secs, None).unwrap(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[1].node.invoice_features()); let scorer = test_utils::TestScorer::with_penalty(0); let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); @@ -1033,7 +1033,7 @@ fn failed_probe_yields_event() { create_announced_chan_between_nodes(&nodes, 0, 1); create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000); - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()); + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42); let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_998_000, 42); @@ -1080,7 +1080,7 @@ fn onchain_failed_probe_yields_event() { let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; create_announced_chan_between_nodes(&nodes, 1, 2); - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()); + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42); // Send a dust HTLC, which will be treated as if it timed out once the channel hits the chain. let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 1_000, 42); @@ -1425,7 +1425,7 @@ fn do_test_intercepted_payment(test: InterceptTest) { let amt_msat = 100_000; let intercept_scid = nodes[1].node.get_intercept_scid(); - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_route_hints(vec![ RouteHint(vec![RouteHintHop { src_node_id: nodes[1].node.get_our_node_id(), @@ -1623,7 +1623,7 @@ fn do_automatic_retries(test: AutoRetry) { invoice_features.set_variable_length_onion_required(); invoice_features.set_payment_secret_required(); invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_features(invoice_features); let route_params = RouteParameters { @@ -1805,7 +1805,7 @@ fn auto_retry_partial_failure() { invoice_features.set_variable_length_onion_required(); invoice_features.set_payment_secret_required(); invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_features(invoice_features); let route_params = RouteParameters { @@ -1844,7 +1844,7 @@ fn auto_retry_partial_failure() { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(route_params.payment_params.clone()), }; let retry_1_route = Route { paths: vec![ @@ -1865,7 +1865,7 @@ fn auto_retry_partial_failure() { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(route_params.payment_params.clone()), }; let retry_2_route = Route { paths: vec![ @@ -1878,11 +1878,17 @@ fn auto_retry_partial_failure() { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(route_params.payment_params.clone()), }; - nodes[0].router.expect_find_route(Ok(send_route)); - nodes[0].router.expect_find_route(Ok(retry_1_route)); - nodes[0].router.expect_find_route(Ok(retry_2_route)); + nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route)); + nodes[0].router.expect_find_route(RouteParameters { + payment_params: route_params.payment_params.clone(), + final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV + }, Ok(retry_1_route)); + nodes[0].router.expect_find_route(RouteParameters { + payment_params: route_params.payment_params.clone(), + final_value_msat: amt_msat / 4, final_cltv_expiry_delta: TEST_FINAL_CLTV + }, Ok(retry_2_route)); // Send a payment that will partially fail on send, then partially fail on retry, then succeed. nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(3)).unwrap(); @@ -2001,7 +2007,7 @@ fn auto_retry_zero_attempts_send_error() { invoice_features.set_variable_length_onion_required(); invoice_features.set_payment_secret_required(); invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_features(invoice_features); let route_params = RouteParameters { @@ -2039,7 +2045,7 @@ fn fails_paying_after_rejected_by_payee() { invoice_features.set_variable_length_onion_required(); invoice_features.set_payment_secret_required(); invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_features(invoice_features); let route_params = RouteParameters { @@ -2074,6 +2080,27 @@ fn retry_multi_path_single_failed_payment() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + + let amt_msat = 100_010_000; + + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params: payment_params.clone(), + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + let chans = nodes[0].node.list_usable_channels(); let mut route = Route { paths: vec![ @@ -2094,15 +2121,37 @@ fn retry_multi_path_single_failed_payment() { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(payment_params), }; - nodes[0].router.expect_find_route(Ok(route.clone())); + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); // On retry, split the payment across both channels. route.paths[0][0].fee_msat = 50_000_001; route.paths[1][0].fee_msat = 50_000_000; - nodes[0].router.expect_find_route(Ok(route.clone())); + nodes[0].router.expect_find_route(RouteParameters { + payment_params: route.payment_params.clone().unwrap(), + // Note that the second request here requests the amount we originally failed to send, + // not the amount remaining on the full payment, which should be changed. + final_value_msat: 100_000_001, final_cltv_expiry_delta: TEST_FINAL_CLTV + }, Ok(route.clone())); - let amt_msat = 100_010_000; + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(htlc_msgs.len(), 2); + check_added_monitors!(nodes[0], 2); +} + +#[test] +fn immediate_retry_on_failure() { + // Tests that we can/will retry immediately after a failure + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + + let amt_msat = 100_000_001; let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); #[cfg(feature = "std")] let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; @@ -2112,7 +2161,7 @@ fn retry_multi_path_single_failed_payment() { invoice_features.set_variable_length_onion_required(); invoice_features.set_payment_secret_required(); invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) .with_expiry_time(payment_expiry_secs as u64) .with_features(invoice_features); let route_params = RouteParameters { @@ -2121,22 +2170,6 @@ fn retry_multi_path_single_failed_payment() { final_cltv_expiry_delta: TEST_FINAL_CLTV, }; - nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); - let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(htlc_msgs.len(), 2); - check_added_monitors!(nodes[0], 2); -} - -#[test] -fn immediate_retry_on_failure() { - // Tests that we can/will retry immediately after a failure - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None, None]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); - create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); let chans = nodes[0].node.list_usable_channels(); let mut route = Route { paths: vec![ @@ -2149,34 +2182,18 @@ fn immediate_retry_on_failure() { cltv_expiry_delta: 100, }], ], - payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())), + payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV)), }; - nodes[0].router.expect_find_route(Ok(route.clone())); + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); // On retry, split the payment across both channels. route.paths.push(route.paths[0].clone()); route.paths[0][0].short_channel_id = chans[1].short_channel_id.unwrap(); route.paths[0][0].fee_msat = 50_000_000; route.paths[1][0].fee_msat = 50_000_001; - nodes[0].router.expect_find_route(Ok(route.clone())); - - let amt_msat = 100_010_000; - let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); - #[cfg(feature = "std")] - let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; - #[cfg(not(feature = "std"))] - let payment_expiry_secs = 60 * 60; - let mut invoice_features = InvoiceFeatures::empty(); - invoice_features.set_variable_length_onion_required(); - invoice_features.set_payment_secret_required(); - invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) - .with_expiry_time(payment_expiry_secs as u64) - .with_features(invoice_features); - let route_params = RouteParameters { - payment_params, - final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, - }; + nodes[0].router.expect_find_route(RouteParameters { + payment_params: route_params.payment_params.clone(), + final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV + }, Ok(route.clone())); nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events(); @@ -2208,6 +2225,25 @@ fn no_extra_retries_on_back_to_back_fail() { let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id; let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id; + let amt_msat = 200_000_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + let mut route = Route { paths: vec![ vec![RouteHop { @@ -2215,7 +2251,7 @@ fn no_extra_retries_on_back_to_back_fail() { node_features: nodes[1].node.node_features(), short_channel_id: chan_1_scid, channel_features: nodes[1].node.channel_features(), - fee_msat: 0, + fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee cltv_expiry_delta: 100, }, RouteHop { pubkey: nodes[2].node.get_our_node_id(), @@ -2230,7 +2266,7 @@ fn no_extra_retries_on_back_to_back_fail() { node_features: nodes[1].node.node_features(), short_channel_id: chan_1_scid, channel_features: nodes[1].node.channel_features(), - fee_msat: 0, + fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee cltv_expiry_delta: 100, }, RouteHop { pubkey: nodes[2].node.get_our_node_id(), @@ -2241,31 +2277,18 @@ fn no_extra_retries_on_back_to_back_fail() { cltv_expiry_delta: 100, }] ], - payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())), + payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)), }; - nodes[0].router.expect_find_route(Ok(route.clone())); - // On retry, we'll only be asked for one path + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); + let mut second_payment_params = route_params.payment_params.clone(); + second_payment_params.previously_failed_channels = vec![chan_2_scid, chan_2_scid]; + // On retry, we'll only return one path route.paths.remove(1); - nodes[0].router.expect_find_route(Ok(route.clone())); - - let amt_msat = 100_010_000; - let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat); - #[cfg(feature = "std")] - let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; - #[cfg(not(feature = "std"))] - let payment_expiry_secs = 60 * 60; - let mut invoice_features = InvoiceFeatures::empty(); - invoice_features.set_variable_length_onion_required(); - invoice_features.set_payment_secret_required(); - invoice_features.set_basic_mpp_optional(); - let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()) - .with_expiry_time(payment_expiry_secs as u64) - .with_features(invoice_features); - let route_params = RouteParameters { - payment_params, - final_value_msat: amt_msat, - final_cltv_expiry_delta: TEST_FINAL_CLTV, - }; + route.paths[0][1].fee_msat = amt_msat; + nodes[0].router.expect_find_route(RouteParameters { + payment_params: second_payment_params, + final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV, + }, Ok(route.clone())); nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); let htlc_updates = SendEvent::from_node(&nodes[0]); @@ -2329,10 +2352,16 @@ fn no_extra_retries_on_back_to_back_fail() { // At this point A has sent two HTLCs which both failed due to lack of fee. It now has two // pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second - // with it set. The first event will use up the only retry we are allowed, with the second - // `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd - // treated this as "HTLC complete" and dropped the retry counter, causing us to retry again - // if the final HTLC failed. + // with it set. + // + // Previously, we retried payments in an event consumer, which would retry each + // `PaymentPathFailed` individually. In that setup, we had retried the payment in response to + // the first `PaymentPathFailed`, then seen the second `PaymentPathFailed` with + // `all_paths_failed` set and assumed the payment was completely failed. We ultimately fixed it + // by adding the `PaymentFailed` event. + // + // Because we now retry payments as a batch, we simply return a single-path route in the + // second, batched, request, have that fail, then complete the payment via `abandon_payment`. let mut events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 4); match events[0] { @@ -2388,3 +2417,161 @@ fn no_extra_retries_on_back_to_back_fail() { _ => panic!("Unexpected event"), } } + +#[test] +fn test_simple_partial_retry() { + // In the first version of the in-`ChannelManager` payment retries, retries were sent for the + // full amount of the payment, rather than only the missing amount. Here we simply test for + // this by sending a payment with two parts, failing one, and retrying the second. Note that + // `TestRouter` will check that the `RouteParameters` (which contain the amount) matches the + // request. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let chan_1_scid = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 0).0.contents.short_channel_id; + let chan_2_scid = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 0).0.contents.short_channel_id; + + let amt_msat = 200_000_000; + let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[2], amt_msat); + #[cfg(feature = "std")] + let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60; + #[cfg(not(feature = "std"))] + let payment_expiry_secs = 60 * 60; + let mut invoice_features = InvoiceFeatures::empty(); + invoice_features.set_variable_length_onion_required(); + invoice_features.set_payment_secret_required(); + invoice_features.set_basic_mpp_optional(); + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_expiry_time(payment_expiry_secs as u64) + .with_features(invoice_features); + let route_params = RouteParameters { + payment_params, + final_value_msat: amt_msat, + final_cltv_expiry_delta: TEST_FINAL_CLTV, + }; + + let mut route = Route { + paths: vec![ + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_scid, + channel_features: nodes[1].node.channel_features(), + fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee + cltv_expiry_delta: 100, + }, RouteHop { + pubkey: nodes[2].node.get_our_node_id(), + node_features: nodes[2].node.node_features(), + short_channel_id: chan_2_scid, + channel_features: nodes[2].node.channel_features(), + fee_msat: 100_000_000, + cltv_expiry_delta: 100, + }], + vec![RouteHop { + pubkey: nodes[1].node.get_our_node_id(), + node_features: nodes[1].node.node_features(), + short_channel_id: chan_1_scid, + channel_features: nodes[1].node.channel_features(), + fee_msat: 100_000, + cltv_expiry_delta: 100, + }, RouteHop { + pubkey: nodes[2].node.get_our_node_id(), + node_features: nodes[2].node.node_features(), + short_channel_id: chan_2_scid, + channel_features: nodes[2].node.channel_features(), + fee_msat: 100_000_000, + cltv_expiry_delta: 100, + }] + ], + payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)), + }; + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); + let mut second_payment_params = route_params.payment_params.clone(); + second_payment_params.previously_failed_channels = vec![chan_2_scid]; + // On retry, we'll only be asked for one path (or 100k sats) + route.paths.remove(0); + nodes[0].router.expect_find_route(RouteParameters { + payment_params: second_payment_params, + final_value_msat: amt_msat / 2, final_cltv_expiry_delta: TEST_FINAL_CLTV, + }, Ok(route.clone())); + + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)).unwrap(); + let htlc_updates = SendEvent::from_node(&nodes[0]); + check_added_monitors!(nodes[0], 1); + assert_eq!(htlc_updates.msgs.len(), 1); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &htlc_updates.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &htlc_updates.commitment_msg); + check_added_monitors!(nodes[1], 1); + let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa); + check_added_monitors!(nodes[0], 1); + let second_htlc_updates = SendEvent::from_node(&nodes[0]); + + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs); + check_added_monitors!(nodes[0], 1); + let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &second_htlc_updates.msgs[0]); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &second_htlc_updates.commitment_msg); + check_added_monitors!(nodes[1], 1); + let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa); + check_added_monitors!(nodes[1], 1); + let bs_fail_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa); + check_added_monitors!(nodes[0], 1); + + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &bs_fail_update.update_fail_htlcs[0]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_fail_update.commitment_signed); + check_added_monitors!(nodes[0], 1); + let (as_second_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa); + check_added_monitors!(nodes[1], 1); + + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs); + check_added_monitors!(nodes[1], 1); + + let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa); + check_added_monitors!(nodes[0], 1); + + let mut events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + match events[0] { + Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => { + assert_eq!(payment_hash, ev_payment_hash); + assert_eq!(payment_failed_permanently, false); + }, + _ => panic!("Unexpected event"), + } + match events[1] { + Event::PendingHTLCsForwardable { .. } => {}, + _ => panic!("Unexpected event"), + } + + nodes[0].node.process_pending_htlc_forwards(); + let retry_htlc_updates = SendEvent::from_node(&nodes[0]); + check_added_monitors!(nodes[0], 1); + + nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &retry_htlc_updates.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], &retry_htlc_updates.commitment_msg, false, true); + + expect_pending_htlcs_forwardable!(nodes[1]); + check_added_monitors!(nodes[1], 1); + + let bs_forward_update = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id()); + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &bs_forward_update.update_add_htlcs[0]); + nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &bs_forward_update.update_add_htlcs[1]); + commitment_signed_dance!(nodes[2], nodes[1], &bs_forward_update.commitment_signed, false); + + expect_pending_htlcs_forwardable!(nodes[2]); + expect_payment_claimable!(nodes[2], payment_hash, payment_secret, amt_msat); +} diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index aca781b9fd6..a6fd7c1dc09 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -66,7 +66,7 @@ fn test_priv_forwarding_rejection() { htlc_maximum_msat: None, }]); let last_hops = vec![route_hint]; - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV) .with_features(nodes[2].node.invoice_features()) .with_route_hints(last_hops); let (route, our_payment_hash, our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 10_000, TEST_FINAL_CLTV); @@ -233,7 +233,7 @@ fn test_routed_scid_alias() { htlc_maximum_msat: None, htlc_minimum_msat: None, }])]; - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42) .with_features(nodes[2].node.invoice_features()) .with_route_hints(hop_hints); let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 100_000, 42); @@ -388,7 +388,7 @@ fn test_inbound_scid_privacy() { htlc_maximum_msat: None, htlc_minimum_msat: None, }])]; - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42) .with_features(nodes[2].node.invoice_features()) .with_route_hints(hop_hints.clone()); let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 100_000, 42); @@ -403,7 +403,7 @@ fn test_inbound_scid_privacy() { // what channel we're talking about. hop_hints[0].0[0].short_channel_id = last_hop[0].short_channel_id.unwrap(); - let payment_params_2 = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params_2 = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42) .with_features(nodes[2].node.invoice_features()) .with_route_hints(hop_hints); let (route_2, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params_2, 100_000, 42); @@ -454,7 +454,7 @@ fn test_scid_alias_returned() { htlc_maximum_msat: None, htlc_minimum_msat: None, }])]; - let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id()) + let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42) .with_features(nodes[2].node.invoice_features()) .with_route_hints(hop_hints); let (mut route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, 10_000, 42); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 2e703600f55..c1fe23a62a6 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -93,9 +93,9 @@ fn updates_shutdown_wait() { let (_, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[0]); - let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id()).with_features(nodes[1].node.invoice_features()); + let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[1].node.invoice_features()); let route_1 = get_route(&nodes[0].node.get_our_node_id(), &payment_params_1, &nodes[0].network_graph.read_only(), None, 100000, TEST_FINAL_CLTV, &logger, &scorer, &random_seed_bytes).unwrap(); - let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id()).with_features(nodes[0].node.invoice_features()); + let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), TEST_FINAL_CLTV).with_features(nodes[0].node.invoice_features()); let route_2 = get_route(&nodes[1].node.get_our_node_id(), &payment_params_2, &nodes[1].network_graph.read_only(), None, 100000, TEST_FINAL_CLTV, &logger, &scorer, &random_seed_bytes).unwrap(); unwrap_send_err!(nodes[0].node.send_payment(&route_1, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)), true, APIError::ChannelUnavailable {..}, {}); unwrap_send_err!(nodes[1].node.send_payment(&route_2, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)), true, APIError::ChannelUnavailable {..}, {}); diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 1e2ca10c0bf..950384d957e 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -372,6 +372,9 @@ pub struct RouteParameters { pub final_value_msat: u64, /// The CLTV on the final hop of the failed payment path. + /// + /// This field is deprecated, [`PaymentParameters::final_cltv_expiry_delta`] should be used + /// instead, if available. pub final_cltv_expiry_delta: u32, } @@ -452,6 +455,11 @@ pub struct PaymentParameters { /// payment to fail. Future attempts for the same payment shouldn't be relayed through any of /// these SCIDs. pub previously_failed_channels: Vec, + + /// The minimum CLTV delta at the end of the route. + /// + /// This field should always be set to `Some` and may be required in a future release. + pub final_cltv_expiry_delta: Option, } impl_writeable_tlv_based!(PaymentParameters, { @@ -463,11 +471,15 @@ impl_writeable_tlv_based!(PaymentParameters, { (5, max_channel_saturation_power_of_half, (default_value, 2)), (6, expiry_time, option), (7, previously_failed_channels, vec_type), + (9, final_cltv_expiry_delta, option), }); impl PaymentParameters { /// Creates a payee with the node id of the given `pubkey`. - pub fn from_node_id(payee_pubkey: PublicKey) -> Self { + /// + /// The `final_cltv_expiry_delta` should match the expected final CLTV delta the recipient has + /// provided. + pub fn from_node_id(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32) -> Self { Self { payee_pubkey, features: None, @@ -477,12 +489,16 @@ impl PaymentParameters { max_path_count: DEFAULT_MAX_PATH_COUNT, max_channel_saturation_power_of_half: 2, previously_failed_channels: Vec::new(), + final_cltv_expiry_delta: Some(final_cltv_expiry_delta), } } /// Creates a payee with the node id of the given `pubkey` to use for keysend payments. - pub fn for_keysend(payee_pubkey: PublicKey) -> Self { - Self::from_node_id(payee_pubkey).with_features(InvoiceFeatures::for_keysend()) + /// + /// The `final_cltv_expiry_delta` should match the expected final CLTV delta the recipient has + /// provided. + pub fn for_keysend(payee_pubkey: PublicKey, final_cltv_expiry_delta: u32) -> Self { + Self::from_node_id(payee_pubkey, final_cltv_expiry_delta).with_features(InvoiceFeatures::for_keysend()) } /// Includes the payee's features. @@ -948,8 +964,11 @@ pub fn find_route( ) -> Result where L::Target: Logger, GL::Target: Logger { let graph_lock = network_graph.read_only(); + let final_cltv_expiry_delta = + if let Some(delta) = route_params.payment_params.final_cltv_expiry_delta { delta } + else { route_params.final_cltv_expiry_delta }; let mut route = get_route(our_node_pubkey, &route_params.payment_params, &graph_lock, first_hops, - route_params.final_value_msat, route_params.final_cltv_expiry_delta, logger, scorer, + route_params.final_value_msat, final_cltv_expiry_delta, logger, scorer, random_seed_bytes)?; add_random_cltv_offset(&mut route, &route_params.payment_params, &graph_lock, random_seed_bytes); Ok(route) @@ -986,6 +1005,9 @@ where L::Target: Logger { if payment_params.max_total_cltv_expiry_delta <= final_cltv_expiry_delta { return Err(LightningError{err: "Can't find a route where the maximum total CLTV expiry delta is below the final CLTV expiry.".to_owned(), action: ErrorAction::IgnoreError}); } + if let Some(delta) = payment_params.final_cltv_expiry_delta { + debug_assert_eq!(delta, final_cltv_expiry_delta); + } // The general routing idea is the following: // 1. Fill first/last hops communicated by the caller. @@ -2165,7 +2187,7 @@ mod tests { fn simple_route_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2198,7 +2220,7 @@ mod tests { fn invalid_first_hop_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2220,7 +2242,7 @@ mod tests { fn htlc_minimum_test() { let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2348,7 +2370,7 @@ mod tests { let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config)); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2486,7 +2508,7 @@ mod tests { fn disable_channels_test() { let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph(); let (our_privkey, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2546,7 +2568,7 @@ mod tests { fn disable_node_test() { let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph(); let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2596,7 +2618,7 @@ mod tests { let random_seed_bytes = keys_manager.get_secure_random_bytes(); // Route to 1 via 2 and 3 because our channel to 1 is disabled - let payment_params = PaymentParameters::from_node_id(nodes[0]); + let payment_params = PaymentParameters::from_node_id(nodes[0], 42); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths[0].len(), 3); @@ -2622,7 +2644,7 @@ mod tests { assert_eq!(route.paths[0][2].channel_features.le_flags(), &id_to_feature_flags(3)); // If we specify a channel to node7, that overrides our local channel view and that gets used - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)]; let route = get_route(&our_id, &payment_params, &network_graph.read_only(), Some(&our_chans.iter().collect::>()), 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -2746,13 +2768,13 @@ mod tests { let mut invalid_last_hops = last_hops_multi_private_channels(&nodes); invalid_last_hops.push(invalid_last_hop); { - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(invalid_last_hops); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(invalid_last_hops); if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes) { assert_eq!(err, "Route hint cannot have the payee as the source."); } else { panic!(); } } - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops_multi_private_channels(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops_multi_private_channels(&nodes)); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths[0].len(), 5); @@ -2822,7 +2844,7 @@ mod tests { fn ignores_empty_last_hops_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(empty_last_hop(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(empty_last_hop(&nodes)); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2902,7 +2924,7 @@ mod tests { let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph(); let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx); let last_hops = multi_hop_last_hops_hint([nodes[2], nodes[3]]); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone()); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone()); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -2976,7 +2998,7 @@ mod tests { let non_announced_pubkey = PublicKey::from_secret_key(&secp_ctx, &non_announced_privkey); let last_hops = multi_hop_last_hops_hint([nodes[2], non_announced_pubkey]); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone()); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone()); let scorer = ln_test_utils::TestScorer::with_penalty(0); // Test through channels 2, 3, 0xff00, 0xff01. // Test shows that multiple hop hints are considered. @@ -3082,7 +3104,7 @@ mod tests { fn last_hops_with_public_channel_test() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops_with_public_channel(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops_with_public_channel(&nodes)); let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -3141,7 +3163,7 @@ mod tests { // Simple test with outbound channel to 4 to test that last_hops and first_hops connect let our_chans = vec![get_channel_details(Some(42), nodes[3].clone(), InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)]; let mut last_hops = last_hops(&nodes); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops.clone()); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops.clone()); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), Some(&our_chans.iter().collect::>()), 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths[0].len(), 2); @@ -3162,7 +3184,7 @@ mod tests { last_hops[0].0[0].fees.base_msat = 1000; // Revert to via 6 as the fee on 8 goes up - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); assert_eq!(route.paths[0].len(), 4); @@ -3255,7 +3277,7 @@ mod tests { htlc_minimum_msat: None, htlc_maximum_msat: last_hop_htlc_max, }]); - let payment_params = PaymentParameters::from_node_id(target_node_id).with_route_hints(vec![last_hops]); + let payment_params = PaymentParameters::from_node_id(target_node_id, 42).with_route_hints(vec![last_hops]); let our_chans = vec![get_channel_details(Some(42), middle_node_id, InitFeatures::from_le_bytes(vec![0b11]), outbound_capacity_msat)]; let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); @@ -3322,7 +3344,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config)); // We will use a simple single-path route from // our node to node2 via node0: channels {1, 3}. @@ -3597,7 +3619,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config)); // Path via {node7, node2, node4} is channels {12, 13, 6, 11}. // {12, 13, 11} have the capacities of 100, {6} has a capacity of 50. @@ -3722,7 +3744,7 @@ mod tests { let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); // Path via node0 is channels {1, 3}. Limit them to 100 and 50 sats (total limit 50). update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate { @@ -3771,7 +3793,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]) + let payment_params = PaymentParameters::from_node_id(nodes[2], 42) .with_features(channelmanager::provided_invoice_features(&config)); // We need a route consisting of 3 paths: @@ -3931,7 +3953,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config)); // We need a route consisting of 3 paths: // From our node to node3 via {node0, node2}, {node7, node2, node4} and {node7, node2}. @@ -4096,7 +4118,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config)); // This test checks that if we have two cheaper paths and one more expensive path, // so that liquidity-wise any 2 of 3 combination is sufficient, @@ -4266,7 +4288,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[3]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[3], 42).with_features(channelmanager::provided_invoice_features(&config)); // We need a route consisting of 2 paths: // From our node to node3 via {node0, node2} and {node7, node2, node4}. @@ -4448,7 +4470,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(PublicKey::from_slice(&[02; 33]).unwrap()).with_features(channelmanager::provided_invoice_features(&config)) + let payment_params = PaymentParameters::from_node_id(PublicKey::from_slice(&[02; 33]).unwrap(), 42).with_features(channelmanager::provided_invoice_features(&config)) .with_route_hints(vec![RouteHint(vec![RouteHintHop { src_node_id: nodes[2], short_channel_id: 42, @@ -4540,7 +4562,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config)) + let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config)) .with_max_channel_saturation_power_of_half(0); // We need a route consisting of 3 paths: @@ -4701,7 +4723,7 @@ mod tests { let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let payment_params = PaymentParameters::from_node_id(nodes[6]); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42); add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6); update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate { @@ -4832,7 +4854,7 @@ mod tests { let scorer = ln_test_utils::TestScorer::with_penalty(0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let payment_params = PaymentParameters::from_node_id(nodes[2]); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42); // We modify the graph to set the htlc_maximum of channel 2 to below the value we wish to // send. @@ -4897,7 +4919,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config)); // We modify the graph to set the htlc_minimum of channel 2 and 4 as needed - channel 2 // gets an htlc_maximum_msat of 80_000 and channel 4 an htlc_minimum_msat of 90_000. We @@ -4966,7 +4988,7 @@ mod tests { let network_graph = NetworkGraph::new(genesis_hash, Arc::clone(&logger)); let scorer = ln_test_utils::TestScorer::with_penalty(0); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[0]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[0], 42).with_features(channelmanager::provided_invoice_features(&config)); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -5032,7 +5054,7 @@ mod tests { fn prefers_shorter_route_with_higher_fees() { let (secp_ctx, network_graph, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes)); // Without penalizing each hop 100 msats, a longer path with lower fees is chosen. let scorer = ln_test_utils::TestScorer::with_penalty(0); @@ -5105,7 +5127,7 @@ mod tests { fn avoids_routing_through_bad_channels_and_nodes() { let (secp_ctx, network, _, _, logger) = build_graph(); let (_, our_id, _, nodes) = get_nodes(&secp_ctx); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes)); let network_graph = network.read_only(); // A path to nodes[6] exists when no penalties are applied to any channel. @@ -5228,7 +5250,7 @@ mod tests { // Make sure that generally there is at least one route available let feasible_max_total_cltv_delta = 1008; - let feasible_payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)) + let feasible_payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes)) .with_max_total_cltv_expiry_delta(feasible_max_total_cltv_delta); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -5238,7 +5260,7 @@ mod tests { // But not if we exclude all paths on the basis of their accumulated CLTV delta let fail_max_total_cltv_delta = 23; - let fail_payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)) + let fail_payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes)) .with_max_total_cltv_expiry_delta(fail_max_total_cltv_delta); match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes) { @@ -5258,7 +5280,7 @@ mod tests { let network_graph = network.read_only(); let scorer = ln_test_utils::TestScorer::with_penalty(0); - let mut payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)) + let mut payment_params = PaymentParameters::from_node_id(nodes[6], 0).with_route_hints(last_hops(&nodes)) .with_max_path_count(1); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -5289,14 +5311,14 @@ mod tests { let random_seed_bytes = keys_manager.get_secure_random_bytes(); // First check we can actually create a long route on this graph. - let feasible_payment_params = PaymentParameters::from_node_id(nodes[18]); + let feasible_payment_params = PaymentParameters::from_node_id(nodes[18], 0); let route = get_route(&our_id, &feasible_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); let path = route.paths[0].iter().map(|hop| hop.short_channel_id).collect::>(); assert!(path.len() == MAX_PATH_LENGTH_ESTIMATE.into()); // But we can't create a path surpassing the MAX_PATH_LENGTH_ESTIMATE limit. - let fail_payment_params = PaymentParameters::from_node_id(nodes[19]); + let fail_payment_params = PaymentParameters::from_node_id(nodes[19], 0); match get_route(&our_id, &fail_payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes) { @@ -5314,7 +5336,7 @@ mod tests { let scorer = ln_test_utils::TestScorer::with_penalty(0); - let payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes)); + let payment_params = PaymentParameters::from_node_id(nodes[6], 42).with_route_hints(last_hops(&nodes)); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap(); @@ -5347,7 +5369,7 @@ mod tests { let network_nodes = network_graph.nodes(); let network_channels = network_graph.channels(); let scorer = ln_test_utils::TestScorer::with_penalty(0); - let payment_params = PaymentParameters::from_node_id(nodes[3]); + let payment_params = PaymentParameters::from_node_id(nodes[3], 0); let keys_manager = ln_test_utils::TestKeysInterface::new(&[4u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); @@ -5414,7 +5436,7 @@ mod tests { let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); - let payment_params = PaymentParameters::from_node_id(nodes[3]); + let payment_params = PaymentParameters::from_node_id(nodes[3], 0); let hops = [nodes[1], nodes[2], nodes[4], nodes[3]]; let route = build_route_from_hops_internal(&our_id, &hops, &payment_params, &network_graph, 100, 0, Arc::clone(&logger), &random_seed_bytes).unwrap(); @@ -5460,7 +5482,7 @@ mod tests { }); let config = UserConfig::default(); - let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(nodes[2], 42).with_features(channelmanager::provided_invoice_features(&config)); let keys_manager = ln_test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet); let random_seed_bytes = keys_manager.get_secure_random_bytes(); // 100,000 sats is less than the available liquidity on each channel, set above. @@ -5507,7 +5529,7 @@ mod tests { let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed = seed.overflowing_mul(0xdeadbeef).0; let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payment_params = PaymentParameters::from_node_id(dst); + let payment_params = PaymentParameters::from_node_id(dst, 42); let amt = seed as u64 % 200_000_000; let params = ProbabilisticScoringParameters::default(); let scorer = ProbabilisticScorer::new(params, &graph, &logger); @@ -5545,7 +5567,7 @@ mod tests { let src = &PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed = seed.overflowing_mul(0xdeadbeef).0; let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let payment_params = PaymentParameters::from_node_id(dst).with_features(channelmanager::provided_invoice_features(&config)); + let payment_params = PaymentParameters::from_node_id(dst, 42).with_features(channelmanager::provided_invoice_features(&config)); let amt = seed as u64 % 200_000_000; let params = ProbabilisticScoringParameters::default(); let scorer = ProbabilisticScorer::new(params, &graph, &logger); @@ -5578,7 +5600,7 @@ mod tests { assert_eq!(scorer.channel_penalty_msat(42, &NodeId::from_pubkey(&nodes[3]), &NodeId::from_pubkey(&nodes[4]), usage), 456); // Then check we can get a normal route - let payment_params = PaymentParameters::from_node_id(nodes[10]); + let payment_params = PaymentParameters::from_node_id(nodes[10], 42); let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100, 42, Arc::clone(&logger), &scorer, &random_seed_bytes); assert!(route.is_ok()); @@ -5745,7 +5767,7 @@ mod benches { let src = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); seed *= 0xdeadbeef; let dst = PublicKey::from_slice(nodes.unordered_keys().skip(seed % nodes.len()).next().unwrap().as_slice()).unwrap(); - let params = PaymentParameters::from_node_id(dst).with_features(features.clone()); + let params = PaymentParameters::from_node_id(dst, 42).with_features(features.clone()); let first_hop = first_hop(src); let amt = seed as u64 % 1_000_000; if let Ok(route) = get_route(&payer, ¶ms, &graph.read_only(), Some(&[&first_hop]), amt, 42, &DummyLogger{}, &scorer, &random_seed_bytes) { diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 1dae61ab3ef..f9a2eafb82d 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -77,7 +77,7 @@ impl chaininterface::FeeEstimator for TestFeeEstimator { pub struct TestRouter<'a> { pub network_graph: Arc>, - pub next_routes: Mutex>>, + pub next_routes: Mutex)>>, } impl<'a> TestRouter<'a> { @@ -85,9 +85,9 @@ impl<'a> TestRouter<'a> { Self { network_graph, next_routes: Mutex::new(VecDeque::new()), } } - pub fn expect_find_route(&self, result: Result) { + pub fn expect_find_route(&self, query: RouteParameters, result: Result) { let mut expected_routes = self.next_routes.lock().unwrap(); - expected_routes.push_back(result); + expected_routes.push_back((query, result)); } } @@ -96,8 +96,9 @@ impl<'a> Router for TestRouter<'a> { &self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&channelmanager::ChannelDetails]>, inflight_htlcs: &InFlightHtlcs ) -> Result { - if let Some(find_route_res) = self.next_routes.lock().unwrap().pop_front() { - return find_route_res + if let Some((find_route_query, find_route_res)) = self.next_routes.lock().unwrap().pop_front() { + assert_eq!(find_route_query, *params); + return find_route_res; } let logger = TestLogger::new(); find_route(