From: Matt Corallo <git@bluematt.me>
Date: Wed, 28 Dec 2022 18:12:29 +0000 (+0000)
Subject: Ensure the per-channel key derivation counter doesn't role over
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=7d4df95e70fe08af4b7dee8c44d55b8614aaf8ea;p=rust-lightning

Ensure the per-channel key derivation counter doesn't role over

Previously, the `derive_channel_keys` derivation ID asserted that
the high bit of the per-channel key derivation counter doesn't
role over as it checked the 31st bit was zero. As we no longer do
that, we should ensure the assertion in `generate_channel_keys_id`
asserts that we don't role over.
---

diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs
index 0a464193d..d260d0294 100644
--- a/lightning/src/chain/keysinterface.rs
+++ b/lightning/src/chain/keysinterface.rs
@@ -1263,7 +1263,12 @@ impl KeysInterface for KeysManager {
 
 	fn generate_channel_keys_id(&self, _inbound: bool, _channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32] {
 		let child_idx = self.channel_child_index.fetch_add(1, Ordering::AcqRel);
-		assert!(child_idx <= core::u32::MAX as usize);
+		// `child_idx` is the only thing guaranteed to make each channel unique without a restart
+		// (though `user_channel_id` should help, depending on user behavior). If it manages to
+		// roll over, we may generate duplicate keys for two different channels, which could result
+		// in loss of funds. Because we only support 32-bit+ systems, assert that our `AtomicUsize`
+		// doesn't reach `u32::MAX`.
+		assert!(child_idx < core::u32::MAX as usize, "2^32 channels opened without restart");
 		let mut id = [0; 32];
 		id[0..4].copy_from_slice(&(child_idx as u32).to_be_bytes());
 		id[4..8].copy_from_slice(&self.starting_time_nanos.to_be_bytes());