Allow KeysManager to opt-into the new remote_key derivation

TheBlueMatt · TheBlueMatt · commit 189b8ac4a767 · 2025-10-09T20:10:40.000Z
The `remote_key` derived by default in `KeysManager` depends on the
chanel's `channel_keys_id`, which generally has sufficient entropy
that without it the `remote_key` cannot be re-derived. In disaster
case where there is no remaining state except the `KeysManager`'s
`seed`, this results in lost funds, even if the counterparty
force-closes the channel.

Luckily, because of the `static_remote_key` feature, there's no
need for this. If the `remote_key` we derive is one of a countable
set, we can simply scan the chain for outputs to our `remote_key`s.

Here we finally allow users to opt into the new derivation scheme,
using the new derivation scheme for `remote_key`s for new and
spliced channels if a new `KeysManager::new` argument is set to
`true`.
diff --git a/ext-functional-test-demo/src/main.rs b/ext-functional-test-demo/src/main.rs
@@ -16,7 +16,7 @@ mod tests {
 
 	impl TestSignerFactory for BrokenSignerFactory {
 		fn make_signer(
-			&self, _seed: &[u8; 32], _now: Duration,
+			&self, _seed: &[u8; 32], _now: Duration, _v2_remote_key_derivation: bool,
 		) -> Box<dyn DynKeysInterfaceTrait<EcdsaSigner = DynSigner>> {
 			panic!()
 		}
diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
@@ -391,6 +391,7 @@ impl SignerProvider for KeyProvider {
 			// real reason to fuzz differences here.
 			SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, self.node_secret[31]]).unwrap(),
 			SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 6, self.node_secret[31]]).unwrap(),
+			true,
 			SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, self.node_secret[31]]).unwrap(),
 			SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, self.node_secret[31]]).unwrap(),
 			[id, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, self.node_secret[31]],
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
@@ -478,7 +478,7 @@ impl SignerProvider for KeyProvider {
 		f = key;
 		// We leave both the v1 and v2 derivation to_remote keys the same as there's not any real
 		// reason to fuzz differences here, and it keeps us consistent with past behavior.
-		let signer = InMemorySigner::new(a, b, c, c, d, e, f, keys_id, keys_id);
+		let signer = InMemorySigner::new(a, b, c, c, true, d, e, f, keys_id, keys_id);
 
 		TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false, false)
 	}
