Make payment_key derivation deterministic

Previously, `KeysManager::derive_channel_keys` would derive the
channel's `payment_key` uniquely on a per-channel basis which would
disallow users losing their `channel_keys_id` to recover funds. As it's
no real necessity to have `payment_key` derivation depend on
`channel_keys_id` we can allow for easier recovery of any non-HTLC
encumbered funds if we make `payment_key` derivation deterministic.

Here we do just this but also include a larger refactor introducing a
`ChannelKeysDerivationParameters` struct holding a versioning field to
be able to express this change in the derivation scheme. To this end we
ensure that legacy channels will continue to use the old derivation
scheme after upgrade, while new channels will derive the `payment_key`
deterministicly.

Author: tnull

fuzz/src/chanmon_consistency.rs

@@ -61,7 +61,8 @@ use lightning::offers::invoice_request::UnsignedInvoiceRequest;
 use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath};
 use lightning::routing::router::{InFlightHtlcs, Path, Route, RouteHop, RouteParameters, Router};
 use lightning::sign::{
-	EntropySource, InMemorySigner, KeyMaterial, NodeSigner, Recipient, SignerProvider,
+	ChannelKeysDerivationParameters, EntropySource, InMemorySigner, KeyMaterial, NodeSigner,
+	Recipient, SignerProvider,
 };
 use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::util::config::UserConfig;
