Merge pull request #3303 from TheBlueMatt/2024-09-inbound-payment-id

Add a `PaymentId` for inbound payments

Author: tnull

fuzz/src/full_stack.rs

@@ -666,7 +666,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger>) {
 	// 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)));

lightning/src/events/mod.rs

@@ -749,6 +749,14 @@ pub enum Event {
 		///
 		/// [`ChannelManager::claim_funds`]: crate::ln::channelmanager::ChannelManager::claim_funds
 		claim_deadline: Option<u32>,
+		/// 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<PaymentId>,
 	},
 	/// 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<RecipientOnionFields>,
+		/// 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<PaymentId>,
 	},
 	/// 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<Vec<ClaimedHTLC>> = Some(vec![]);
 					let mut sender_intended_total_msat: Option<u64> = 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()

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};
@@ -369,6 +369,7 @@ pub(crate) struct HTLCPreviousHopData {
 	counterparty_node_id: Option<PublicKey>,
 }
 
+#[derive(PartialEq, Eq)]
 enum OnionPayload {
 	/// Indicates this incoming onion payload is for the purpose of paying an invoice.
 	Invoice {
@@ -381,6 +382,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,
@@ -412,6 +414,23 @@ impl From<&ClaimableHTLC> for events::ClaimedHTLC {
 	}
 }
 
+impl PartialOrd for ClaimableHTLC {
+	fn partial_cmp(&self, other: &ClaimableHTLC) -> Option<cmp::Ordering> {
+		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
@@ -492,6 +511,22 @@ impl Verification for PaymentId {
 	}
 }
 
+impl PaymentId {
+	fn for_inbound_from_htlcs<I: Iterator<Item=(ChannelId, u64)>>(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::<Sha256>::from_engine(hasher).to_byte_array())
+	}
+}
+
 impl Writeable for PaymentId {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		self.0.write(w)
@@ -765,6 +800,7 @@ struct ClaimingPayment {
 	htlcs: Vec<events::ClaimedHTLC>,
 	sender_intended_value: Option<u64>,
 	onion_fields: Option<RecipientOnionFields>,
+	payment_id: Option<PaymentId>,
 }
 impl_writeable_tlv_based!(ClaimingPayment, {
 	(0, amount_msat, required),
@@ -773,6 +809,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 {
@@ -781,6 +818,15 @@ struct ClaimablePayment {
 	htlcs: Vec<ClaimableHTLC>,
 }
 
+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
@@ -2283,6 +2329,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.
@@ -3176,6 +3225,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),
 
@@ -5769,10 +5819,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 {
@@ -5794,13 +5843,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,
@@ -5811,13 +5865,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;
 											}
@@ -6614,6 +6669,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(|_| {
@@ -6631,6 +6687,7 @@ where
 							htlcs,
 							sender_intended_value,
 							onion_fields: payment.onion_fields,
+							payment_id: Some(payment_id),
 						}
 					});
 
@@ -7148,6 +7205,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,
@@ -7157,6 +7215,7 @@ where
 							htlcs,
 							sender_intended_total_msat,
 							onion_fields,
+							payment_id,
 						}, None));
 					}
 				},
@@ -12428,6 +12487,7 @@ where
 		let mut events_override = None;
 		let mut in_flight_monitor_updates: Option<HashMap<(PublicKey, OutPoint), Vec<ChannelMonitorUpdate>>> = None;
 		let mut decode_update_add_htlcs: Option<HashMap<u64, Vec<msgs::UpdateAddHTLC>>> = None;
+		let mut inbound_payment_id_secret = None;
 		read_tlv_fields!(reader, {
 			(1, pending_outbound_payments_no_retry, option),
 			(2, pending_intercepted_htlcs, option),
@@ -12442,6 +12502,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() {
@@ -12452,6 +12513,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;
 		}
@@ -12900,6 +12965,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,
@@ -12908,6 +12974,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));
 				}
 			}
@@ -12978,6 +13045,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,