From 6dfe9130f5083fe02b7f2bbdd9e205debd5d1aef Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 23 Sep 2024 15:44:46 +0900 Subject: [PATCH 01/10] Drop `InMemorySigner` `Writeable` impl ... which we haven't been using since 0.0.119 / commit 7a951b1bf7615fd7afb1e20c1c627e58a3599c84. --- lightning/src/sign/mod.rs | 26 ++--------------------- lightning/src/util/test_channel_signer.rs | 15 +------------ 2 files changed, 3 insertions(+), 38 deletions(-) diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index fa463df9388..51a0189917d 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -58,11 +58,11 @@ use crate::ln::script::ShutdownScript; use crate::offers::invoice::UnsignedBolt12Invoice; use crate::offers::invoice_request::UnsignedInvoiceRequest; use crate::types::payment::PaymentPreimage; -use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer}; +use crate::util::ser::{Readable, ReadableArgs, Writeable}; use crate::util::transaction_utils; use crate::crypto::chacha20::ChaCha20; -use crate::io::{self, Error}; +use crate::io; use crate::ln::msgs::DecodeError; use crate::prelude::*; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -1785,28 +1785,6 @@ impl TaprootChannelSigner for InMemorySigner { const SERIALIZATION_VERSION: u8 = 1; -const MIN_SERIALIZATION_VERSION: u8 = 1; - -impl Writeable for InMemorySigner { - fn write(&self, writer: &mut W) -> Result<(), Error> { - write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); - - self.funding_key.write(writer)?; - self.revocation_base_key.write(writer)?; - self.payment_key.write(writer)?; - self.delayed_payment_base_key.write(writer)?; - self.htlc_base_key.write(writer)?; - self.commitment_seed.write(writer)?; - self.channel_parameters.write(writer)?; - self.channel_value_satoshis.write(writer)?; - self.channel_keys_id.write(writer)?; - - write_tlv_fields!(writer, {}); - - Ok(()) - } -} - impl ReadableArgs for InMemorySigner where ES::Target: EntropySource, diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index f3ef4dc1557..29acd16933b 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -9,7 +9,7 @@ use crate::ln::channel::{ANCHOR_OUTPUT_VALUE_SATOSHI, MIN_CHAN_DUST_LIMIT_SATOSHIS}; use crate::ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction, ClosingTransaction}; -use crate::ln::channel_keys::{HtlcKey}; +use crate::ln::channel_keys::HtlcKey; use crate::ln::msgs; use crate::types::payment::PaymentPreimage; use crate::sign::{InMemorySigner, ChannelSigner}; @@ -35,8 +35,6 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; #[cfg(taproot)] use musig2::types::{PartialSignature, PublicNonce}; use crate::sign::HTLCDescriptor; -use crate::util::ser::{Writeable, Writer}; -use crate::io::Error; use crate::types::features::ChannelTypeFeatures; #[cfg(taproot)] use crate::ln::msgs::PartialSignatureWithNonce; @@ -402,17 +400,6 @@ impl TaprootChannelSigner for TestChannelSigner { } } -impl Writeable for TestChannelSigner { - fn write(&self, writer: &mut W) -> Result<(), Error> { - // TestChannelSigner has two fields - `inner` ([`InMemorySigner`]) and `state` - // ([`EnforcementState`]). `inner` is serialized here and deserialized by - // [`SignerProvider::read_chan_signer`]. `state` is managed by [`SignerProvider`] - // and will be serialized as needed by the implementation of that trait. - self.inner.write(writer)?; - Ok(()) - } -} - impl TestChannelSigner { fn verify_counterparty_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { commitment_tx.verify( From 75c2b0abdc79088e2122e579879f1d956d65eb8e Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 25 Sep 2024 09:40:40 +0900 Subject: [PATCH 02/10] Rename `payment_key` to `payment_basepoint` in `chan_utils` .. as it doesn't use the actual signer's `payment_key`, but the associated public key. --- lightning/src/ln/chan_utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index ae76308f0ce..1c81e0595ac 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -550,11 +550,11 @@ pub fn get_revokeable_redeemscript(revocation_key: &RevocationKey, contest_delay /// Returns the script for the counterparty's output on a holder's commitment transaction based on /// the channel type. -pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatures, payment_key: &PublicKey) -> ScriptBuf { +pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatures, payment_basepoint: &PublicKey) -> ScriptBuf { if channel_type_features.supports_anchors_zero_fee_htlc_tx() { - get_to_countersignatory_with_anchors_redeemscript(payment_key).to_p2wsh() + get_to_countersignatory_with_anchors_redeemscript(payment_basepoint).to_p2wsh() } else { - ScriptBuf::new_p2wpkh(&WPubkeyHash::hash(&payment_key.serialize())) + ScriptBuf::new_p2wpkh(&WPubkeyHash::hash(&payment_basepoint.serialize())) } } From 611c1a3422da586f9e6c6d15d8088d8600dcd3b0 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 25 Sep 2024 11:34:06 +0900 Subject: [PATCH 03/10] Rename `params` to `channel_keys_id` and take it by value .. to make the name more clear and since before we `clone` it in `derive_channel_keys` anyways. --- lightning/src/sign/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 51a0189917d..3038aee5a75 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1961,11 +1961,11 @@ impl KeysManager { /// Derive an old [`EcdsaChannelSigner`] containing per-channel secrets based on a key derivation parameters. pub fn derive_channel_keys( - &self, channel_value_satoshis: u64, params: &[u8; 32], + &self, channel_value_satoshis: u64, channel_keys_id: [u8; 32], ) -> InMemorySigner { - let chan_id = u64::from_be_bytes(params[0..8].try_into().unwrap()); + let chan_id = u64::from_be_bytes(channel_keys_id[0..8].try_into().unwrap()); let mut unique_start = Sha256::engine(); - unique_start.input(params); + unique_start.input(&channel_keys_id); unique_start.input(&self.seed); // We only seriously intend to rely on the channel_master_key for true secure @@ -2015,7 +2015,7 @@ impl KeysManager { htlc_base_key, commitment_seed, channel_value_satoshis, - params.clone(), + channel_keys_id, prng_seed, ) } @@ -2048,7 +2048,7 @@ impl KeysManager { { let mut signer = self.derive_channel_keys( descriptor.channel_value_satoshis, - &descriptor.channel_keys_id, + descriptor.channel_keys_id, ); if let Some(channel_params) = descriptor.channel_transaction_parameters.as_ref() @@ -2073,7 +2073,7 @@ impl KeysManager { keys_cache = Some(( self.derive_channel_keys( descriptor.channel_value_satoshis, - &descriptor.channel_keys_id, + descriptor.channel_keys_id, ), descriptor.channel_keys_id, )); @@ -2273,7 +2273,7 @@ impl SignerProvider for KeysManager { fn derive_channel_signer( &self, channel_value_satoshis: u64, channel_keys_id: [u8; 32], ) -> Self::EcdsaSigner { - self.derive_channel_keys(channel_value_satoshis, &channel_keys_id) + self.derive_channel_keys(channel_value_satoshis, channel_keys_id) } fn read_chan_signer(&self, reader: &[u8]) -> Result { @@ -2461,9 +2461,9 @@ impl PhantomKeysManager { /// See [`KeysManager::derive_channel_keys`] for documentation on this method. pub fn derive_channel_keys( - &self, channel_value_satoshis: u64, params: &[u8; 32], + &self, channel_value_satoshis: u64, channel_keys_id: [u8; 32], ) -> InMemorySigner { - self.inner.derive_channel_keys(channel_value_satoshis, params) + self.inner.derive_channel_keys(channel_value_satoshis, channel_keys_id) } /// Gets the "node_id" secret key used to sign gossip announcements, decode onion data, etc. From 6be731e9db176062bfe5450790c257bb90159fe0 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Sat, 28 Sep 2024 15:46:56 +0900 Subject: [PATCH 04/10] Rename `keys_id` to `channel_keys_id` .. to align the field everywhere. --- lightning/src/chain/channelmonitor.rs | 4 ++-- lightning/src/chain/onchaintx.rs | 2 +- lightning/src/events/bump_transaction.rs | 6 +++--- lightning/src/ln/channel.rs | 2 +- lightning/src/sign/mod.rs | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 8257e211c0d..4097d6855c1 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3298,7 +3298,7 @@ impl ChannelMonitorImpl { commitment_tx_fee_satoshis, anchor_descriptor: AnchorDescriptor { channel_derivation_parameters: ChannelDerivationParameters { - keys_id: self.channel_keys_id, + channel_keys_id: self.channel_keys_id, value_satoshis: self.channel_value_satoshis, transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), }, @@ -3322,7 +3322,7 @@ impl ChannelMonitorImpl { for htlc in htlcs { htlc_descriptors.push(HTLCDescriptor { channel_derivation_parameters: ChannelDerivationParameters { - keys_id: self.channel_keys_id, + channel_keys_id: self.channel_keys_id, value_satoshis: self.channel_value_satoshis, transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(), }, diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index f19198330d2..43afd1c9596 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1199,7 +1199,7 @@ impl OnchainTxHandler { let htlc_descriptor = HTLCDescriptor { channel_derivation_parameters: ChannelDerivationParameters { value_satoshis: self.channel_value_satoshis, - keys_id: self.channel_keys_id, + channel_keys_id: self.channel_keys_id, transaction_parameters: self.channel_transaction_parameters.clone(), }, commitment_txid: trusted_tx.txid(), diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 3acb2145e5b..c5537cf7d89 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -99,7 +99,7 @@ impl AnchorDescriptor { { let mut signer = signer_provider.derive_channel_signer( self.channel_derivation_parameters.value_satoshis, - self.channel_derivation_parameters.keys_id, + self.channel_derivation_parameters.channel_keys_id, ); signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters); signer @@ -791,7 +791,7 @@ where let mut signers = BTreeMap::new(); for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() { - let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id) + let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.channel_keys_id) .or_insert_with(|| htlc_descriptor.derive_channel_signer(&self.signer_provider)); let htlc_sig = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?; let witness_script = htlc_descriptor.witness_script(&self.secp); @@ -955,7 +955,7 @@ mod tests { anchor_descriptor: AnchorDescriptor { channel_derivation_parameters: ChannelDerivationParameters { value_satoshis: 42_000_000, - keys_id: [42; 32], + channel_keys_id: [42; 32], transaction_parameters: ChannelTransactionParameters::test_dummy(), }, outpoint: OutPoint { txid: Txid::from_byte_array([42; 32]), vout: 0 }, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6d5a4f32a24..1450e36b65b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10488,7 +10488,7 @@ mod tests { let htlc_holder_sig = signer.sign_holder_htlc_transaction(&htlc_tx, 0, &HTLCDescriptor { channel_derivation_parameters: ChannelDerivationParameters { value_satoshis: chan.context.channel_value_satoshis, - keys_id: chan.context.channel_keys_id, + channel_keys_id: chan.context.channel_keys_id, transaction_parameters: chan.context.channel_transaction_parameters.clone(), }, commitment_txid: trusted_tx.txid(), diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 3038aee5a75..2f190314106 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -553,7 +553,7 @@ pub struct ChannelDerivationParameters { /// The value in satoshis of the channel we're attempting to spend the anchor output of. pub value_satoshis: u64, /// The unique identifier to re-derive the signer for the associated channel. - pub keys_id: [u8; 32], + pub channel_keys_id: [u8; 32], /// The necessary channel parameters that need to be provided to the re-derived signer through /// [`ChannelSigner::provide_channel_parameters`]. pub transaction_parameters: ChannelTransactionParameters, @@ -561,7 +561,7 @@ pub struct ChannelDerivationParameters { impl_writeable_tlv_based!(ChannelDerivationParameters, { (0, value_satoshis, required), - (2, keys_id, required), + (2, channel_keys_id, required), (4, transaction_parameters, required), }); @@ -715,7 +715,7 @@ impl HTLCDescriptor { { let mut signer = signer_provider.derive_channel_signer( self.channel_derivation_parameters.value_satoshis, - self.channel_derivation_parameters.keys_id, + self.channel_derivation_parameters.channel_keys_id, ); signer .provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters); From ed2d63e931b594d8ed47e5d8d567362f44887be7 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 31 Oct 2024 11:24:09 +0100 Subject: [PATCH 05/10] Rename `payment_point` to `payment_basepoint` .. to align the field naming with the spec for clarity. --- lightning/src/chain/channelmonitor.rs | 10 +++++----- lightning/src/ln/chan_utils.rs | 24 ++++++++++++------------ lightning/src/ln/channel.rs | 22 +++++++++++----------- lightning/src/sign/mod.rs | 12 +++++++----- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 4097d6855c1..d7327568e41 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1347,7 +1347,7 @@ impl ChannelMonitor { assert!(commitment_transaction_number_obscure_factor <= (1 << 48)); let counterparty_payment_script = chan_utils::get_counterparty_payment_script( - &channel_parameters.channel_type_features, &keys.pubkeys().payment_point + &channel_parameters.channel_type_features, &keys.pubkeys().payment_basepoint ); let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap(); @@ -4971,9 +4971,9 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP if onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() && counterparty_payment_script.is_p2wpkh() { - let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_point; + let payment_basepoint = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_basepoint; counterparty_payment_script = - chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point).to_p2wsh(); + chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_basepoint).to_p2wsh(); } Ok((best_block.block_hash, ChannelMonitor::from_impl(ChannelMonitorImpl { @@ -5243,7 +5243,7 @@ mod tests { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()), revocation_basepoint: RevocationBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap())), - payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()), + payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()), delayed_payment_basepoint: DelayedPaymentBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap())), htlc_basepoint: HtlcBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())) }; @@ -5495,7 +5495,7 @@ mod tests { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()), revocation_basepoint: RevocationBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap())), - payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()), + payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()), delayed_payment_basepoint: DelayedPaymentBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap())), htlc_basepoint: HtlcBasepoint::from(PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())), }; diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 1c81e0595ac..5559f588d77 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -478,7 +478,7 @@ pub struct ChannelPublicKeys { /// The public key on which the non-broadcaster (ie the countersignatory) receives an immediately /// spendable primary channel balance on the broadcaster's commitment transaction. This key is /// static across every commitment transaction. - pub payment_point: PublicKey, + pub payment_basepoint: PublicKey, /// The base point which is used (with derive_public_key) to derive a per-commitment payment /// public key which receives non-HTLC-encumbered funds which are only available for spending /// after some delay (or can be claimed via the revocation path). @@ -491,7 +491,7 @@ pub struct ChannelPublicKeys { impl_writeable_tlv_based!(ChannelPublicKeys, { (0, funding_pubkey, required), (2, revocation_basepoint, required), - (4, payment_point, required), + (4, payment_basepoint, required), (6, delayed_payment_basepoint, required), (8, htlc_basepoint, required), }); @@ -816,9 +816,9 @@ pub(crate) fn legacy_deserialization_prevention_marker_for_channel_type_features /// Gets the witnessScript for the to_remote output when anchors are enabled. #[inline] -pub fn get_to_countersignatory_with_anchors_redeemscript(payment_point: &PublicKey) -> ScriptBuf { +pub fn get_to_countersignatory_with_anchors_redeemscript(payment_basepoint: &PublicKey) -> ScriptBuf { Builder::new() - .push_slice(payment_point.serialize()) + .push_slice(payment_basepoint.serialize()) .push_opcode(opcodes::all::OP_CHECKSIGVERIFY) .push_int(1) .push_opcode(opcodes::all::OP_CSV) @@ -933,7 +933,7 @@ impl ChannelTransactionParameters { let dummy_keys = ChannelPublicKeys { funding_pubkey: PublicKey::from_slice(&[2; 33]).unwrap(), revocation_basepoint: PublicKey::from_slice(&[2; 33]).unwrap().into(), - payment_point: PublicKey::from_slice(&[2; 33]).unwrap(), + payment_basepoint: PublicKey::from_slice(&[2; 33]).unwrap(), delayed_payment_basepoint: PublicKey::from_slice(&[2; 33]).unwrap().into(), htlc_basepoint: PublicKey::from_slice(&[2; 33]).unwrap().into(), }; @@ -1119,7 +1119,7 @@ impl HolderCommitmentTransaction { let channel_pubkeys = ChannelPublicKeys { funding_pubkey: dummy_key.clone(), revocation_basepoint: RevocationBasepoint::from(dummy_key), - payment_point: dummy_key.clone(), + payment_basepoint: dummy_key.clone(), delayed_payment_basepoint: DelayedPaymentBasepoint::from(dummy_key.clone()), htlc_basepoint: HtlcBasepoint::from(dummy_key.clone()) }; @@ -1515,9 +1515,9 @@ impl CommitmentTransaction { if to_countersignatory_value_sat > Amount::ZERO { let script = if channel_parameters.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - get_to_countersignatory_with_anchors_redeemscript(&countersignatory_pubkeys.payment_point).to_p2wsh() + get_to_countersignatory_with_anchors_redeemscript(&countersignatory_pubkeys.payment_basepoint).to_p2wsh() } else { - ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_pubkeys.payment_point.serialize()).into()) + ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_pubkeys.payment_basepoint.serialize()).into()) }; txouts.push(( TxOut { @@ -1608,8 +1608,8 @@ impl CommitmentTransaction { let broadcaster_pubkeys = channel_parameters.broadcaster_pubkeys(); let countersignatory_pubkeys = channel_parameters.countersignatory_pubkeys(); let commitment_transaction_number_obscure_factor = get_commitment_transaction_number_obscure_factor( - &broadcaster_pubkeys.payment_point, - &countersignatory_pubkeys.payment_point, + &broadcaster_pubkeys.payment_basepoint, + &countersignatory_pubkeys.payment_basepoint, channel_parameters.is_outbound(), ); @@ -1974,13 +1974,13 @@ mod tests { // Generate broadcaster and counterparty outputs let tx = builder.build(1000, 2000); assert_eq!(tx.built.transaction.output.len(), 2); - assert_eq!(tx.built.transaction.output[1].script_pubkey, bitcoin::address::Address::p2wpkh(&CompressedPublicKey(builder.counterparty_pubkeys.payment_point), Network::Testnet).script_pubkey()); + assert_eq!(tx.built.transaction.output[1].script_pubkey, bitcoin::address::Address::p2wpkh(&CompressedPublicKey(builder.counterparty_pubkeys.payment_basepoint), Network::Testnet).script_pubkey()); // Generate broadcaster and counterparty outputs as well as two anchors builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); let tx = builder.build(1000, 2000); assert_eq!(tx.built.transaction.output.len(), 4); - assert_eq!(tx.built.transaction.output[3].script_pubkey, get_to_countersignatory_with_anchors_redeemscript(&builder.counterparty_pubkeys.payment_point).to_p2wsh()); + assert_eq!(tx.built.transaction.output[3].script_pubkey, get_to_countersignatory_with_anchors_redeemscript(&builder.counterparty_pubkeys.payment_basepoint).to_p2wsh()); // Generate broadcaster output and anchor let tx = builder.build(3000, 0); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1450e36b65b..783ff8c56a4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1588,7 +1588,7 @@ trait InitialRemoteCommitmentReceiver where SP::Target: SignerProvide let funding_redeemscript = context.get_funding_redeemscript(); let funding_txo = context.get_funding_txo().unwrap(); let funding_txo_script = funding_redeemscript.to_p2wsh(); - let obscure_factor = get_commitment_transaction_number_obscure_factor(&context.get_holder_pubkeys().payment_point, &context.get_counterparty_pubkeys().payment_point, context.is_outbound()); + let obscure_factor = get_commitment_transaction_number_obscure_factor(&context.get_holder_pubkeys().payment_basepoint, &context.get_counterparty_pubkeys().payment_basepoint, context.is_outbound()); let shutdown_script = context.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let mut monitor_signer = signer_provider.derive_channel_signer(context.channel_value_satoshis, context.channel_keys_id); monitor_signer.provide_channel_parameters(&context.channel_transaction_parameters); @@ -2483,7 +2483,7 @@ impl ChannelContext where SP::Target: SignerProvider { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: common_fields.funding_pubkey, revocation_basepoint: RevocationBasepoint::from(common_fields.revocation_basepoint), - payment_point: common_fields.payment_basepoint, + payment_basepoint: common_fields.payment_basepoint, delayed_payment_basepoint: DelayedPaymentBasepoint::from(common_fields.delayed_payment_basepoint), htlc_basepoint: HtlcBasepoint::from(common_fields.htlc_basepoint) }; @@ -2749,7 +2749,7 @@ impl ChannelContext where SP::Target: SignerProvider { log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...", commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number), - get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()), + get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_basepoint, &self.get_counterparty_pubkeys().payment_basepoint, self.is_outbound()), &self.channel_id, if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw); @@ -7975,7 +7975,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, revocation_basepoint: keys.revocation_basepoint.to_public_key(), - payment_basepoint: keys.payment_point, + payment_basepoint: keys.payment_basepoint, delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), htlc_basepoint: keys.htlc_basepoint.to_public_key(), first_per_commitment_point, @@ -8116,7 +8116,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.common_fields.funding_pubkey, revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint), - payment_point: msg.common_fields.payment_basepoint, + payment_basepoint: msg.common_fields.payment_basepoint, delayed_payment_basepoint: DelayedPaymentBasepoint::from(msg.common_fields.delayed_payment_basepoint), htlc_basepoint: HtlcBasepoint::from(msg.common_fields.htlc_basepoint) }; @@ -8189,7 +8189,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, revocation_basepoint: keys.revocation_basepoint.to_public_key(), - payment_basepoint: keys.payment_point, + payment_basepoint: keys.payment_basepoint, delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), htlc_basepoint: keys.htlc_basepoint.to_public_key(), first_per_commitment_point, @@ -8381,7 +8381,7 @@ impl OutboundV2Channel where SP::Target: SignerProvider { max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, revocation_basepoint: keys.revocation_basepoint.to_public_key(), - payment_basepoint: keys.payment_point, + payment_basepoint: keys.payment_basepoint, delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), htlc_basepoint: keys.htlc_basepoint.to_public_key(), first_per_commitment_point, @@ -8439,7 +8439,7 @@ impl InboundV2Channel where SP::Target: SignerProvider { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.common_fields.funding_pubkey, revocation_basepoint: RevocationBasepoint(msg.common_fields.revocation_basepoint), - payment_point: msg.common_fields.payment_basepoint, + payment_basepoint: msg.common_fields.payment_basepoint, delayed_payment_basepoint: DelayedPaymentBasepoint(msg.common_fields.delayed_payment_basepoint), htlc_basepoint: HtlcBasepoint(msg.common_fields.htlc_basepoint) }; @@ -8530,7 +8530,7 @@ impl InboundV2Channel where SP::Target: SignerProvider { max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, revocation_basepoint: keys.revocation_basepoint.to_public_key(), - payment_basepoint: keys.payment_point, + payment_basepoint: keys.payment_basepoint, delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), htlc_basepoint: keys.htlc_basepoint.to_public_key(), first_per_commitment_point, @@ -10367,7 +10367,7 @@ mod tests { let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"), revocation_basepoint: RevocationBasepoint::from(PublicKey::from_slice(&>::from_hex("02466d7fcae563e5cb09a0d1870bb580344804617879a14949cf22285f1bae3f27").unwrap()[..]).unwrap()), - payment_point: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444"), + payment_basepoint: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444"), delayed_payment_basepoint: DelayedPaymentBasepoint::from(public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13")), htlc_basepoint: HtlcBasepoint::from(public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444")) }; @@ -10379,7 +10379,7 @@ mod tests { chan.context.channel_transaction_parameters.funding_outpoint = Some(funding_info); signer.provide_channel_parameters(&chan.context.channel_transaction_parameters); - assert_eq!(counterparty_pubkeys.payment_point.serialize()[..], + assert_eq!(counterparty_pubkeys.payment_basepoint.serialize()[..], >::from_hex("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]); assert_eq!(counterparty_pubkeys.funding_pubkey.serialize()[..], diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 2f190314106..7cefbcc0bcf 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -178,8 +178,10 @@ impl StaticPaymentOutputDescriptor { pub fn witness_script(&self) -> Option { self.channel_transaction_parameters.as_ref().and_then(|channel_params| { if channel_params.supports_anchors() { - let payment_point = channel_params.holder_pubkeys.payment_point; - Some(chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point)) + let payment_basepoint = channel_params.holder_pubkeys.payment_basepoint; + Some(chan_utils::get_to_countersignatory_with_anchors_redeemscript( + &payment_basepoint, + )) } else { None } @@ -282,7 +284,7 @@ pub enum SpendableOutputDescriptor { /// [`chan_utils::get_revokeable_redeemscript`]. DelayedPaymentOutput(DelayedPaymentOutputDescriptor), /// An output spendable exclusively by our payment key (i.e., the private key that corresponds - /// to the `payment_point` in [`ChannelSigner::pubkeys`]). The output type depends on the + /// to the `payment_basepoint` in [`ChannelSigner::pubkeys`]). The output type depends on the /// channel type negotiated. /// /// On an anchor outputs channel, the witness in the spending input is: @@ -1125,7 +1127,7 @@ impl InMemorySigner { ChannelPublicKeys { funding_pubkey: from_secret(&funding_key), revocation_basepoint: RevocationBasepoint::from(from_secret(&revocation_base_key)), - payment_point: from_secret(&payment_key), + payment_basepoint: from_secret(&payment_key), delayed_payment_basepoint: DelayedPaymentBasepoint::from(from_secret( &delayed_payment_base_key, )), @@ -1226,7 +1228,7 @@ impl InMemorySigner { return Err(()); } - let remotepubkey = bitcoin::PublicKey::new(self.pubkeys().payment_point); + let remotepubkey = bitcoin::PublicKey::new(self.pubkeys().payment_basepoint); // We cannot always assume that `channel_parameters` is set, so can't just call // `self.channel_parameters()` or anything that relies on it let supports_anchors_zero_fee_htlc_tx = self From aea8e5e45764c1b4e93df7603d7b5214945828bf Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 6 Nov 2024 10:54:25 +0100 Subject: [PATCH 06/10] 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. To this end, we use the first byte of `channel_keys_id` as a versioning byte indicating the version of the used channel keys derivation scheme. Note that Previously `KeysManager::generate_channel_keys_id` would with very high likelyhood never have generated a `channel_keys_id` with a non-null first byte, which makes this a backwards-compatible change for any users that didn't run custom implementations of `SignerProvider::generate_channel_keys_id` conflicting with this assumption. --- lightning/src/sign/mod.rs | 69 ++++++++++++++++++++++++++++++++------- 1 file changed, 57 insertions(+), 12 deletions(-) diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 7cefbcc0bcf..ce5a5d74e01 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1983,29 +1983,61 @@ impl KeysManager { .expect("Your RNG is busted"); unique_start.input(&child_privkey.private_key[..]); - let seed = Sha256::from_engine(unique_start).to_byte_array(); + let unique_seed = Sha256::from_engine(unique_start).to_byte_array(); let commitment_seed = { let mut sha = Sha256::engine(); - sha.input(&seed); + sha.input(&unique_seed); sha.input(&b"commitment seed"[..]); Sha256::from_engine(sha).to_byte_array() }; macro_rules! key_step { ($info: expr, $prev_key: expr) => {{ let mut sha = Sha256::engine(); - sha.input(&seed); + sha.input(&unique_seed); sha.input(&$prev_key[..]); sha.input(&$info[..]); SecretKey::from_slice(&Sha256::from_engine(sha).to_byte_array()) .expect("SHA-256 is busted") }}; } - let funding_key = key_step!(b"funding key", commitment_seed); - let revocation_base_key = key_step!(b"revocation base key", funding_key); - let payment_key = key_step!(b"payment key", revocation_base_key); - let delayed_payment_base_key = key_step!(b"delayed payment base key", payment_key); - let htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key); + + let funding_key; + let revocation_base_key; + let payment_key; + let delayed_payment_base_key; + let htlc_base_key; + + let channel_keys_derivation_version = + channel_keys_derivation_version_from_id(channel_keys_id); + if channel_keys_derivation_version < 1 { + // In LDK versions prior to 0.1 we used to derive the `payment_key` uniquely on a + // per-channel basis, which disallowed users to re-derive them if they lost their + // `channel_keys_id`. + funding_key = key_step!(b"funding key", commitment_seed); + revocation_base_key = key_step!(b"revocation base key", funding_key); + payment_key = key_step!(b"payment key", revocation_base_key); + delayed_payment_base_key = key_step!(b"delayed payment base key", payment_key); + htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key); + } else { + funding_key = key_step!(b"funding key", commitment_seed); + revocation_base_key = key_step!(b"revocation base key", funding_key); + delayed_payment_base_key = key_step!(b"delayed payment base key", revocation_base_key); + htlc_base_key = key_step!(b"HTLC base key", delayed_payment_base_key); + + // Starting with LDK v0.1, we derive `payment_key` directly from our seed, allowing + // users to re-derive it even when losing all channel state. This allows them to + // recover any non-HTLC-encumbered funds in case of data loss if the counterparty is so + // nice to force-closure for them. + payment_key = { + let mut sha = Sha256::engine(); + sha.input(&self.seed); + sha.input(&b"static payment key"[..]); + SecretKey::from_slice(&Sha256::from_engine(sha).to_byte_array()) + .expect("SHA-256 is busted") + }; + }; + let prng_seed = self.get_secure_random_bytes(); InMemorySigner::new( @@ -2249,6 +2281,14 @@ impl OutputSpender for KeysManager { } } +/// The version of the channel key derivation scheme. +pub(crate) const CHANNEL_KEYS_DERIVATION_VERSION: u8 = 1; + +/// Returns the keys derviation version set in `channel_keys_id`. +pub(crate) fn channel_keys_derivation_version_from_id(channel_keys_id: [u8; 32]) -> u8 { + channel_keys_id[0] +} + impl SignerProvider for KeysManager { type EcdsaSigner = InMemorySigner; #[cfg(taproot)] @@ -2261,11 +2301,16 @@ impl SignerProvider for KeysManager { // `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"); + // in loss of funds. Because we only support 32-bit+ systems, and we use the first byte as + // a versioning field, assert that our `AtomicUsize` + // doesn't reach the maximal 24-bit value, i.e., `U24_MAX`. + const U24_MAX: usize = 0xFFFFFF; + assert!(child_idx < U24_MAX, "2^24 channels opened without restart"); + let child_idx_be_bytes = (child_idx as u32).to_be_bytes(); + let mut id = [0; 32]; - id[0..4].copy_from_slice(&(child_idx as u32).to_be_bytes()); + id[0] = CHANNEL_KEYS_DERIVATION_VERSION; + id[1..4].copy_from_slice(&child_idx_be_bytes[1..4]); id[4..8].copy_from_slice(&self.starting_time_nanos.to_be_bytes()); id[8..16].copy_from_slice(&self.starting_time_secs.to_be_bytes()); id[16..32].copy_from_slice(&user_channel_id.to_be_bytes()); From a781d23402f985f47360e442071c57e0e0b49f37 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 31 Oct 2024 11:03:57 +0100 Subject: [PATCH 07/10] f Adjust test cases to accommodate new derivation scheme Some test cases have hard-coded values which we change here (to be squashed in after review). --- lightning/src/ln/chan_utils.rs | 8 ++++---- lightning/src/ln/functional_tests.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 5559f588d77..2334481b697 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -2015,9 +2015,9 @@ mod tests { assert_eq!(tx.built.transaction.output[0].script_pubkey, get_htlc_redeemscript(&received_htlc, &ChannelTypeFeatures::only_static_remote_key(), &keys).to_p2wsh()); assert_eq!(tx.built.transaction.output[1].script_pubkey, get_htlc_redeemscript(&offered_htlc, &ChannelTypeFeatures::only_static_remote_key(), &keys).to_p2wsh()); assert_eq!(get_htlc_redeemscript(&received_htlc, &ChannelTypeFeatures::only_static_remote_key(), &keys).to_p2wsh().to_hex_string(), - "0020e43a7c068553003fe68fcae424fb7b28ec5ce48cd8b6744b3945631389bad2fb"); + "0020510277dd095f7e3a7a46efbd9de3a49f527b5b32f4acc70f144dc5c928529520"); assert_eq!(get_htlc_redeemscript(&offered_htlc, &ChannelTypeFeatures::only_static_remote_key(), &keys).to_p2wsh().to_hex_string(), - "0020215d61bba56b19e9eadb6107f5a85d7f99c40f65992443f69229c290165bc00d"); + "0020b981c1ea6c0d0c3971e89800312d6fd5564872f22040771d1555605edf5b1cfe"); // Generate broadcaster output and received and offered HTLC outputs, with anchors builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); @@ -2027,9 +2027,9 @@ mod tests { assert_eq!(tx.built.transaction.output[2].script_pubkey, get_htlc_redeemscript(&received_htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &keys).to_p2wsh()); assert_eq!(tx.built.transaction.output[3].script_pubkey, get_htlc_redeemscript(&offered_htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &keys).to_p2wsh()); assert_eq!(get_htlc_redeemscript(&received_htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &keys).to_p2wsh().to_hex_string(), - "0020b70d0649c72b38756885c7a30908d912a7898dd5d79457a7280b8e9a20f3f2bc"); + "002078193f615fc1b050814d866d6c46097f9d19fdae0dbc4905b2b35f427c8d0662"); assert_eq!(get_htlc_redeemscript(&offered_htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), &keys).to_p2wsh().to_hex_string(), - "002087a3faeb1950a469c0e2db4a79b093a41b9526e5a6fc6ef5cb949bde3be379c7"); + "0020ac28c4778c24e79755cedc11e7feb638630aa74478781ae7c809aed06e6bd325"); } #[test] diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 79ce43507d8..23feff57ce2 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7683,8 +7683,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[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); + assert_eq!(node_txn[0].input[0].previous_output, revoked_htlc_txn[0].input[0].previous_output); + assert_eq!(node_txn[1].input[0].previous_output, revoked_htlc_txn[1].input[0].previous_output); // node_txn[3] spends the revoked outputs from the revoked_htlc_txn (which only have one // output, checked above). From bf14ec8d94c20646215b2164e5c1f06cbea12c49 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 6 Nov 2024 12:38:32 +0100 Subject: [PATCH 08/10] f Allow legacy keys derivation for `test_restored_packages_retry` --- lightning/src/ln/functional_test_utils.rs | 7 ++++++- lightning/src/ln/monitor_tests.rs | 5 +++-- lightning/src/util/test_utils.rs | 8 +++++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index de0b4c7d4bb..50bbb9fdcea 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3193,6 +3193,10 @@ pub fn fail_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: & } pub fn create_chanmon_cfgs(node_count: usize) -> Vec { + create_chanmon_cfgs_with_params(node_count, false) +} + +pub fn create_chanmon_cfgs_with_params(node_count: usize, legacy_channel_keys_derivation: bool) -> Vec { let mut chan_mon_cfgs = Vec::new(); for i in 0..node_count { let tx_broadcaster = test_utils::TestBroadcaster::new(Network::Testnet); @@ -3201,7 +3205,8 @@ pub fn create_chanmon_cfgs(node_count: usize) -> Vec { 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 mut keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet); + keys_manager.legacy_channel_keys_derivation = legacy_channel_keys_derivation; let scorer = RwLock::new(test_utils::TestScorer::new()); chan_mon_cfgs.push(TestChanMonCfg { tx_broadcaster, fee_estimator, chain_source, logger, persister, keys_manager, scorer }); diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 4aad8f569fc..2d36c74e245 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2242,8 +2242,9 @@ fn test_claimable_balance_correct_while_payment_pending() { fn do_test_restored_packages_retry(check_old_monitor_retries_after_upgrade: bool) { // Tests that we'll retry packages that were previously timelocked after we've restored them. - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let chanmon_cfgs = create_chanmon_cfgs_with_params(2, true); + let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let persister; let new_chain_monitor; diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 122d7f0af48..229e97e1c63 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -1240,6 +1240,7 @@ pub struct TestKeysInterface { enforcement_states: Mutex>>>, expectations: Mutex>>, pub unavailable_signers_ops: Mutex>>, + pub legacy_channel_keys_derivation: bool, } impl EntropySource for TestKeysInterface { @@ -1292,7 +1293,11 @@ impl SignerProvider for TestKeysInterface { type TaprootSigner = TestChannelSigner; fn generate_channel_keys_id(&self, inbound: bool, channel_value_satoshis: u64, user_channel_id: u128) -> [u8; 32] { - self.backing.generate_channel_keys_id(inbound, channel_value_satoshis, user_channel_id) + let mut channel_keys_id = self.backing.generate_channel_keys_id(inbound, channel_value_satoshis, user_channel_id); + if self.legacy_channel_keys_derivation { + channel_keys_id[0] = 0; + } + channel_keys_id } fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> TestChannelSigner { @@ -1344,6 +1349,7 @@ impl TestKeysInterface { enforcement_states: Mutex::new(new_hash_map()), expectations: Mutex::new(None), unavailable_signers_ops: Mutex::new(new_hash_map()), + legacy_channel_keys_derivation: false, } } From c70f7ec20cbb85fe4eb0e04b64ffd8a59756f812 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Wed, 6 Nov 2024 13:54:44 +0100 Subject: [PATCH 09/10] Have `SignerProvider::get_shutdown_scriptpubkey` take `channel_keys_id` .. to allow users to return specific scripts per channel. --- fuzz/src/chanmon_consistency.rs | 2 +- fuzz/src/full_stack.rs | 2 +- fuzz/src/onion_message.rs | 2 +- lightning/src/ln/async_signer_tests.rs | 5 +++-- lightning/src/ln/channel.rs | 10 +++++----- lightning/src/ln/shutdown_tests.rs | 4 +++- lightning/src/sign/mod.rs | 11 ++++++----- lightning/src/util/test_utils.rs | 6 +++--- 8 files changed, 23 insertions(+), 19 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 821acfc5301..80466c03fdb 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -417,7 +417,7 @@ impl SignerProvider for KeyProvider { .into_script()) } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { let secp_ctx = Secp256k1::signing_only(); #[rustfmt::skip] let secret_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, 3, self.node_secret[31]]).unwrap(); diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 59f1275a600..c053b4d8caf 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -559,7 +559,7 @@ impl SignerProvider for KeyProvider { .into_script()) } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { let secp_ctx = Secp256k1::signing_only(); let secret_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, diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index edf304b5467..4bb69fa33bf 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -278,7 +278,7 @@ impl SignerProvider for KeyProvider { unreachable!() } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { unreachable!() } } diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index f607431820e..42e9d21e995 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -917,8 +917,9 @@ fn do_test_closing_signed(extra_closing_signed: bool, reconnect: bool) { if extra_closing_signed { let node_1_closing_signed_2_bad = { let mut node_1_closing_signed_2 = node_1_closing_signed.clone(); - let holder_script = nodes[0].keys_manager.get_shutdown_scriptpubkey().unwrap(); - let counterparty_script = nodes[1].keys_manager.get_shutdown_scriptpubkey().unwrap(); + let channel_keys_id = [42u8; 32]; + let holder_script = nodes[0].keys_manager.get_shutdown_scriptpubkey(channel_keys_id).unwrap(); + let counterparty_script = nodes[1].keys_manager.get_shutdown_scriptpubkey(channel_keys_id).unwrap(); let funding_outpoint = bitcoin::OutPoint { txid: chan_1.3.compute_txid(), vout: 0 }; let closing_tx_2 = ClosingTransaction::new(50000, 0, holder_script.into(), counterparty_script.into(), funding_outpoint); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 783ff8c56a4..8cd789f068e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1805,7 +1805,7 @@ impl ChannelContext where SP::Target: SignerProvider { } else { None }; let shutdown_scriptpubkey = if config.channel_handshake_config.commit_upfront_shutdown_pubkey { - match signer_provider.get_shutdown_scriptpubkey() { + match signer_provider.get_shutdown_scriptpubkey(channel_keys_id) { Ok(scriptpubkey) => Some(scriptpubkey), Err(_) => return Err(ChannelError::close("Failed to get upfront shutdown scriptpubkey".to_owned())), } @@ -2050,7 +2050,7 @@ impl ChannelContext where SP::Target: SignerProvider { secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); let shutdown_scriptpubkey = if config.channel_handshake_config.commit_upfront_shutdown_pubkey { - match signer_provider.get_shutdown_scriptpubkey() { + match signer_provider.get_shutdown_scriptpubkey(channel_keys_id) { Ok(scriptpubkey) => Some(scriptpubkey), Err(_) => return Err(APIError::ChannelUnavailable { err: "Failed to get shutdown scriptpubkey".to_owned()}), } @@ -6219,7 +6219,7 @@ impl Channel where Some(_) => false, None => { assert!(send_shutdown); - let shutdown_scriptpubkey = match signer_provider.get_shutdown_scriptpubkey() { + let shutdown_scriptpubkey = match signer_provider.get_shutdown_scriptpubkey(self.context.channel_keys_id) { Ok(scriptpubkey) => scriptpubkey, Err(_) => return Err(ChannelError::close("Failed to get shutdown scriptpubkey".to_owned())), }; @@ -7711,7 +7711,7 @@ impl Channel where Some(script) => script, None => { // otherwise, use the shutdown scriptpubkey provided by the signer - match signer_provider.get_shutdown_scriptpubkey() { + match signer_provider.get_shutdown_scriptpubkey(self.context.channel_keys_id) { Ok(scriptpubkey) => scriptpubkey, Err(_) => return Err(APIError::ChannelUnavailable{err: "Failed to get shutdown scriptpubkey".to_owned()}), } @@ -9724,7 +9724,7 @@ mod tests { Ok(Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(channel_monitor_claim_key_hash).into_script()) } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { let secp_ctx = Secp256k1::signing_only(); let channel_close_key = SecretKey::from_slice(&>::from_hex("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); Ok(ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key))) diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 9fd428329af..ae61a5dcace 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -984,7 +984,9 @@ fn test_unsupported_anysegwit_shutdown_script() { let nodes = create_network(3, &node_cfgs, &node_chanmgrs); // Check that using an unsupported shutdown script fails and a supported one succeeds. - let supported_shutdown_script = chanmon_cfgs[1].keys_manager.get_shutdown_scriptpubkey().unwrap(); + let channel_keys_id = [42u8; 32]; + let supported_shutdown_script = chanmon_cfgs[1].keys_manager + .get_shutdown_scriptpubkey(channel_keys_id).unwrap(); let unsupported_witness_program = WitnessProgram::new(WitnessVersion::V16, &[0, 40]).unwrap(); let unsupported_shutdown_script = ShutdownScript::new_witness_program(&unsupported_witness_program).unwrap(); diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index ce5a5d74e01..a5ceca5558e 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1010,8 +1010,9 @@ pub trait SignerProvider { /// channel force close. /// /// This method should return a different value each time it is called, to avoid linking - /// on-chain funds across channels as controlled to the same user. - fn get_shutdown_scriptpubkey(&self) -> Result; + /// on-chain funds across channels as controlled to the same user. `channel_keys_id` may be + /// used to derive a unique value for each channel. + fn get_shutdown_scriptpubkey(&self, channel_keys_id: [u8; 32]) -> Result; } /// A helper trait that describes an on-chain wallet capable of returning a (change) destination @@ -2331,7 +2332,7 @@ impl SignerProvider for KeysManager { Ok(self.destination_script.clone()) } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { Ok(ShutdownScript::new_p2wpkh_from_pubkey(self.shutdown_pubkey.clone())) } } @@ -2470,8 +2471,8 @@ impl SignerProvider for PhantomKeysManager { self.inner.get_destination_script(channel_keys_id) } - fn get_shutdown_scriptpubkey(&self) -> Result { - self.inner.get_shutdown_scriptpubkey() + fn get_shutdown_scriptpubkey(&self, channel_keys_id: [u8; 32]) -> Result { + self.inner.get_shutdown_scriptpubkey(channel_keys_id) } } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 229e97e1c63..ca907cc3338 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -332,7 +332,7 @@ impl SignerProvider for OnlyReadsKeysInterface { } fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result { Err(()) } - fn get_shutdown_scriptpubkey(&self) -> Result { Err(()) } + fn get_shutdown_scriptpubkey(&self, _channel_keys_id: [u8; 32]) -> Result { Err(()) } } pub struct TestChainMonitor<'a> { @@ -1328,9 +1328,9 @@ impl SignerProvider for TestKeysInterface { fn get_destination_script(&self, channel_keys_id: [u8; 32]) -> Result { self.backing.get_destination_script(channel_keys_id) } - fn get_shutdown_scriptpubkey(&self) -> Result { + fn get_shutdown_scriptpubkey(&self, channel_keys_id: [u8; 32]) -> Result { match &mut *self.expectations.lock().unwrap() { - None => self.backing.get_shutdown_scriptpubkey(), + None => self.backing.get_shutdown_scriptpubkey(channel_keys_id), Some(expectations) => match expectations.pop_front() { None => panic!("Unexpected get_shutdown_scriptpubkey"), Some(expectation) => Ok(expectation.returns), From 8991d74a42c36682d50eadfa0af8201674af98c6 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 11 Nov 2024 14:14:33 +0100 Subject: [PATCH 10/10] Drop outdated comment In 6a55dcce8380297c28dc70c37ef543bd18f62617 we fixed an issue where we'd just `unwrap` `InMemorySigner`'s `channel_parameters` field and added a comment explaining why we can't do that. However, in particular as related APIs were also changed in follow-up commits to return `Option`s, the introduced comment is now outdated and doesn't really make sense anymore without that historical context present. As it's more or less misleading by now, we just drop it. --- lightning/src/sign/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index a5ceca5558e..49f084ee12c 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1230,8 +1230,6 @@ impl InMemorySigner { } let remotepubkey = bitcoin::PublicKey::new(self.pubkeys().payment_basepoint); - // We cannot always assume that `channel_parameters` is set, so can't just call - // `self.channel_parameters()` or anything that relies on it let supports_anchors_zero_fee_htlc_tx = self .channel_type_features() .map(|features| features.supports_anchors_zero_fee_htlc_tx())