From ab255895eed9c4d9ed2dc400d7e0c720ea763ba8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Apr 2023 04:12:55 +0000 Subject: [PATCH 1/2] Allow claiming a payment if a channel with an HTLC has closed Previously, LDK would refuse to claim a payment if a channel on which the payment was received had been closed between when the HTLC was received and when we went to claim it. This makes sense in the payment case - why pay an on-chain fee to claim the HTLC when presumably the sender may retry later. Long ago it also reduced total code in the claim pipeline. However, this doesn't make sense if you're trying to do an atomic swap or some other protocol that requires atomicity with some other action - if your money got claimed elsewhere you need to be able to claim the HTLC in lightning no matter what. Further, this is an over-optimization - there should be a very, very low likelihood that a channel closes between when we receive the last HTLC for a payment and the user goes to claim the payment. Since we now have code to handle this anyway we should allow it. Fixes #2017. --- lightning/src/ln/channelmanager.rs | 41 +-------- lightning/src/ln/payment_tests.rs | 139 ++++++++++++++++++++++++++++- 2 files changed, 141 insertions(+), 39 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c2ff9da67f3..8b536c5dfc6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3981,21 +3981,10 @@ where }; debug_assert!(!sources.is_empty()); - // If we are claiming an MPP payment, we check that all channels which contain a claimable - // HTLC still exist. While this isn't guaranteed to remain true if a channel closes while - // we're claiming (or even after we claim, before the commitment update dance completes), - // it should be a relatively rare race, and we'd rather not claim HTLCs that require us to - // go on-chain (and lose the on-chain fee to do so) than just reject the payment. - // - // Note that we'll still always get our funds - as long as the generated - // `ChannelMonitorUpdate` makes it out to the relevant monitor we can claim on-chain. - // - // If we find an HTLC which we would need to claim but for which we do not have a - // channel, we will fail all parts of the MPP payment. While we could wait and see if - // the sender retries the already-failed path(s), it should be a pretty rare case where - // we got all the HTLCs and then a channel closed while we were waiting for the user to - // provide the preimage, so worrying too much about the optimal handling isn't worth - // it. + // Just in case one HTLC has been failed between when we generated the `PaymentClaimable` + // and when we got here we need to check that the amount we're about to claim matches the + // amount we told the user in the last `PaymentClaimable`. We also do a sanity-check that + // the MPP parts all have the same `total_msat`. let mut claimable_amt_msat = 0; let mut prev_total_msat = None; let mut expected_amt_msat = None; @@ -4003,28 +3992,6 @@ where let mut errs = Vec::new(); let per_peer_state = self.per_peer_state.read().unwrap(); for htlc in sources.iter() { - let (counterparty_node_id, chan_id) = match self.short_to_chan_info.read().unwrap().get(&htlc.prev_hop.short_channel_id) { - Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()), - None => { - valid_mpp = false; - break; - } - }; - - let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id); - if peer_state_mutex_opt.is_none() { - valid_mpp = false; - break; - } - - let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap(); - let peer_state = &mut *peer_state_lock; - - if peer_state.channel_by_id.get(&chan_id).is_none() { - valid_mpp = false; - break; - } - if prev_total_msat.is_some() && prev_total_msat != Some(htlc.total_msat) { log_error!(self.logger, "Somehow ended up with an MPP payment with different expected total amounts - this should not be reachable!"); debug_assert!(false); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 1ce0cc03458..fb50f5a4a38 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -12,7 +12,7 @@ //! payments thereafter. use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch}; -use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS}; +use crate::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::keysinterface::EntropySource; use crate::chain::transaction::OutPoint; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure}; @@ -23,7 +23,7 @@ use crate::ln::msgs; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::outbound_payment::Retry; use crate::routing::gossip::{EffectiveCapacity, RoutingFees}; -use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters}; +use crate::routing::router::{get_route, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters}; use crate::routing::scoring::ChannelUsage; use crate::util::test_utils; use crate::util::errors::APIError; @@ -2838,3 +2838,138 @@ fn no_missing_sent_on_midpoint_reload() { do_no_missing_sent_on_midpoint_reload(false); do_no_missing_sent_on_midpoint_reload(true); } + +fn do_claim_from_closed_chan(fail_payment: bool) { + // Previously, LDK would refuse to claim a payment if a channel on which the payment was + // received had been closed between when the HTLC was received and when we went to claim it. + // This makes sense in the payment case - why pay an on-chain fee to claim the HTLC when + // presumably the sender may retry later. Long ago it also reduced total code in the claim + // pipeline. + // + // However, this doesn't make sense if you're trying to do an atomic swap or some other + // protocol that requires atomicity with some other action - if your money got claimed + // elsewhere you need to be able to claim the HTLC in lightning no matter what. Further, this + // is an over-optimization - there should be a very, very low likelihood that a channel closes + // between when we receive the last HTLC for a payment and the user goes to claim the payment. + // Since we now have code to handle this anyway we should allow it. + + // Build 4 nodes and send an MPP payment across two paths. By building a route manually set the + // CLTVs on the paths to different value resulting in a different claim deadline. + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]); + let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes(&nodes, 0, 1); + create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 1_000_000, 0); + let chan_bd = create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 1_000_000, 0).2; + create_announced_chan_between_nodes(&nodes, 2, 3); + + let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[3]); + let mut route_params = RouteParameters { + payment_params: PaymentParameters::from_node_id(nodes[3].node.get_our_node_id(), TEST_FINAL_CLTV) + .with_features(nodes[1].node.invoice_features()), + final_value_msat: 10_000_000, + }; + let mut route = nodes[0].router.find_route(&nodes[0].node.get_our_node_id(), &route_params, + None, &nodes[0].node.compute_inflight_htlcs()).unwrap(); + // Make sure the route is ordered as the B->D path before C->D + route.paths.sort_by(|a, _| if a[0].pubkey == nodes[1].node.get_our_node_id() { + std::cmp::Ordering::Less } else { std::cmp::Ordering::Greater }); + + // Note that we add an extra 1 in the send pipeline to compensate for any blocks found while + // the HTLC is being relayed. + route.paths[0][1].cltv_expiry_delta = TEST_FINAL_CLTV + 8; + route.paths[1][1].cltv_expiry_delta = TEST_FINAL_CLTV + 12; + let final_cltv = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + 8 + 1; + + nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone())); + nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), + PaymentId(payment_hash.0), route_params.clone(), Retry::Attempts(1)).unwrap(); + check_added_monitors(&nodes[0], 2); + let mut send_msgs = nodes[0].node.get_and_clear_pending_msg_events(); + send_msgs.sort_by(|a, _| { + let a_node_id = + if let MessageSendEvent::UpdateHTLCs { node_id, .. } = a { node_id } else { panic!() }; + let node_b_id = nodes[1].node.get_our_node_id(); + if *a_node_id == node_b_id { std::cmp::Ordering::Less } else { std::cmp::Ordering::Greater } + }); + + assert_eq!(send_msgs.len(), 2); + pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 10_000_000, + payment_hash, Some(payment_secret), send_msgs.remove(0), false, None); + pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000, + payment_hash, Some(payment_secret), send_msgs.remove(0), true, None); + + // Ensure that the claim_deadline is correct, with the payment failing at exactly the given + // height. + connect_blocks(&nodes[3], final_cltv - HTLC_FAIL_BACK_BUFFER - nodes[3].best_block_info().1 + - if fail_payment { 0 } else { 2 }); + if fail_payment { + // We fail the HTLC on the A->B->D path first as it expires 4 blocks earlier. We go ahead + // and expire both immediately, though, by connecting another 4 blocks. + let reason = HTLCDestination::FailedPayment { payment_hash }; + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[3], [reason.clone()]); + connect_blocks(&nodes[3], 4); + expect_pending_htlcs_forwardable_and_htlc_handling_failed!(&nodes[3], [reason]); + pass_failed_payment_back(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_hash); + } else { + nodes[1].node.force_close_broadcasting_latest_txn(&chan_bd, &nodes[3].node.get_our_node_id()).unwrap(); + check_closed_event(&nodes[1], 1, ClosureReason::HolderForceClosed, false); + check_closed_broadcast(&nodes[1], 1, true); + let bs_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); + assert_eq!(bs_tx.len(), 1); + + mine_transaction(&nodes[3], &bs_tx[0]); + check_added_monitors(&nodes[3], 1); + check_closed_broadcast(&nodes[3], 1, true); + check_closed_event(&nodes[3], 1, ClosureReason::CommitmentTxConfirmed, false); + + nodes[3].node.claim_funds(payment_preimage); + check_added_monitors(&nodes[3], 2); + expect_payment_claimed!(nodes[3], payment_hash, 10_000_000); + + let ds_tx = nodes[3].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); + assert_eq!(ds_tx.len(), 1); + check_spends!(&ds_tx[0], &bs_tx[0]); + + mine_transactions(&nodes[1], &[&bs_tx[0], &ds_tx[0]]); + check_added_monitors(&nodes[1], 1); + expect_payment_forwarded!(nodes[1], nodes[0], nodes[3], Some(1000), false, true); + + let bs_claims = nodes[1].node.get_and_clear_pending_msg_events(); + check_added_monitors(&nodes[1], 1); + assert_eq!(bs_claims.len(), 1); + if let MessageSendEvent::UpdateHTLCs { updates, .. } = &bs_claims[0] { + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false, true); + } else { panic!(); } + + expect_payment_sent!(nodes[0], payment_preimage); + + let ds_claim_msgs = nodes[3].node.get_and_clear_pending_msg_events(); + assert_eq!(ds_claim_msgs.len(), 1); + let cs_claim_msgs = if let MessageSendEvent::UpdateHTLCs { updates, .. } = &ds_claim_msgs[0] { + nodes[2].node.handle_update_fulfill_htlc(&nodes[3].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); + let cs_claim_msgs = nodes[2].node.get_and_clear_pending_msg_events(); + check_added_monitors(&nodes[2], 1); + commitment_signed_dance!(nodes[2], nodes[3], updates.commitment_signed, false, true); + expect_payment_forwarded!(nodes[2], nodes[0], nodes[3], Some(1000), false, false); + cs_claim_msgs + } else { panic!(); }; + + assert_eq!(cs_claim_msgs.len(), 1); + if let MessageSendEvent::UpdateHTLCs { updates, .. } = &cs_claim_msgs[0] { + nodes[0].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[2], updates.commitment_signed, false, true); + } else { panic!(); } + + expect_payment_path_successful!(nodes[0]); + } +} + +#[test] +fn claim_from_closed_chan() { + do_claim_from_closed_chan(true); + do_claim_from_closed_chan(false); +} From 4a8d01dd195cf1b0c4a53ab609700e24f3a9f2a3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 4 Apr 2023 04:21:45 +0000 Subject: [PATCH 2/2] Add a `claim_deadline` field to `PaymentClaimable` with guarantees Now that we guarantee `claim_payment` will always succeed we have to let the user know what the deadline is. We still fail payments if they haven't been claimed in time, which we now expose in `PaymentClaimable`. --- lightning/src/events/mod.rs | 18 ++++++++++++++++-- lightning/src/ln/chanmon_update_fail_tests.rs | 10 +++++----- lightning/src/ln/channelmanager.rs | 13 ++++++++++--- lightning/src/ln/functional_test_utils.rs | 14 +++++++++----- lightning/src/ln/functional_tests.rs | 6 +++--- lightning/src/ln/payment_tests.rs | 9 ++++++++- 6 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 2d576d33a05..318cc2a6d1a 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -321,7 +321,9 @@ pub enum Event { /// /// # Note /// LDK will not stop an inbound payment from being paid multiple times, so multiple - /// `PaymentClaimable` events may be generated for the same payment. + /// `PaymentClaimable` events may be generated for the same payment. In such a case it is + /// polite (and required in the lightning specification) to fail the payment the second time + /// and give the sender their money back rather than accepting double payment. /// /// # Note /// This event used to be called `PaymentReceived` in LDK versions 0.0.112 and earlier. @@ -349,6 +351,14 @@ pub enum Event { via_channel_id: Option<[u8; 32]>, /// The `user_channel_id` indicating over which channel we received the payment. via_user_channel_id: Option, + /// The block height at which this payment will be failed back and will no longer be + /// eligible for claiming. + /// + /// Prior to this height, a call to [`ChannelManager::claim_funds`] is guaranteed to + /// succeed, however you should wait for [`Event::PaymentClaimed`] to be sure. + /// + /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds + claim_deadline: Option, }, /// Indicates a payment has been claimed and we've received money! /// @@ -773,7 +783,7 @@ impl Writeable for Event { // We never write out FundingGenerationReady events as, upon disconnection, peers // drop any channels which have not yet exchanged funding_signed. }, - &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id } => { + &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, ref claim_deadline } => { 1u8.write(writer)?; let mut payment_secret = None; let payment_preimage; @@ -794,6 +804,7 @@ impl Writeable for Event { (4, amount_msat, required), (5, via_user_channel_id, option), (6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier + (7, claim_deadline, option), (8, payment_preimage, option), }); }, @@ -992,6 +1003,7 @@ impl MaybeReadable for Event { let mut receiver_node_id = None; let mut _user_payment_id = None::; // For compatibility with 0.0.103 and earlier let mut via_channel_id = None; + let mut claim_deadline = None; let mut via_user_channel_id = None; read_tlv_fields!(reader, { (0, payment_hash, required), @@ -1001,6 +1013,7 @@ impl MaybeReadable for Event { (4, amount_msat, required), (5, via_user_channel_id, option), (6, _user_payment_id, option), + (7, claim_deadline, option), (8, payment_preimage, option), }); let purpose = match payment_secret { @@ -1018,6 +1031,7 @@ impl MaybeReadable for Event { purpose, via_channel_id, via_user_channel_id, + claim_deadline, })) }; f() diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 94df7371a27..d12fe5c3a6b 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -204,7 +204,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { let events_3 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_3.len(), 1); match events_3[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(payment_hash_1, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); @@ -573,7 +573,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { let events_5 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_5.len(), 1); match events_5[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(payment_hash_2, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); @@ -690,7 +690,7 @@ fn test_monitor_update_fail_cs() { let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(payment_hash, our_payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); @@ -1668,7 +1668,7 @@ fn test_monitor_update_fail_claim() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match events[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id, .. } => { assert_eq!(payment_hash_2, *payment_hash); assert_eq!(1_000_000, amount_msat); assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id()); @@ -1685,7 +1685,7 @@ fn test_monitor_update_fail_claim() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(payment_hash_3, *payment_hash); assert_eq!(1_000_000, amount_msat); assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id()); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8b536c5dfc6..241235fd20a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3363,8 +3363,10 @@ where } } let mut total_value = claimable_htlc.sender_intended_value; + let mut earliest_expiry = claimable_htlc.cltv_expiry; for htlc in htlcs.iter() { total_value += htlc.sender_intended_value; + earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry); match &htlc.onion_payload { OnionPayload::Invoice { .. } => { if htlc.total_msat != $payment_data.total_msat { @@ -3397,6 +3399,7 @@ where amount_msat, via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), + claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER), }); payment_claimable_generated = true; } else { @@ -3450,6 +3453,7 @@ where hash_map::Entry::Vacant(e) => { let amount_msat = claimable_htlc.value; claimable_htlc.total_value_received = Some(amount_msat); + let claim_deadline = Some(claimable_htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER); let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); e.insert((purpose.clone(), vec![claimable_htlc])); let prev_channel_id = prev_funding_outpoint.to_channel_id(); @@ -3460,6 +3464,7 @@ where purpose, via_channel_id: Some(prev_channel_id), via_user_channel_id: Some(prev_user_channel_id), + claim_deadline, }); }, hash_map::Entry::Occupied(_) => { @@ -3935,9 +3940,10 @@ where /// Provides a payment preimage in response to [`Event::PaymentClaimable`], generating any /// [`MessageSendEvent`]s needed to claim the payment. /// - /// Note that calling this method does *not* guarantee that the payment has been claimed. You - /// *must* wait for an [`Event::PaymentClaimed`] event which upon a successful claim will be - /// provided to your [`EventHandler`] when [`process_pending_events`] is next called. + /// This method is guaranteed to ensure the payment has been claimed but only if the current + /// height is strictly below [`Event::PaymentClaimable::claim_deadline`]. To avoid race + /// conditions, you should wait for an [`Event::PaymentClaimed`] before considering the payment + /// successful. It will generally be available in the next [`process_pending_events`] call. /// /// Note that if you did not set an `amount_msat` when calling [`create_inbound_payment`] or /// [`create_inbound_payment_for_hash`] you must check that the amount in the `PaymentClaimable` @@ -3945,6 +3951,7 @@ where /// the sender "proof-of-payment" when they did not fulfill the full expected payment. /// /// [`Event::PaymentClaimable`]: crate::events::Event::PaymentClaimable + /// [`Event::PaymentClaimable::claim_deadline`]: crate::events::Event::PaymentClaimable::claim_deadline /// [`Event::PaymentClaimed`]: crate::events::Event::PaymentClaimed /// [`process_pending_events`]: EventsProvider::process_pending_events /// [`create_inbound_payment`]: Self::create_inbound_payment diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d7fd9c8e993..2d52135da09 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1680,7 +1680,7 @@ macro_rules! expect_payment_claimable { let events = $node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - $crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id: _, via_user_channel_id: _ } => { + $crate::events::Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, .. } => { assert_eq!($expected_payment_hash, *payment_hash); assert_eq!($expected_recv_value, amount_msat); assert_eq!($expected_receiver_node_id, receiver_node_id.unwrap()); @@ -1962,9 +1962,10 @@ pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, payment_id } -pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option, ev: MessageSendEvent, payment_claimable_expected: bool, clear_recipient_events: bool, expected_preimage: Option) { +pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option, ev: MessageSendEvent, payment_claimable_expected: bool, clear_recipient_events: bool, expected_preimage: Option) -> Option { let mut payment_event = SendEvent::from_event(ev); let mut prev_node = origin_node; + let mut event = None; for (idx, &node) in expected_path.iter().enumerate() { assert_eq!(node.node.get_our_node_id(), payment_event.node_id); @@ -1980,7 +1981,7 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p if payment_claimable_expected { assert_eq!(events_2.len(), 1); match events_2[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, ref via_channel_id, ref via_user_channel_id, claim_deadline } => { assert_eq!(our_payment_hash, *payment_hash); assert_eq!(node.node.get_our_node_id(), receiver_node_id.unwrap()); match &purpose { @@ -1996,9 +1997,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p assert_eq!(amount_msat, recv_value); assert!(node.node.list_channels().iter().any(|details| details.channel_id == via_channel_id.unwrap())); assert!(node.node.list_channels().iter().any(|details| details.user_channel_id == via_user_channel_id.unwrap())); + assert!(claim_deadline.unwrap() > node.best_block_info().1); }, _ => panic!("Unexpected event"), } + event = Some(events_2[0].clone()); } else { assert!(events_2.is_empty()); } @@ -2012,10 +2015,11 @@ pub fn do_pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_p prev_node = node; } + event } -pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option, ev: MessageSendEvent, payment_claimable_expected: bool, expected_preimage: Option) { - do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage); +pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option, ev: MessageSendEvent, payment_claimable_expected: bool, expected_preimage: Option) -> Option { + do_pass_along_path(origin_node, expected_path, recv_value, our_payment_hash, our_payment_secret, ev, payment_claimable_expected, true, expected_preimage) } pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4f73627f4b4..9c0a4b4b066 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1966,7 +1966,7 @@ fn test_channel_reserve_holding_cell_htlcs() { let events = nodes[2].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); match events[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(our_payment_hash_21, *payment_hash); assert_eq!(recv_value_21, amount_msat); assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap()); @@ -1982,7 +1982,7 @@ fn test_channel_reserve_holding_cell_htlcs() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(our_payment_hash_22, *payment_hash); assert_eq!(recv_value_22, amount_msat); assert_eq!(nodes[2].node.get_our_node_id(), receiver_node_id.unwrap()); @@ -3764,7 +3764,7 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken let events_2 = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events_2.len(), 1); match events_2[0] { - Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, via_user_channel_id: _ } => { + Event::PaymentClaimable { ref payment_hash, ref purpose, amount_msat, receiver_node_id, via_channel_id, .. } => { assert_eq!(payment_hash_1, *payment_hash); assert_eq!(amount_msat, 1_000_000); assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index fb50f5a4a38..028af20ee60 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -2898,9 +2898,16 @@ fn do_claim_from_closed_chan(fail_payment: bool) { assert_eq!(send_msgs.len(), 2); pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], 10_000_000, payment_hash, Some(payment_secret), send_msgs.remove(0), false, None); - pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000, + let receive_event = pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 10_000_000, payment_hash, Some(payment_secret), send_msgs.remove(0), true, None); + match receive_event.unwrap() { + Event::PaymentClaimable { claim_deadline, .. } => { + assert_eq!(claim_deadline.unwrap(), final_cltv - HTLC_FAIL_BACK_BUFFER); + }, + _ => panic!(), + } + // Ensure that the claim_deadline is correct, with the payment failing at exactly the given // height. connect_blocks(&nodes[3], final_cltv - HTLC_FAIL_BACK_BUFFER - nodes[3].best_block_info().1