diff --git a/fuzz/src/lsps_message.rs b/fuzz/src/lsps_message.rs
@@ -39,7 +39,7 @@ pub fn do_test(data: &[u8]) {
 	let scorer = Arc::new(LockingWrapper::new(TestScorer::new()));
 	let now = Duration::from_secs(genesis_block.header.time as u64);
 	let seed = sha256::Hash::hash(b"lsps-message-seed").to_byte_array();
-	let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos()));
+	let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
 	let router = Arc::new(DefaultRouter::new(
 		Arc::clone(&network_graph),
 		Arc::clone(&logger),
diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs
@@ -2313,7 +2313,8 @@ mod tests {
 			let scorer = Arc::new(LockingWrapper::new(TestScorer::new()));
 			let now = Duration::from_secs(genesis_block.header.time as u64);
 			let seed = [i as u8; 32];
-			let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos()));
+			let keys_manager =
+				Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
 			let router = Arc::new(DefaultRouter::new(
 				Arc::clone(&network_graph),
 				Arc::clone(&logger),
@@ -2329,7 +2330,8 @@ mod tests {
 			let kv_store =
 				Arc::new(Persister::new(format!("{}_persister_{}", &persist_dir, i).into()));
 			let now = Duration::from_secs(genesis_block.header.time as u64);
-			let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos()));
+			let keys_manager =
+				Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_nanos(), true));
 			let chain_monitor = Arc::new(chainmonitor::ChainMonitor::new(
 				Some(Arc::clone(&chain_source)),
 				Arc::clone(&tx_broadcaster),
diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs
@@ -231,7 +231,7 @@ mod test {
 			&self, recipient: PublicKey, local_node_receive_key: ReceiveAuthKey,
 			context: MessageContext, _peers: Vec<MessageForwardNode>, secp_ctx: &Secp256k1<T>,
 		) -> Result<Vec<BlindedMessagePath>, ()> {
-			let keys = KeysManager::new(&[0; 32], 42, 43);
+			let keys = KeysManager::new(&[0; 32], 42, 43, true);
 			Ok(vec![BlindedMessagePath::one_hop(
 				recipient,
 				local_node_receive_key,
@@ -274,7 +274,7 @@ mod test {
 	}
 
 	fn create_resolver() -> (impl AOnionMessenger, PublicKey) {
-		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43));
+		let resolver_keys = Arc::new(KeysManager::new(&[99; 32], 42, 43, true));
 		let resolver_logger = TestLogger { node: "resolver" };
 		let resolver = OMDomainResolver::ignoring_incoming_proofs("8.8.8.8:53".parse().unwrap());
 		let resolver = Arc::new(resolver);
@@ -313,7 +313,7 @@ mod test {
 		let payment_id = PaymentId([42; 32]);
 		let name = HumanReadableName::from_encoded("matt@mattcorallo.com").unwrap();
 
-		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43));
+		let payer_keys = Arc::new(KeysManager::new(&[2; 32], 42, 43, true));
 		let payer_logger = TestLogger { node: "payer" };
 		let payer_id = payer_keys.get_node_id(Recipient::Node).unwrap();
 		let payer = Arc::new(URIResolver {
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
@@ -6876,6 +6876,7 @@ mod tests {
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
+			true,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			[41; 32],
@@ -7138,6 +7139,7 @@ mod tests {
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
+			true,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			[41; 32],
diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
@@ -1305,6 +1305,7 @@ mod tests {
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
+			true,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			[41; 32],
diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs
@@ -1147,7 +1147,7 @@ mod tests {
 				),
 			]),
 		};
-		let signer = KeysManager::new(&[42; 32], 42, 42);
+		let signer = KeysManager::new(&[42; 32], 42, 42, true);
 		let logger = TestLogger::new();
 		let handler = BumpTransactionEventHandlerSync::new(&broadcaster, &source, &signer, &logger);
 
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -15987,6 +15987,7 @@ mod tests {
 			SecretKey::from_slice(&<Vec<u8>>::from_hex("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
 			SecretKey::from_slice(&<Vec<u8>>::from_hex("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),
 			SecretKey::from_slice(&<Vec<u8>>::from_hex("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),
+			true,
 			SecretKey::from_slice(&<Vec<u8>>::from_hex("3333333333333333333333333333333333333333333333333333333333333333").unwrap()[..]).unwrap(),
 			SecretKey::from_slice(&<Vec<u8>>::from_hex("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),
 
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -19083,7 +19083,7 @@ pub mod bench {
 		config.channel_handshake_config.minimum_depth = 1;
 
 		let seed_a = [1u8; 32];
-		let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
+		let keys_manager_a = KeysManager::new(&seed_a, 42, 42, true);
 		let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a, &keys_manager_a, keys_manager_a.get_peer_storage_key());
 		let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &router, &message_router, &logger_a, &keys_manager_a, &keys_manager_a, &keys_manager_a, config.clone(), ChainParameters {
 			network,
@@ -19093,7 +19093,7 @@ pub mod bench {
 
 		let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
 		let seed_b = [2u8; 32];
-		let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
+		let keys_manager_b = KeysManager::new(&seed_b, 42, 42, true);
 		let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b, &keys_manager_b, keys_manager_b.get_peer_storage_key());
 		let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &router, &message_router, &logger_b, &keys_manager_b, &keys_manager_b, &keys_manager_b, config.clone(), ChainParameters {
 			network,
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
@@ -4167,10 +4167,10 @@ pub fn fail_payment<'a, 'b, 'c>(
 }
 
 pub fn create_chanmon_cfgs(node_count: usize) -> Vec<TestChanMonCfg> {
-	create_chanmon_cfgs_with_keys(node_count, None)
+	create_chanmon_cfgs_with_legacy_keys(node_count, None)
 }
 
-pub fn create_chanmon_cfgs_with_keys(
+pub fn create_chanmon_cfgs_with_legacy_keys(
 	node_count: usize, predefined_keys_ids: Option<Vec<[u8; 32]>>,
 ) -> Vec<TestChanMonCfg> {
 	let mut chan_mon_cfgs = Vec::new();
@@ -4181,7 +4181,12 @@ pub fn create_chanmon_cfgs_with_keys(
 		let logger = test_utils::TestLogger::with_id(format!("node {}", i));
 		let persister = test_utils::TestPersister::new();
 		let seed = [i as u8; 32];
-		let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
+		let keys_manager = if predefined_keys_ids.is_some() {
+			// Use legacy (V1) remote_key derivation for tests using legacy key sets.
+			test_utils::TestKeysInterface::with_v1_remote_key_derivation(&seed, Network::Testnet)
+		} else {
+			test_utils::TestKeysInterface::new(&seed, Network::Testnet)
+		};
 		let scorer = RwLock::new(test_utils::TestScorer::new());
 
 		// Set predefined keys_id if provided
diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs
@@ -1211,7 +1211,7 @@ mod test {
 
 	fn make_dyn_keys_interface(seed: &[u8; 32]) -> DynKeysInterface {
 		let cross_node_seed = [44u8; 32];
-		let inner = PhantomKeysManager::new(&seed, 43, 44, &cross_node_seed);
+		let inner = PhantomKeysManager::new(&seed, 43, 44, &cross_node_seed, true);
 		let dyn_inner = DynPhantomKeysInterface::new(inner);
 		DynKeysInterface::new(Box::new(dyn_inner))
 	}
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
@@ -2301,7 +2301,7 @@ fn do_test_restored_packages_retry(check_old_monitor_retries_after_upgrade: bool
 	let node1_key_id = <[u8; 32]>::from_hex("0000000000000000000000004D49E5DAD000D6201F116BAFD379F1D61DF161B9").unwrap();
 	let predefined_keys_ids = Some(vec![node0_key_id, node1_key_id]);
 
-	let chanmon_cfgs = create_chanmon_cfgs_with_keys(2, predefined_keys_ids);
+	let chanmon_cfgs = create_chanmon_cfgs_with_legacy_keys(2, predefined_keys_ids);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let persister;
 	let new_chain_monitor;
@@ -2451,7 +2451,9 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 		if should_bump {
 			assert!(htlc_tx_feerate > prev_htlc_tx_feerate.take().unwrap());
 		} else if let Some(prev_feerate) = prev_htlc_tx_feerate.take() {
-			assert_eq!(htlc_tx_feerate, prev_feerate);
+			// Feerates may fluctuate marginally based on signature size
+			assert!(htlc_tx_feerate >= prev_feerate - 1);
+			assert!(htlc_tx_feerate <= prev_feerate + 1);
 		}
 		prev_htlc_tx_feerate = Some(htlc_tx_feerate);
 		Some(htlc_tx)
diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs
@@ -663,9 +663,9 @@ mod tests {
 		// adding an intermediate onion layer, causing the receiver to error with "final payload
 		// provided for us as an intermediate node."
 		let secp_ctx = Secp256k1::new();
-		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42);
+		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true);
 		let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
-		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42);
+		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true);
 		let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
 
 		let (
@@ -693,9 +693,9 @@ mod tests {
 		use super::*;
 		let secp_ctx = Secp256k1::new();
 
-		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42);
+		let bob = crate::sign::KeysManager::new(&[2; 32], 42, 42, true);
 		let bob_pk = PublicKey::from_secret_key(&secp_ctx, &bob.get_node_secret_key());
-		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42);
+		let charlie = crate::sign::KeysManager::new(&[3; 32], 42, 42, true);
 		let charlie_pk = PublicKey::from_secret_key(&secp_ctx, &charlie.get_node_secret_key());
 
 		let (session_priv, total_amt_msat, cur_height, recipient_onion, preimage, payment_hash,
diff --git a/lightning/src/ln/our_peer_storage.rs b/lightning/src/ln/our_peer_storage.rs
@@ -37,7 +37,7 @@ use crate::prelude::*;
 /// use lightning::ln::our_peer_storage::DecryptedOurPeerStorage;
 /// use lightning::sign::{KeysManager, NodeSigner};
 /// let seed = [1u8; 32];
-/// let keys_mgr = KeysManager::new(&seed, 42, 42);
+/// let keys_mgr = KeysManager::new(&seed, 42, 42, true);
 /// let key = keys_mgr.get_peer_storage_key();
 /// let decrypted_ops = DecryptedOurPeerStorage::new(vec![1, 2, 3]);
 /// let our_peer_storage = decrypted_ops.encrypt(&key, &[0u8; 32]);
diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs
@@ -561,7 +561,7 @@ mod tests {
 	#[test]
 	#[cfg(feature = "dnssec")]
 	fn test_expiry() {
-		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0);
+		let keys = crate::sign::KeysManager::new(&[33; 32], 0, 0, true);
 		let resolver = OMNameResolver::new(42, 42);
 		let name = HumanReadableName::new("user", "example.com").unwrap();
 
diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs
@@ -222,7 +222,7 @@ where
 /// # }
 /// # let seed = [42u8; 32];
 /// # let time = Duration::from_secs(123456);
-/// # let keys_manager = KeysManager::new(&seed, time.as_secs(), time.subsec_nanos());
+/// # let keys_manager = KeysManager::new(&seed, time.as_secs(), time.subsec_nanos(), true);
 /// # let logger = Arc::new(FakeLogger {});
 /// # let node_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
 /// # let secp_ctx = Secp256k1::new();
diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@ mod tests {`
`16`	`16`
`17`	`17`	`impl TestSignerFactory for BrokenSignerFactory {`
`18`	`18`	`fn make_signer(`
`19`		`- &self, _seed: &[u8; 32], _now: Duration,`
	`19`	`+ &self, _seed: &[u8; 32], _now: Duration, _v2_remote_key_derivation: bool,`
`20`	`20`	`) -> Box<dyn DynKeysInterfaceTrait<EcdsaSigner = DynSigner>> {`
`21`	`21`	`panic!()`
`22`	`22`	`}`
Original file line number	Diff line number	Diff line change
`@@ -478,7 +478,7 @@ impl SignerProvider for KeyProvider {`
`478`	`478`	`f = key;`
`479`	`479`	`// We leave both the v1 and v2 derivation to_remote keys the same as there's not any real`
`480`	`480`	`// reason to fuzz differences here, and it keeps us consistent with past behavior.`
`481`		`- let signer = InMemorySigner::new(a, b, c, c, d, e, f, keys_id, keys_id);`
	`481`	`+ let signer = InMemorySigner::new(a, b, c, c, true, d, e, f, keys_id, keys_id);`
`482`	`482`
`483`	`483`	`TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false, false)`
`484`	`484`	`}`