diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 90449248e32..e0ce11537ef 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -664,7 +664,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { // Adding new calls to `EntropySource::get_secure_random_bytes` during startup can change all the // keys subsequently generated in this test. Rather than regenerating all the messages manually, // it's easier to just increment the counter here so the keys don't change. - keys_manager.counter.fetch_sub(3, Ordering::AcqRel); + keys_manager.counter.fetch_sub(4, Ordering::AcqRel); let network_graph = Arc::new(NetworkGraph::new(network, Arc::clone(&logger))); let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger))); diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index fe52d08c9e1..379a0d79a9a 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 2cfa60ea761..241b02e0957 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 @@ -2261,6 +2307,9 @@ where /// keeping additional state. probing_cookie_secret: [u8; 32], + /// When generating [`PaymentId`]s for inbound payments, we HMAC the HTLCs with this secret. + inbound_payment_id_secret: [u8; 32], + /// The highest block timestamp we've seen, which is usually a good guess at the current time. /// Assuming most miners are generating blocks with reasonable timestamps, this shouldn't be /// very far in the past, and can only ever be up to two hours in the future. @@ -3152,6 +3201,7 @@ where fake_scid_rand_bytes: entropy_source.get_secure_random_bytes(), probing_cookie_secret: entropy_source.get_secure_random_bytes(), + inbound_payment_id_secret: entropy_source.get_secure_random_bytes(), highest_seen_timestamp: AtomicUsize::new(current_timestamp as usize), @@ -5745,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 { @@ -5770,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, @@ -5787,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; } @@ -6590,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(|_| { @@ -6607,6 +6663,7 @@ where htlcs, sender_intended_value, onion_fields: payment.onion_fields, + payment_id: Some(payment_id), } }); @@ -7124,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, @@ -7133,6 +7191,7 @@ where htlcs, sender_intended_total_msat, onion_fields, + payment_id, }, None)); } }, @@ -12381,6 +12440,7 @@ where let mut events_override = None; let mut in_flight_monitor_updates: Option>> = None; let mut decode_update_add_htlcs: Option>> = None; + let mut inbound_payment_id_secret = None; read_tlv_fields!(reader, { (1, pending_outbound_payments_no_retry, option), (2, pending_intercepted_htlcs, option), @@ -12395,6 +12455,7 @@ where (11, probing_cookie_secret, option), (13, claimable_htlc_onion_fields, optional_vec), (14, decode_update_add_htlcs, option), + (15, inbound_payment_id_secret, option), }); let mut decode_update_add_htlcs = decode_update_add_htlcs.unwrap_or_else(|| new_hash_map()); if fake_scid_rand_bytes.is_none() { @@ -12405,6 +12466,10 @@ where probing_cookie_secret = Some(args.entropy_source.get_secure_random_bytes()); } + if inbound_payment_id_secret.is_none() { + inbound_payment_id_secret = Some(args.entropy_source.get_secure_random_bytes()); + } + if let Some(events) = events_override { pending_events_read = events; } @@ -12853,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, @@ -12861,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)); } } @@ -12930,6 +12997,7 @@ where fake_scid_rand_bytes: fake_scid_rand_bytes.unwrap(), probing_cookie_secret: probing_cookie_secret.unwrap(), + inbound_payment_id_secret: inbound_payment_id_secret.unwrap(), our_network_pubkey, secp_ctx, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 1c5af0d8f06..fe5cd18d50e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1083,19 +1083,29 @@ 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) { +fn claimed_htlc_matches_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, path: &[&Node<'a, 'b, 'c>], htlc: &ClaimedHTLC) -> bool { 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()); + htlc.user_channel_id == ch.user_channel_id && + ch.counterparty.node_id == prev.get_our_node_id() +} + +fn check_claimed_htlcs_match_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: &[&[&Node<'a, 'b, 'c>]], htlcs: &[ClaimedHTLC]) { + assert_eq!(route.len(), htlcs.len()); + for path in route { + let mut found_matching_htlc = false; + for htlc in htlcs { + if claimed_htlc_matches_path(origin_node, path, htlc) { + found_matching_htlc = true; + break; + } + } + assert!(found_matching_htlc); + } } 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> { @@ -2832,7 +2842,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { assert_eq!(htlcs.len(), expected_paths.len()); // One per path. assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::(), amount_msat); assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs); - expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc)); + check_claimed_htlcs_match_route(origin_node, expected_paths, htlcs); fwd_amt_msat = amount_msat; }, Event::PaymentClaimed { @@ -2849,7 +2859,7 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 { assert_eq!(htlcs.len(), expected_paths.len()); // One per path. assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::(), amount_msat); assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs); - expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc)); + check_claimed_htlcs_match_route(origin_node, expected_paths, htlcs); fwd_amt_msat = amount_msat; } _ => panic!(), diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index efd2fc9e9d6..31346c6b78b 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7670,8 +7670,8 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { assert_ne!(node_txn[0].input[0].previous_output, node_txn[2].input[0].previous_output); assert_ne!(node_txn[1].input[0].previous_output, node_txn[2].input[0].previous_output); - assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[1].input[0].previous_output); - assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output); + assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[1].input[0].previous_output); + assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output); // node_txn[3] spends the revoked outputs from the revoked_htlc_txn (which only have one // output, checked above). diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index c89274f47d8..be5ecb27ae0 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 {