Merge pull request #4117 from TheBlueMatt/2025-09-actually-static-remote_key

Allow `KeysManager` to opt-into the new `remote_key` derivation

Author: tnull

.github/workflows/build.yml

@@ -287,3 +287,5 @@ jobs:
           rustup component add rustfmt
       - name: Run rustfmt checks
         run: cargo fmt --check
+      - name: Run rustfmt checks on lightning-tests
+        run: cd lightning-tests && cargo fmt --check

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!()
 		}

fuzz/src/chanmon_consistency.rs

@@ -386,14 +386,16 @@ impl SignerProvider for KeyProvider {
 	}
 
 	fn derive_channel_signer(&self, channel_keys_id: [u8; 32]) -> Self::EcdsaSigner {
-		let secp_ctx = Secp256k1::signing_only();
 		let id = channel_keys_id[0];
 		#[rustfmt::skip]
 		let keys = InMemorySigner::new(
-			&secp_ctx,
 			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, 4, 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, 5, self.node_secret[31]]).unwrap(),
+			// We leave both the v1 and v2 derivation to_remote keys the same as there's not any
+			// 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]],

fuzz/src/full_stack.rs

@@ -458,7 +458,6 @@ impl SignerProvider for KeyProvider {
 	}
 
 	fn derive_channel_signer(&self, keys_id: [u8; 32]) -> Self::EcdsaSigner {
-		let secp_ctx = Secp256k1::signing_only();
 		let ctr = keys_id[0];
 		let (inbound, state) = self.signer_state.borrow().get(&ctr).unwrap().clone();
 
@@ -477,7 +476,9 @@ impl SignerProvider for KeyProvider {
 		e = SecretKey::from_slice(&key).unwrap();
 		key[30] = 6 + if inbound { 0 } else { 6 };
 		f = key;
-		let signer = InMemorySigner::new(&secp_ctx, a, b, c, d, e, f, keys_id, keys_id);
+		// 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, true, d, e, f, keys_id, keys_id);
 
 		TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false, false)
 	}

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),

lightning-background-processor/src/lib.rs

@@ -2326,7 +2326,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),
@@ -2342,7 +2343,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),

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("[email protected]").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 {

lightning-tests/src/upgrade_downgrade_tests.rs

@@ -10,6 +10,7 @@
 //! Tests which test upgrading from previous versions of LDK or downgrading to previous versions of
 //! LDK.
 
+use lightning_0_1::events::ClosureReason as ClosureReason_0_1;
 use lightning_0_1::get_monitor as get_monitor_0_1;
 use lightning_0_1::ln::functional_test_utils as lightning_0_1_utils;
 use lightning_0_1::util::ser::Writeable as _;
@@ -28,10 +29,19 @@ use lightning_0_0_125::ln::msgs::ChannelMessageHandler as _;
 use lightning_0_0_125::routing::router as router_0_0_125;
 use lightning_0_0_125::util::ser::Writeable as _;
 
+use lightning::chain::channelmonitor::ANTI_REORG_DELAY;
+use lightning::events::{ClosureReason, Event};
 use lightning::ln::functional_test_utils::*;
+use lightning::sign::OutputSpender;
 
 use lightning_types::payment::PaymentPreimage;
 
+use bitcoin::opcodes;
+use bitcoin::script::Builder;
+use bitcoin::secp256k1::Secp256k1;
+
+use std::sync::Arc;
+
 #[test]
 fn simple_upgrade() {
 	// Tests a simple case of upgrading from LDK 0.1 with a pending payment
@@ -213,3 +223,79 @@ fn test_125_dangling_post_update_actions() {
 	let config = test_default_channel_config();
 	reload_node!(nodes[3], config, &node_d_ser, &[&mon_ser], persister, chain_mon, node);
 }
+
+#[test]
+fn test_0_1_legacy_remote_key_derivation() {
+	// Test that a channel opened with a v1/legacy `remote_key` derivation will be properly spent
+	// even after upgrading and opting into the new v2 derivation for new channels.
+	let (node_a_ser, node_b_ser, mon_a_ser, mon_b_ser, commitment_tx, channel_id);
+	let node_a_blocks;
+	{
+		let chanmon_cfgs = lightning_0_1_utils::create_chanmon_cfgs(2);
+		let node_cfgs = lightning_0_1_utils::create_node_cfgs(2, &chanmon_cfgs);
+		let node_chanmgrs = lightning_0_1_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+		let nodes = lightning_0_1_utils::create_network(2, &node_cfgs, &node_chanmgrs);
+
+		let node_a_id = nodes[0].node.get_our_node_id();
+
+		let chan_id = lightning_0_1_utils::create_announced_chan_between_nodes(&nodes, 0, 1).2;
+		channel_id = chan_id.0;
+
+		let err = "".to_owned();
+		nodes[1].node.force_close_broadcasting_latest_txn(&chan_id, &node_a_id, err).unwrap();
+		commitment_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+		assert_eq!(commitment_tx.len(), 1);
+
+		lightning_0_1_utils::check_added_monitors(&nodes[1], 1);
+		let reason = ClosureReason_0_1::HolderForceClosed { broadcasted_latest_txn: Some(true) };
+		lightning_0_1_utils::check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], 100000);
+		lightning_0_1_utils::check_closed_broadcast(&nodes[1], 1, true);
+
+		node_a_ser = nodes[0].node.encode();
+		node_b_ser = nodes[1].node.encode();
+		mon_a_ser = get_monitor_0_1!(nodes[0], chan_id).encode();
+		mon_b_ser = get_monitor_0_1!(nodes[1], chan_id).encode();
+
+		node_a_blocks = Arc::clone(&nodes[0].blocks);
+	}
+
+	// Create a dummy node to reload over with the 0.1 state
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let (persister_a, persister_b, chain_mon_a, chain_mon_b);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let (node_a, node_b);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let config = test_default_channel_config();
+	let a_mons = &[&mon_a_ser[..]];
+	reload_node!(nodes[0], config.clone(), &node_a_ser, a_mons, persister_a, chain_mon_a, node_a);
+	reload_node!(nodes[1], config, &node_b_ser, &[&mon_b_ser], persister_b, chain_mon_b, node_b);
+
+	nodes[0].blocks = node_a_blocks;
+
+	let node_b_id = nodes[1].node.get_our_node_id();
+
+	mine_transaction(&nodes[0], &commitment_tx[0]);
+	let reason = ClosureReason::CommitmentTxConfirmed;
+	check_closed_event(&nodes[0], 1, reason, false, &[node_b_id], 100_000);
+	check_added_monitors(&nodes[0], 1);
+	check_closed_broadcast(&nodes[0], 1, false);
+
+	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+	let mut spendable_event = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
+	assert_eq!(spendable_event.len(), 1);
+	if let Event::SpendableOutputs { outputs, channel_id: ev_id } = spendable_event.pop().unwrap() {
+		assert_eq!(ev_id.unwrap().0, channel_id);
+		assert_eq!(outputs.len(), 1);
+		let spk = Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script();
+		let spend_tx = nodes[0]
+			.keys_manager
+			.backing
+			.spend_spendable_outputs(&[&outputs[0]], Vec::new(), spk, 253, None, &Secp256k1::new())
+			.unwrap();
+		check_spends!(spend_tx, commitment_tx[0]);
+	} else {
+		panic!("Wrong event");
+	}
+}

