From: Matt Corallo <git@bluematt.me>
Date: Sun, 8 Sep 2024 16:38:22 +0000 (+0000)
Subject: Add an `inbound_payment_id_secret` to `ChannelManager`
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=aa09c33a1719944769ba98624bfe18ea33083f44;p=rust-lightning

Add an `inbound_payment_id_secret` to `ChannelManager`

In the next commit we'll start generating `PaymentId`s for inbound
payments randomly by HMAC'ing the HTLC set of the payment. Here we
start by defining the HMAC secret for these HMACs.

This requires one small test adaptation and a full_stack_target
fuzz change because it changes the RNG consumption.
---

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index 90449248e..e0ce11537 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<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)));
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 2cfa60ea7..0f9bfd889 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2261,6 +2261,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 +3155,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),
 
@@ -12381,6 +12385,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),
@@ -12395,6 +12400,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 +12411,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;
 		}
@@ -12930,6 +12940,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_tests.rs b/lightning/src/ln/functional_tests.rs
index efd2fc9e9..31346c6b7 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).