@@ -366,17 +367,23 @@ impl SignerProvider for KeyProvider {
 	#[cfg(taproot)]
 	type TaprootSigner = TestChannelSigner;
 
-	fn generate_channel_keys_id(
+	fn generate_channel_keys_derivation_params(
 		&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128,
-	) -> [u8; 32] {
+	) -> ChannelKeysDerivationParameters {
 		let id = self.rand_bytes_id.fetch_add(1, atomic::Ordering::Relaxed) as u8;
-		[id; 32]
+		let channel_keys_id = [id; 32];
+		ChannelKeysDerivationParameters {
+			channel_keys_derivation_version: Some(1),
+			channel_keys_id,
+		}
 	}
 
 	fn derive_channel_signer(
-		&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32],
+		&self, channel_value_satoshis: u64,
+		channel_keys_derivation_params: ChannelKeysDerivationParameters,
 	) -> Self::EcdsaSigner {
 		let secp_ctx = Secp256k1::signing_only();
+		let channel_keys_id = channel_keys_derivation_params.channel_keys_id;
 		let id = channel_keys_id[0];
 		#[rustfmt::skip]
 		let keys = InMemorySigner::new(
@@ -388,7 +395,7 @@ impl SignerProvider for KeyProvider {
 			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]],
 			channel_value_satoshis,
-			channel_keys_id,
+			channel_keys_derivation_params,
 			channel_keys_id,
 		);
 		let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
@@ -404,7 +411,9 @@ impl SignerProvider for KeyProvider {
 		Ok(TestChannelSigner::new_with_revoked(inner, state, false))
 	}
 
-	fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result<ScriptBuf, ()> {
+	fn get_destination_script(
+		&self, _channel_keys_derivation_params: ChannelKeysDerivationParameters,
+	) -> Result<ScriptBuf, ()> {
 		let secp_ctx = Secp256k1::signing_only();
 		#[rustfmt::skip]
 		let channel_monitor_claim_key = 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, 2, self.node_secret[31]]).unwrap();

fuzz/src/full_stack.rs

@@ -58,7 +58,8 @@ use lightning::routing::router::{
 };
 use lightning::routing::utxo::UtxoLookup;
 use lightning::sign::{
-	EntropySource, InMemorySigner, KeyMaterial, NodeSigner, Recipient, SignerProvider,
+	ChannelKeysDerivationParameters, EntropySource, InMemorySigner, KeyMaterial, NodeSigner,
+	Recipient, SignerProvider,
 };
 use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 use lightning::util::config::{ChannelConfig, UserConfig};
@@ -439,20 +440,26 @@ impl SignerProvider for KeyProvider {
 	#[cfg(taproot)]
 	type TaprootSigner = TestChannelSigner;
 
-	fn generate_channel_keys_id(
+	fn generate_channel_keys_derivation_params(
 		&self, inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128,
-	) -> [u8; 32] {
+	) -> ChannelKeysDerivationParameters {
 		let ctr = self.counter.fetch_add(1, Ordering::Relaxed) as u8;
 		self.signer_state
 			.borrow_mut()
 			.insert(ctr, (inbound, Arc::new(Mutex::new(EnforcementState::new()))));
-		[ctr; 32]
+		let channel_keys_id = [ctr; 32];
+		ChannelKeysDerivationParameters {
+			channel_keys_derivation_version: Some(1),
+			channel_keys_id,
+		}
 	}
 
 	fn derive_channel_signer(
-		&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32],
+		&self, channel_value_satoshis: u64,
+		channel_keys_derivation_params: ChannelKeysDerivationParameters,
 	) -> Self::EcdsaSigner {
 		let secp_ctx = Secp256k1::signing_only();
+		let channel_keys_id = channel_keys_derivation_params.channel_keys_id;
 		let ctr = channel_keys_id[0];
 		let (inbound, state) = self.signer_state.borrow().get(&ctr).unwrap().clone();
 		TestChannelSigner::new_with_revoked(
@@ -489,7 +496,7 @@ impl SignerProvider for KeyProvider {
 						0, 0, 0, 0, 0, 6, ctr,
 					],
 					channel_value_satoshis,
-					channel_keys_id,
+					channel_keys_derivation_params,
 					channel_keys_id,
 				)
 			} else {
@@ -525,7 +532,7 @@ impl SignerProvider for KeyProvider {
 						0, 0, 0, 0, 0, 12, ctr,
 					],
 					channel_value_satoshis,
-					channel_keys_id,
+					channel_keys_derivation_params,
 					channel_keys_id,
 				)
 			},
@@ -541,7 +548,9 @@ impl SignerProvider for KeyProvider {
 		Ok(TestChannelSigner::new_with_revoked(inner, state, false))
 	}
 
-	fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result<ScriptBuf, ()> {
+	fn get_destination_script(
+		&self, _channel_keys_derivation_params: ChannelKeysDerivationParameters,
+	) -> Result<ScriptBuf, ()> {
 		let secp_ctx = Secp256k1::signing_only();
 		let channel_monitor_claim_key = SecretKey::from_slice(
 			&<Vec<u8>>::from_hex(

fuzz/src/onion_message.rs

@@ -23,7 +23,10 @@ use lightning::onion_message::messenger::{
 };
 use lightning::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use lightning::onion_message::packet::OnionMessageContents;
-use lightning::sign::{EntropySource, KeyMaterial, NodeSigner, Recipient, SignerProvider};
+use lightning::sign::{
+	ChannelKeysDerivationParameters, EntropySource, KeyMaterial, NodeSigner, Recipient,
+	SignerProvider,
+};
 use lightning::types::features::InitFeatures;
 use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, Writeable, Writer};
@@ -258,14 +261,15 @@ impl SignerProvider for KeyProvider {
 	#[cfg(taproot)]
 	type TaprootSigner = TestChannelSigner;
 
-	fn generate_channel_keys_id(
+	fn generate_channel_keys_derivation_params(
 		&self, _inbound: bool, _channel_value_satoshis: u64, _user_channel_id: u128,
-	) -> [u8; 32] {
+	) -> ChannelKeysDerivationParameters {
 		unreachable!()
 	}
 
 	fn derive_channel_signer(
-		&self, _channel_value_satoshis: u64, _channel_keys_id: [u8; 32],
+		&self, _channel_value_satoshis: u64,
+		_channel_keys_derivation_params: ChannelKeysDerivationParameters,
 	) -> Self::EcdsaSigner {
 		unreachable!()
 	}
@@ -274,7 +278,9 @@ impl SignerProvider for KeyProvider {
 		unreachable!()
 	}
 
-	fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result<ScriptBuf, ()> {
+	fn get_destination_script(
+		&self, _channel_keys_derivation_params: ChannelKeysDerivationParameters,
+	) -> Result<ScriptBuf, ()> {
 		unreachable!()
 	}
 

lightning/src/chain/channelmonitor.rs

@@ -43,7 +43,7 @@ use crate::chain;
 use crate::chain::{BestBlock, WatchedOutput};
 use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::EcdsaChannelSigner, SignerProvider, EntropySource};
+use crate::sign::{ChannelDerivationParameters, ChannelKeysDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::EcdsaChannelSigner, SignerProvider, EntropySource};
 use crate::chain::onchaintx::{ClaimEvent, FeerateStrategy, OnchainTxHandler};
 use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
 use crate::chain::Filter;
@@ -872,7 +872,7 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	counterparty_payment_script: ScriptBuf,
 	shutdown_script: Option<ScriptBuf>,
 
-	channel_keys_id: [u8; 32],
+	channel_keys_derivation_params: ChannelKeysDerivationParameters,
 	holder_revocation_basepoint: RevocationBasepoint,
 	channel_id: ChannelId,
 	funding_info: (OutPoint, ScriptBuf),
@@ -1076,7 +1076,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
 			None => ScriptBuf::new().write(writer)?,
 		}
 
-		self.channel_keys_id.write(writer)?;
+		self.channel_keys_derivation_params.channel_keys_id.write(writer)?;
 		self.holder_revocation_basepoint.write(writer)?;
 		writer.write_all(&self.funding_info.0.txid[..])?;
 		writer.write_all(&self.funding_info.0.index.to_be_bytes())?;
@@ -1237,6 +1237,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
 			(21, self.balances_empty_height, option),
 			(23, self.holder_pays_commitment_tx_fee, option),
 			(25, self.payment_preimages, required),
+			(27, self.channel_keys_derivation_params.channel_keys_derivation_version, option),
 		});
 
 		Ok(())
@@ -1355,7 +1356,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint;
 		let counterparty_commitment_params = CounterpartyCommitmentParameters { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv };
 
-		let channel_keys_id = keys.channel_keys_id();
+		let channel_keys_derivation_params = keys.channel_keys_derivation_params();
 		let holder_revocation_basepoint = keys.pubkeys().revocation_basepoint;
 
 		// block for Rust 1.34 compat
@@ -1379,7 +1380,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 		};
 
 		let onchain_tx_handler = OnchainTxHandler::new(
-			channel_value_satoshis, channel_keys_id, destination_script.into(), keys,
+			channel_value_satoshis, channel_keys_derivation_params, destination_script.into(), keys,
 			channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx
 		);
 
@@ -1395,7 +1396,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 			counterparty_payment_script,
 			shutdown_script,
 
-			channel_keys_id,
+			channel_keys_derivation_params,
 			holder_revocation_basepoint,
 			channel_id,
 			funding_info,
@@ -3304,7 +3305,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						commitment_tx_fee_satoshis,
 						anchor_descriptor: AnchorDescriptor {
 							channel_derivation_parameters: ChannelDerivationParameters {
-								channel_keys_id: self.channel_keys_id,
+								channel_keys_derivation_params: self.channel_keys_derivation_params,
 								value_satoshis: self.channel_value_satoshis,
 								transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(),
 							},
@@ -3328,7 +3329,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					for htlc in htlcs {
 						htlc_descriptors.push(HTLCDescriptor {
 							channel_derivation_parameters: ChannelDerivationParameters {
-								channel_keys_id: self.channel_keys_id,
+								channel_keys_derivation_params: self.channel_keys_derivation_params,
 								value_satoshis: self.channel_value_satoshis,
 								transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(),
 							},
@@ -4609,7 +4610,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
 					outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 },
 					output: outp.clone(),
-					channel_keys_id: Some(self.channel_keys_id),
+					channel_keys_derivation_params: Some(self.channel_keys_derivation_params),
 				});
 			}
 			if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
@@ -4620,7 +4621,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						to_self_delay: self.on_holder_tx_csv,
 						output: outp.clone(),
 						revocation_pubkey: broadcasted_holder_revokable_script.2,
-						channel_keys_id: self.channel_keys_id,
+						channel_keys_derivation_params: self.channel_keys_derivation_params,
 						channel_value_satoshis: self.channel_value_satoshis,
 						channel_transaction_parameters: Some(self.onchain_tx_handler.channel_transaction_parameters.clone()),
 					}));
@@ -4630,7 +4631,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				spendable_outputs.push(SpendableOutputDescriptor::StaticPaymentOutput(StaticPaymentOutputDescriptor {
 					outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 },
 					output: outp.clone(),
-					channel_keys_id: self.channel_keys_id,
+					channel_keys_derivation_params: self.channel_keys_derivation_params,
 					channel_value_satoshis: self.channel_value_satoshis,
 					channel_transaction_parameters: Some(self.onchain_tx_handler.channel_transaction_parameters.clone()),
 				}));
@@ -4639,7 +4640,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 				spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
 					outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 },
 					output: outp.clone(),
-					channel_keys_id: Some(self.channel_keys_id),
+					channel_keys_derivation_params: Some(self.channel_keys_derivation_params),
 				});
 			}
 		}
@@ -4929,6 +4930,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut channel_id = None;
 		let mut holder_pays_commitment_tx_fee = None;
 		let mut payment_preimages_with_info: Option<HashMap<_, _>> = None;
+		let mut channel_keys_derivation_version: Option<u8> = None;
 		read_tlv_fields!(reader, {
 			(1, funding_spend_confirmed, option),
 			(3, htlcs_resolved_on_chain, optional_vec),
@@ -4943,6 +4945,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(21, balances_empty_height, option),
 			(23, holder_pays_commitment_tx_fee, option),
 			(25, payment_preimages_with_info, option),
+			(27, channel_keys_derivation_version, option),
 		});
 		if let Some(payment_preimages_with_info) = payment_preimages_with_info {
 			if payment_preimages_with_info.len() != payment_preimages.len() {
@@ -4982,6 +4985,11 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 				chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_basepoint).to_p2wsh();
 		}
 