lightning/src/chain/channelmonitor.rs

@@ -1872,7 +1872,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 
 		assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
 		let holder_pubkeys = &channel_parameters.holder_pubkeys;
-		let counterparty_payment_script = chan_utils::get_counterparty_payment_script(
+		let counterparty_payment_script = chan_utils::get_countersigner_payment_script(
 			&channel_parameters.channel_type_features, &holder_pubkeys.payment_point
 		);
 
@@ -6806,7 +6806,7 @@ mod tests {
 	use crate::ln::functional_test_utils::*;
 	use crate::ln::script::ShutdownScript;
 	use crate::ln::types::ChannelId;
-	use crate::sign::InMemorySigner;
+	use crate::sign::{ChannelSigner, InMemorySigner};
 	use crate::sync::Arc;
 	use crate::types::features::ChannelTypeFeatures;
 	use crate::types::payment::{PaymentHash, PaymentPreimage};
@@ -6979,11 +6979,12 @@ mod tests {
 		}
 
 		let keys = InMemorySigner::new(
-			&secp_ctx,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			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],
 			[0; 32],
@@ -7000,7 +7001,7 @@ mod tests {
 		let funding_outpoint = OutPoint { txid: Txid::all_zeros(), index: u16::MAX };
 		let channel_id = ChannelId::v1_from_funding_outpoint(funding_outpoint);
 		let channel_parameters = ChannelTransactionParameters {
-			holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+			holder_pubkeys: keys.new_pubkeys(None, &secp_ctx),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
@@ -7241,11 +7242,12 @@ mod tests {
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 
 		let keys = InMemorySigner::new(
-			&secp_ctx,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			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],
 			[0; 32],
@@ -7262,7 +7264,7 @@ mod tests {
 		let funding_outpoint = OutPoint { txid: Txid::all_zeros(), index: u16::MAX };
 		let channel_id = ChannelId::v1_from_funding_outpoint(funding_outpoint);
 		let channel_parameters = ChannelTransactionParameters {
-			holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+			holder_pubkeys: keys.new_pubkeys(None, &secp_ctx),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {

lightning/src/chain/onchaintx.rs

@@ -1287,7 +1287,7 @@ mod tests {
 	};
 	use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint};
 	use crate::ln::functional_test_utils::create_dummy_block;
-	use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, InMemorySigner};
+	use crate::sign::{ChannelDerivationParameters, ChannelSigner, HTLCDescriptor, InMemorySigner};
 	use crate::types::payment::{PaymentHash, PaymentPreimage};
 	use crate::util::test_utils::{TestBroadcaster, TestFeeEstimator, TestLogger};
 
@@ -1301,11 +1301,12 @@ mod tests {
 	fn test_broadcast_height() {
 		let secp_ctx = Secp256k1::new();
 		let signer = InMemorySigner::new(
-			&secp_ctx,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			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],
 			[0; 32],
@@ -1338,7 +1339,7 @@ mod tests {
 		// Use non-anchor channels so that HTLC-Timeouts are broadcast immediately instead of sent
 		// to the user for external funding.
 		let chan_params = ChannelTransactionParameters {
-			holder_pubkeys: signer.holder_channel_pubkeys.clone(),
+			holder_pubkeys: signer.new_pubkeys(None, &secp_ctx),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {