From 2e562a2ad88fe214e19e2ee1c5f4db743d9ebe07 Mon Sep 17 00:00:00 2001 From: Chris Waterson Date: Sun, 6 Aug 2023 15:39:21 -0700 Subject: [PATCH] Provide the HTLCs that settled a payment. Creates a new `events::ClaimedHTLC` struct that contains the relevant information about a claimed HTLC; e.g., the channel it arrived on, its ID, the amount of the HTLC, the overall amount of the payment, etc. Adds appropriate serialization support. Adds a `Vec` to the `ClaimingPayment` structure. Populates this when creating the struct by converting the `payment.htlcs` (which are `ClaimingHTLC` structs) into `event::ClaimedHTLC` structs. This is a straightforward transformation. Adds a `Vec` to the `events::Event::PaymentClaimed` enum. This is populated directly from the `ClaimingPayment`'s `htlcs` vec. Fixes #2477. --- lightning/src/events/mod.rs | 47 +++++++++++++++++++- lightning/src/ln/channelmanager.rs | 53 ++++++++++++++++++++--- lightning/src/ln/functional_test_utils.rs | 50 ++++++++++++++++++--- 3 files changed, 136 insertions(+), 14 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 418f8d545..77bf2f265 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -79,6 +79,37 @@ impl_writeable_tlv_based_enum!(PaymentPurpose, (2, SpontaneousPayment) ); +/// Information about an HTLC that is part of a payment that can be claimed. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ClaimedHTLC { + /// The `channel_id` of the channel over which the HTLC was received. + pub channel_id: [u8; 32], + /// The `user_channel_id` of the channel over which the HTLC was received. This is the value + /// passed in to [`ChannelManager::create_channel`] for outbound channels, or to + /// [`ChannelManager::accept_inbound_channel`] for inbound channels if + /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise + /// `user_channel_id` will be randomized for an inbound channel. + /// + /// This field will be zero for a payment that was serialized prior to LDK version 0.0.117. (This + /// should only happen in the case that a payment was claimable prior to LDK version 0.0.117, but + /// was not actually claimed until after upgrading.) + /// + /// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels + pub user_channel_id: u128, + /// The block height at which this HTLC expires. + pub cltv_expiry: u32, + /// The amount (in msats) of this part of an MPP. + pub value_msat: u64, +} +impl_writeable_tlv_based!(ClaimedHTLC, { + (0, channel_id, required), + (2, user_channel_id, required), + (4, cltv_expiry, required), + (6, value_msat, required), +}); + /// When the payment path failure took place and extra details about it. [`PathFailure::OnPath`] may /// contain a [`NetworkUpdate`] that needs to be applied to the [`NetworkGraph`]. /// @@ -470,6 +501,12 @@ pub enum Event { /// The purpose of the claimed payment, i.e. whether the payment was for an invoice or a /// spontaneous payment. purpose: PaymentPurpose, + /// The HTLCs that comprise the claimed payment. This will be empty for events serialized prior + /// to LDK version 0.0.117. + htlcs: Vec, + /// The sender-intended sum total of all the MPP parts. This will be `None` for events + /// serialized prior to LDK version 0.0.117. + sender_intended_total_msat: Option, }, /// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target /// and we got back the payment preimage for it). @@ -1035,13 +1072,15 @@ impl Writeable for Event { // We never write the OpenChannelRequest events as, upon disconnection, peers // drop any channels which have not yet exchanged funding_signed. }, - &Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id } => { + &Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat } => { 19u8.write(writer)?; write_tlv_fields!(writer, { (0, payment_hash, required), (1, receiver_node_id, option), (2, purpose, required), (4, amount_msat, required), + (5, *htlcs, optional_vec), + (7, sender_intended_total_msat, option), }); }, &Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => { @@ -1366,17 +1405,23 @@ impl MaybeReadable for Event { let mut purpose = UpgradableRequired(None); let mut amount_msat = 0; let mut receiver_node_id = None; + let mut htlcs: Option> = Some(vec![]); + let mut sender_intended_total_msat: Option = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, receiver_node_id, option), (2, purpose, upgradable_required), (4, amount_msat, required), + (5, htlcs, optional_vec), + (7, sender_intended_total_msat, option), }); Ok(Some(Event::PaymentClaimed { receiver_node_id, payment_hash, purpose: _init_tlv_based_struct_field!(purpose, upgradable_required), amount_msat, + htlcs: htlcs.unwrap_or(vec![]), + sender_intended_total_msat, })) }; f() diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 45fffd540..5ad5549dc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -181,6 +181,7 @@ pub(super) enum HTLCForwardInfo { pub(crate) struct HTLCPreviousHopData { // Note that this may be an outbound SCID alias for the associated channel. short_channel_id: u64, + user_channel_id: Option, htlc_id: u64, incoming_packet_shared_secret: [u8; 32], phantom_shared_secret: Option<[u8; 32]>, @@ -221,6 +222,17 @@ struct ClaimableHTLC { counterparty_skimmed_fee_msat: Option, } +impl From<&ClaimableHTLC> for events::ClaimedHTLC { + fn from(val: &ClaimableHTLC) -> Self { + events::ClaimedHTLC { + channel_id: val.prev_hop.outpoint.to_channel_id(), + user_channel_id: val.prev_hop.user_channel_id.unwrap_or(0), + cltv_expiry: val.cltv_expiry, + value_msat: val.value, + } + } +} + /// A payment identifier used to uniquely identify a payment to LDK. /// /// This is not exported to bindings users as we just use [u8; 32] directly @@ -496,11 +508,15 @@ struct ClaimingPayment { amount_msat: u64, payment_purpose: events::PaymentPurpose, receiver_node_id: PublicKey, + htlcs: Vec, + sender_intended_value: Option, } impl_writeable_tlv_based!(ClaimingPayment, { (0, amount_msat, required), (2, payment_purpose, required), (4, receiver_node_id, required), + (5, htlcs, optional_vec), + (7, sender_intended_value, option), }); struct ClaimablePayment { @@ -3781,6 +3797,7 @@ where if let PendingHTLCRouting::Forward { short_channel_id, .. } = payment.forward_info.routing { let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: payment.prev_short_channel_id, + user_channel_id: Some(payment.prev_user_channel_id), outpoint: payment.prev_funding_outpoint, htlc_id: payment.prev_htlc_id, incoming_packet_shared_secret: payment.forward_info.incoming_shared_secret, @@ -3828,6 +3845,7 @@ where let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: prev_short_channel_id, + user_channel_id: Some(prev_user_channel_id), outpoint: prev_funding_outpoint, htlc_id: prev_htlc_id, incoming_packet_shared_secret: incoming_shared_secret, @@ -3932,7 +3950,7 @@ where for forward_info in pending_forwards.drain(..) { match forward_info { HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { - prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id: _, + prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id, forward_info: PendingHTLCInfo { incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value, routing: PendingHTLCRouting::Forward { onion_packet, .. }, skimmed_fee_msat, .. @@ -3941,6 +3959,7 @@ where log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id); let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: prev_short_channel_id, + user_channel_id: Some(prev_user_channel_id), outpoint: prev_funding_outpoint, htlc_id: prev_htlc_id, incoming_packet_shared_secret: incoming_shared_secret, @@ -4022,6 +4041,7 @@ where let claimable_htlc = ClaimableHTLC { prev_hop: HTLCPreviousHopData { short_channel_id: prev_short_channel_id, + user_channel_id: Some(prev_user_channel_id), outpoint: prev_funding_outpoint, htlc_id: prev_htlc_id, incoming_packet_shared_secret: incoming_shared_secret, @@ -4051,6 +4071,7 @@ where ); failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: $htlc.prev_hop.short_channel_id, + user_channel_id: $htlc.prev_hop.user_channel_id, outpoint: prev_funding_outpoint, htlc_id: $htlc.prev_hop.htlc_id, incoming_packet_shared_secret: $htlc.prev_hop.incoming_packet_shared_secret, @@ -4782,7 +4803,7 @@ where &self.pending_events, &self.logger) { self.push_pending_forwards_ev(); } }, - HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint }) => { + HTLCSource::PreviousHopData(HTLCPreviousHopData { ref short_channel_id, ref htlc_id, ref incoming_packet_shared_secret, ref phantom_shared_secret, ref outpoint, .. }) => { log_trace!(self.logger, "Failing HTLC with payment_hash {} backwards from us with {:?}", log_bytes!(payment_hash.0), onion_error); let err_packet = onion_error.get_encrypted_failure_packet(incoming_packet_shared_secret, phantom_shared_secret); @@ -4869,9 +4890,11 @@ where } } + let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(); + let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat); let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash, ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(), - payment_purpose: payment.purpose, receiver_node_id, + payment_purpose: payment.purpose, receiver_node_id, htlcs, sender_intended_value }); if dup_purpose.is_some() { debug_assert!(false, "Shouldn't get a duplicate pending claim event ever"); @@ -5129,9 +5152,20 @@ where match action { MonitorUpdateCompletionAction::PaymentClaimed { payment_hash } => { let payment = self.claimable_payments.lock().unwrap().pending_claiming_payments.remove(&payment_hash); - if let Some(ClaimingPayment { amount_msat, payment_purpose: purpose, receiver_node_id }) = payment { + if let Some(ClaimingPayment { + amount_msat, + payment_purpose: purpose, + receiver_node_id, + htlcs, + sender_intended_value: sender_intended_total_msat, + }) = payment { self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed { - payment_hash, purpose, amount_msat, receiver_node_id: Some(receiver_node_id), + payment_hash, + purpose, + amount_msat, + receiver_node_id: Some(receiver_node_id), + htlcs, + sender_intended_total_msat, }, None)); } }, @@ -6006,6 +6040,7 @@ where log_info!(self.logger, "Failed to forward incoming HTLC: detected duplicate intercepted payment over short channel id {}", scid); let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: prev_short_channel_id, + user_channel_id: Some(prev_user_channel_id), outpoint: prev_funding_outpoint, htlc_id: prev_htlc_id, incoming_packet_shared_secret: forward_info.incoming_shared_secret, @@ -7124,6 +7159,7 @@ where if height >= htlc.forward_info.outgoing_cltv_value - HTLC_FAIL_BACK_BUFFER { let prev_hop_data = HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: htlc.prev_short_channel_id, + user_channel_id: Some(htlc.prev_user_channel_id), htlc_id: htlc.prev_htlc_id, incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret, phantom_shared_secret: None, @@ -7899,7 +7935,8 @@ impl_writeable_tlv_based!(HTLCPreviousHopData, { (1, phantom_shared_secret, option), (2, outpoint, required), (4, htlc_id, required), - (6, incoming_packet_shared_secret, required) + (6, incoming_packet_shared_secret, required), + (7, user_channel_id, option), }); impl Writeable for ClaimableHTLC { @@ -9147,7 +9184,7 @@ where .expect("Failed to get node_id for phantom node recipient"); receiver_node_id = Some(phantom_pubkey) } - for claimable_htlc in payment.htlcs { + for claimable_htlc in &payment.htlcs { claimable_amt_msat += claimable_htlc.value; // Add a holding-cell claim of the payment to the Channel, which should be @@ -9183,6 +9220,8 @@ where payment_hash, purpose: payment.purpose, amount_msat: claimable_amt_msat, + htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(), + sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat), }, None)); } } diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index b5ac22d47..73d09c76f 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -14,7 +14,7 @@ use crate::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen, Watch use crate::sign::EntropySource; use crate::chain::channelmonitor::ChannelMonitor; use crate::chain::transaction::OutPoint; -use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason}; +use crate::events::{ClaimedHTLC, ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, PaymentFailureReason}; use crate::events::bump_transaction::{BumpTransactionEventHandler, Wallet, WalletSource}; use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use crate::ln::channelmanager::{AChannelManager, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, PaymentId, MIN_CLTV_EXPIRY_DELTA}; @@ -933,6 +933,21 @@ macro_rules! check_added_monitors { } } +/// Checks whether the claimed HTLC for the specified path has the correct channel information. +/// +/// This will panic if the path is empty, if the HTLC's channel ID is not actually a channel that +/// connects the final two nodes in the path, or if the `user_channel_id` is incorrect. +pub fn check_claimed_htlc_channel<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, path: &[&Node<'a, 'b, 'c>], htlc: &ClaimedHTLC) { + let mut nodes = path.iter().rev(); + let dest = nodes.next().expect("path should have a destination").node; + let prev = nodes.next().unwrap_or(&origin_node).node; + let dest_channels = dest.list_channels(); + let ch = dest_channels.iter().find(|ch| ch.channel_id == htlc.channel_id) + .expect("HTLC's channel should be one of destination node's channels"); + assert_eq!(htlc.user_channel_id, ch.user_channel_id); + assert_eq!(ch.counterparty.node_id, prev.get_our_node_id()); +} + pub fn _reload_node<'a, 'b, 'c>(node: &'a Node<'a, 'b, 'c>, default_config: UserConfig, chanman_encoded: &[u8], monitors_encoded: &[&[u8]]) -> TestChannelManager<'b, 'c> { let mut monitors_read = Vec::with_capacity(monitors_encoded.len()); for encoded in monitors_encoded { @@ -2271,11 +2286,34 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, ' let claim_event = expected_paths[0].last().unwrap().node.get_and_clear_pending_events(); assert_eq!(claim_event.len(), 1); match claim_event[0] { - Event::PaymentClaimed { purpose: PaymentPurpose::SpontaneousPayment(preimage), .. }| - Event::PaymentClaimed { purpose: PaymentPurpose::InvoicePayment { payment_preimage: Some(preimage), ..}, .. } => - assert_eq!(preimage, our_payment_preimage), - Event::PaymentClaimed { purpose: PaymentPurpose::InvoicePayment { .. }, payment_hash, .. } => - assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]), + Event::PaymentClaimed { + purpose: PaymentPurpose::SpontaneousPayment(preimage), + amount_msat, + ref htlcs, + .. } + | Event::PaymentClaimed { + purpose: PaymentPurpose::InvoicePayment { payment_preimage: Some(preimage), ..}, + ref htlcs, + amount_msat, + .. + } => { + assert_eq!(preimage, our_payment_preimage); + assert_eq!(htlcs.len(), expected_paths.len()); // One per path. + assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::(), amount_msat); + expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc)); + }, + Event::PaymentClaimed { + purpose: PaymentPurpose::InvoicePayment { .. }, + payment_hash, + amount_msat, + ref htlcs, + .. + } => { + assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]); + assert_eq!(htlcs.len(), expected_paths.len()); // One per path. + assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::(), amount_msat); + expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc)); + } _ => panic!(), } -- 2.39.5