+		let channel_keys_derivation_params = ChannelKeysDerivationParameters {
+			channel_keys_derivation_version,
+			channel_keys_id,
+		};
+
 		Ok((best_block.block_hash, ChannelMonitor::from_impl(ChannelMonitorImpl {
 			latest_update_id,
 			commitment_transaction_number_obscure_factor,
@@ -4991,7 +4999,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			counterparty_payment_script,
 			shutdown_script,
 
-			channel_keys_id,
+			channel_keys_derivation_params,
 			holder_revocation_basepoint,
 			channel_id: channel_id.unwrap_or(ChannelId::v1_from_funding_outpoint(outpoint)),
 			funding_info,
@@ -5070,7 +5078,7 @@ mod tests {
 	use crate::chain::channelmonitor::{ChannelMonitor, WithChannelMonitor};
 	use crate::chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT};
 	use crate::chain::transaction::OutPoint;
-	use crate::sign::InMemorySigner;
+	use crate::sign::{InMemorySigner, ChannelKeysDerivationParameters, CHANNEL_KEYS_DERIVATION_VERSION};
 	use crate::ln::types::ChannelId;
 	use crate::types::payment::{PaymentPreimage, PaymentHash};
 	use crate::ln::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcBasepoint, RevocationBasepoint, RevocationKey};
@@ -5233,6 +5241,11 @@ mod tests {
 			}
 		}
 
+		let dummy_key_derivation_params = ChannelKeysDerivationParameters {
+			channel_keys_derivation_version: Some(CHANNEL_KEYS_DERIVATION_VERSION),
+			channel_keys_id: [0; 32],
+		};
+
 		let keys = InMemorySigner::new(
 			&secp_ctx,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
@@ -5242,7 +5255,7 @@ mod tests {
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			[41; 32],
 			0,
-			[0; 32],
+			dummy_key_derivation_params,
 			[0; 32],
 		);
 
@@ -5485,6 +5498,11 @@ mod tests {
 
 		let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
 
+		let dummy_key_derivation_params = ChannelKeysDerivationParameters {
+			channel_keys_derivation_version: Some(CHANNEL_KEYS_DERIVATION_VERSION),
+			channel_keys_id: [0; 32],
+		};
+
 		let keys = InMemorySigner::new(
 			&secp_ctx,
 			SecretKey::from_slice(&[41; 32]).unwrap(),
@@ -5494,7 +5512,7 @@ mod tests {
 			SecretKey::from_slice(&[41; 32]).unwrap(),
 			[41; 32],
 			0,
-			[0; 32],
+			dummy_key_derivation_params,
 			[0; 32],
 		);