From aaf529d19366bde82c0bee632dddfd8e38692caa Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 12 Sep 2024 18:36:58 +0000 Subject: [PATCH] Add a `PaymentId` for inbound payments We expect our users to have fully idempotent `Event` handling as we may replay events on restart for one of a number of reasons. This isn't a big deal as long as all our events have some kind of identifier users can use to check if the `Event` has already been handled. For outbound payments, this is the `PaymentId` they provide in the send methods, however for inbound payments we don't have a great option. `PaymentHash` largely suffices - users can simply always claim in response to a `PaymentClaimable` of sufficient value and treat a `PaymentClaimed` event as duplicate any time they see a second one for the same `PaymentHash`. This mostly works, but may result in accepting duplicative payments if someone (incorrectly) pays twice for the same `PaymentHash`. Users could also fail for duplicative `PaymentClaimable` events of the same `PaymentHash`, but doing so may result in spuriously failing a payment if the `PaymentClaimable` event is a replay and they never saw a corresponding `PaymentClaimed` event. While none of this will result in spuriously thinking they've been paid when they have not, it does result in some pretty awkward semantics which we'd rather avoid our users having to deal with. Instead, here, we add a new `PaymentId` which is simply an HMAC of the HTLCs (as Channel ID, HTLC ID pairs) which were included in the payment. --- lightning/src/events/mod.rs | 31 ++++++++++++- lightning/src/ln/channelmanager.rs | 73 ++++++++++++++++++++++++++---- lightning/src/ln/msgs.rs | 3 +- 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index fe52d08c9..379a0d79a 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -749,6 +749,14 @@ pub enum Event { /// /// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds claim_deadline: Option, + /// A unique ID describing this payment (derived from the list of HTLCs in the payment). + /// + /// Payers may pay for the same [`PaymentHash`] multiple times (though this is unsafe and + /// an intermediary node may steal the funds). Thus, in order to accurately track when + /// payments are received and claimed, you should use this identifier. + /// + /// Only filled in for payments received on LDK versions 0.1 and higher. + payment_id: Option, }, /// Indicates a payment has been claimed and we've received money! /// @@ -796,6 +804,14 @@ pub enum Event { /// /// Payments received on LDK versions prior to 0.0.124 will have this field unset. onion_fields: Option, + /// A unique ID describing this payment (derived from the list of HTLCs in the payment). + /// + /// Payers may pay for the same [`PaymentHash`] multiple times (though this is unsafe and + /// an intermediary node may steal the funds). Thus, in order to accurately track when + /// payments are received and claimed, you should use this identifier. + /// + /// Only filled in for payments received on LDK versions 0.1 and higher. + payment_id: Option, }, /// Indicates that a peer connection with a node is needed in order to send an [`OnionMessage`]. /// @@ -1432,7 +1448,7 @@ impl Writeable for Event { }, &Event::PaymentClaimable { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat, ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id, - ref claim_deadline, ref onion_fields + ref claim_deadline, ref onion_fields, ref payment_id, } => { 1u8.write(writer)?; let mut payment_secret = None; @@ -1478,6 +1494,7 @@ impl Writeable for Event { (9, onion_fields, option), (10, skimmed_fee_opt, option), (11, payment_context, option), + (13, payment_id, option), }); }, &Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => { @@ -1634,7 +1651,10 @@ 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, ref htlcs, ref sender_intended_total_msat, ref onion_fields } => { + &Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, + ref receiver_node_id, ref htlcs, ref sender_intended_total_msat, ref onion_fields, + ref payment_id, + } => { 19u8.write(writer)?; write_tlv_fields!(writer, { (0, payment_hash, required), @@ -1644,6 +1664,7 @@ impl Writeable for Event { (5, *htlcs, optional_vec), (7, sender_intended_total_msat, option), (9, onion_fields, option), + (11, payment_id, option), }); }, &Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => { @@ -1767,6 +1788,7 @@ impl MaybeReadable for Event { let mut via_user_channel_id = None; let mut onion_fields = None; let mut payment_context = None; + let mut payment_id = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, receiver_node_id, option), @@ -1780,6 +1802,7 @@ impl MaybeReadable for Event { (9, onion_fields, option), (10, counterparty_skimmed_fee_msat_opt, option), (11, payment_context, option), + (13, payment_id, option), }); let purpose = match payment_secret { Some(secret) => PaymentPurpose::from_parts(payment_preimage, secret, payment_context), @@ -1796,6 +1819,7 @@ impl MaybeReadable for Event { via_user_channel_id, claim_deadline, onion_fields, + payment_id, })) }; f() @@ -2037,6 +2061,7 @@ impl MaybeReadable for Event { let mut htlcs: Option> = Some(vec![]); let mut sender_intended_total_msat: Option = None; let mut onion_fields = None; + let mut payment_id = None; read_tlv_fields!(reader, { (0, payment_hash, required), (1, receiver_node_id, option), @@ -2045,6 +2070,7 @@ impl MaybeReadable for Event { (5, htlcs, optional_vec), (7, sender_intended_total_msat, option), (9, onion_fields, option), + (11, payment_id, option), }); Ok(Some(Event::PaymentClaimed { receiver_node_id, @@ -2054,6 +2080,7 @@ impl MaybeReadable for Event { htlcs: htlcs.unwrap_or_default(), sender_intended_total_msat, onion_fields, + payment_id, })) }; f() diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0f9bfd889..241b02e09 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -23,7 +23,7 @@ use bitcoin::constants::ChainHash; use bitcoin::key::constants::SECRET_KEY_SIZE; use bitcoin::network::Network; -use bitcoin::hashes::Hash; +use bitcoin::hashes::{Hash, HashEngine, HmacEngine}; use bitcoin::hashes::hmac::Hmac; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hash_types::{BlockHash, Txid}; @@ -368,6 +368,7 @@ pub(crate) struct HTLCPreviousHopData { counterparty_node_id: Option, } +#[derive(PartialEq, Eq)] enum OnionPayload { /// Indicates this incoming onion payload is for the purpose of paying an invoice. Invoice { @@ -380,6 +381,7 @@ enum OnionPayload { } /// HTLCs that are to us and can be failed/claimed by the user +#[derive(PartialEq, Eq)] struct ClaimableHTLC { prev_hop: HTLCPreviousHopData, cltv_expiry: u32, @@ -411,6 +413,23 @@ impl From<&ClaimableHTLC> for events::ClaimedHTLC { } } +impl PartialOrd for ClaimableHTLC { + fn partial_cmp(&self, other: &ClaimableHTLC) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for ClaimableHTLC { + fn cmp(&self, other: &ClaimableHTLC) -> cmp::Ordering { + let res = (self.prev_hop.channel_id, self.prev_hop.htlc_id).cmp( + &(other.prev_hop.channel_id, other.prev_hop.htlc_id) + ); + if res.is_eq() { + debug_assert!(self == other, "ClaimableHTLCs from the same source should be identical"); + } + res + } +} + /// A trait defining behavior for creating and verifing the HMAC for authenticating a given data. pub trait Verification { /// Constructs an HMAC to include in [`OffersContext`] for the data along with the given @@ -491,6 +510,22 @@ impl Verification for PaymentId { } } +impl PaymentId { + fn for_inbound_from_htlcs>(key: &[u8; 32], htlcs: I) -> PaymentId { + let mut prev_pair = None; + let mut hasher = HmacEngine::new(key); + for (channel_id, htlc_id) in htlcs { + hasher.input(&channel_id.0); + hasher.input(&htlc_id.to_le_bytes()); + if let Some(prev) = prev_pair { + debug_assert!(prev < (channel_id, htlc_id), "HTLCs should be sorted"); + } + prev_pair = Some((channel_id, htlc_id)); + } + PaymentId(Hmac::::from_engine(hasher).to_byte_array()) + } +} + impl Writeable for PaymentId { fn write(&self, w: &mut W) -> Result<(), io::Error> { self.0.write(w) @@ -764,6 +799,7 @@ struct ClaimingPayment { htlcs: Vec, sender_intended_value: Option, onion_fields: Option, + payment_id: Option, } impl_writeable_tlv_based!(ClaimingPayment, { (0, amount_msat, required), @@ -772,6 +808,7 @@ impl_writeable_tlv_based!(ClaimingPayment, { (5, htlcs, optional_vec), (7, sender_intended_value, option), (9, onion_fields, option), + (11, payment_id, option), }); struct ClaimablePayment { @@ -780,6 +817,15 @@ struct ClaimablePayment { htlcs: Vec, } +impl ClaimablePayment { + fn inbound_payment_id(&self, secret: &[u8; 32]) -> PaymentId { + PaymentId::for_inbound_from_htlcs( + secret, + self.htlcs.iter().map(|htlc| (htlc.prev_hop.channel_id, htlc.prev_hop.htlc_id)) + ) + } +} + /// Represent the channel funding transaction type. enum FundingType { /// This variant is useful when we want LDK to validate the funding transaction and @@ -5749,10 +5795,9 @@ where } else { claimable_payment.onion_fields = Some(onion_fields); } - let ref mut htlcs = &mut claimable_payment.htlcs; let mut total_value = claimable_htlc.sender_intended_value; let mut earliest_expiry = claimable_htlc.cltv_expiry; - for htlc in htlcs.iter() { + for htlc in claimable_payment.htlcs.iter() { total_value += htlc.sender_intended_value; earliest_expiry = cmp::min(earliest_expiry, htlc.cltv_expiry); if htlc.total_msat != claimable_htlc.total_msat { @@ -5774,13 +5819,18 @@ where #[allow(unused_assignments)] { committed_to_claimable = true; } - htlcs.push(claimable_htlc); - let amount_msat = htlcs.iter().map(|htlc| htlc.value).sum(); - htlcs.iter_mut().for_each(|htlc| htlc.total_value_received = Some(amount_msat)); - let counterparty_skimmed_fee_msat = htlcs.iter() + claimable_payment.htlcs.push(claimable_htlc); + let amount_msat = + claimable_payment.htlcs.iter().map(|htlc| htlc.value).sum(); + claimable_payment.htlcs.iter_mut() + .for_each(|htlc| htlc.total_value_received = Some(amount_msat)); + let counterparty_skimmed_fee_msat = claimable_payment.htlcs.iter() .map(|htlc| htlc.counterparty_skimmed_fee_msat.unwrap_or(0)).sum(); debug_assert!(total_value.saturating_sub(amount_msat) <= counterparty_skimmed_fee_msat); + claimable_payment.htlcs.sort(); + let payment_id = + claimable_payment.inbound_payment_id(&self.inbound_payment_id_secret); new_events.push_back((events::Event::PaymentClaimable { receiver_node_id: Some(receiver_node_id), payment_hash, @@ -5791,13 +5841,14 @@ where via_user_channel_id: Some(prev_user_channel_id), claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER), onion_fields: claimable_payment.onion_fields.clone(), + payment_id: Some(payment_id), }, None)); payment_claimable_generated = true; } else { // Nothing to do - we haven't reached the total // payment value yet, wait until we receive more // MPP parts. - htlcs.push(claimable_htlc); + claimable_payment.htlcs.push(claimable_htlc); #[allow(unused_assignments)] { committed_to_claimable = true; } @@ -6594,6 +6645,7 @@ where } } + let payment_id = payment.inbound_payment_id(&self.inbound_payment_id_secret); let claiming_payment = claimable_payments.pending_claiming_payments .entry(payment_hash) .and_modify(|_| { @@ -6611,6 +6663,7 @@ where htlcs, sender_intended_value, onion_fields: payment.onion_fields, + payment_id: Some(payment_id), } }); @@ -7128,6 +7181,7 @@ where htlcs, sender_intended_value: sender_intended_total_msat, onion_fields, + payment_id, }) = payment { self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed { payment_hash, @@ -7137,6 +7191,7 @@ where htlcs, sender_intended_total_msat, onion_fields, + payment_id, }, None)); } }, @@ -12863,6 +12918,7 @@ where previous_hop_monitor.provide_payment_preimage(&payment_hash, &payment_preimage, &args.tx_broadcaster, &bounded_fee_estimator, &args.logger); } } + let payment_id = payment.inbound_payment_id(&inbound_payment_id_secret.unwrap()); pending_events_read.push_back((events::Event::PaymentClaimed { receiver_node_id, payment_hash, @@ -12871,6 +12927,7 @@ where htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(), sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat), onion_fields: payment.onion_fields, + payment_id: Some(payment_id), }, None)); } } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index c89274f47..be5ecb27a 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1729,8 +1729,7 @@ pub trait OnionMessageHandler { fn provided_init_features(&self, their_node_id: PublicKey) -> InitFeatures; } -#[derive(Clone)] -#[cfg_attr(test, derive(Debug, PartialEq))] +#[derive(Clone, Debug, PartialEq, Eq)] /// Information communicated in the onion to the recipient for multi-part tracking and proof that /// the payment is associated with an invoice. pub struct FinalOnionHopData { -- 2.39.5