From 1b245da37041f0f7a3046ee01298880021927f2f Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Tue, 17 Jan 2023 16:30:32 -0800 Subject: [PATCH 1/4] Rename `BaseSign` to `EcdsaChannelSigner`. --- lightning/src/chain/keysinterface.rs | 42 ++++++++++----------- lightning/src/chain/onchaintx.rs | 2 +- lightning/src/ln/chan_utils.rs | 2 +- lightning/src/ln/channel.rs | 6 +-- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/enforcing_trait_impls.rs | 4 +- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 7063a38c4..c915562a2 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -75,7 +75,7 @@ pub struct DelayedPaymentOutputDescriptor { /// The revocation point specific to the commitment transaction which was broadcast. Used to /// derive the witnessScript for this output. pub revocation_pubkey: PublicKey, - /// Arbitrary identification information returned by a call to [`BaseSign::channel_keys_id`]. + /// Arbitrary identification information returned by a call to [`EcdsaChannelSigner::channel_keys_id`]. /// This may be useful in re-deriving keys used in the channel to spend the output. pub channel_keys_id: [u8; 32], /// The value of the channel which this output originated from, possibly indirectly. @@ -107,7 +107,7 @@ pub struct StaticPaymentOutputDescriptor { pub outpoint: OutPoint, /// The output which is referenced by the given outpoint. pub output: TxOut, - /// Arbitrary identification information returned by a call to [`BaseSign::channel_keys_id`]. + /// Arbitrary identification information returned by a call to [`EcdsaChannelSigner::channel_keys_id`]. /// This may be useful in re-deriving keys used in the channel to spend the output. pub channel_keys_id: [u8; 32], /// The value of the channel which this transactions spends. @@ -172,15 +172,15 @@ pub enum SpendableOutputDescriptor { /// /// To derive the delayed payment key which is used to sign this input, you must pass the /// holder [`InMemorySigner::delayed_payment_base_key`] (i.e., the private key which corresponds to the - /// [`ChannelPublicKeys::delayed_payment_basepoint`] in [`BaseSign::pubkeys`]) and the provided + /// [`ChannelPublicKeys::delayed_payment_basepoint`] in [`EcdsaChannelSigner::pubkeys`]) and the provided /// [`DelayedPaymentOutputDescriptor::per_commitment_point`] to [`chan_utils::derive_private_key`]. The public key can be /// generated without the secret key using [`chan_utils::derive_public_key`] and only the - /// [`ChannelPublicKeys::delayed_payment_basepoint`] which appears in [`BaseSign::pubkeys`]. + /// [`ChannelPublicKeys::delayed_payment_basepoint`] which appears in [`EcdsaChannelSigner::pubkeys`]. /// /// To derive the [`DelayedPaymentOutputDescriptor::revocation_pubkey`] provided here (which is /// used in the witness script generation), you must pass the counterparty /// [`ChannelPublicKeys::revocation_basepoint`] (which appears in the call to - /// [`BaseSign::provide_channel_parameters`]) and the provided + /// [`EcdsaChannelSigner::provide_channel_parameters`]) and the provided /// [`DelayedPaymentOutputDescriptor::per_commitment_point`] to /// [`chan_utils::derive_public_revocation_key`]. /// @@ -191,7 +191,7 @@ pub enum SpendableOutputDescriptor { /// [`chan_utils::get_revokeable_redeemscript`]. DelayedPaymentOutput(DelayedPaymentOutputDescriptor), /// An output to a P2WPKH, spendable exclusively by our payment key (i.e., the private key - /// which corresponds to the `payment_point` in [`BaseSign::pubkeys`]). The witness + /// which corresponds to the `payment_point` in [`EcdsaChannelSigner::pubkeys`]). The witness /// in the spending input is, thus, simply: /// ```bitcoin /// @@ -219,7 +219,7 @@ impl_writeable_tlv_based_enum!(SpendableOutputDescriptor, /// policies in order to be secure. Please refer to the [VLS Policy /// Controls](https://gitlab.com/lightning-signer/validating-lightning-signer/-/blob/main/docs/policy-controls.md) /// for an example of such policies. -pub trait BaseSign { +pub trait EcdsaChannelSigner { /// Gets the per-commitment point for a specific commitment number /// /// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards. @@ -253,7 +253,7 @@ pub trait BaseSign { fn pubkeys(&self) -> &ChannelPublicKeys; /// Returns an arbitrary identifier describing the set of keys which are provided back to you in /// some [`SpendableOutputDescriptor`] types. This should be sufficient to identify this - /// [`BaseSign`] object uniquely and lookup or re-derive its keys. + /// [`EcdsaChannelSigner`] object uniquely and lookup or re-derive its keys. fn channel_keys_id(&self) -> [u8; 32]; /// Create a signature for a counterparty's commitment transaction and associated HTLC transactions. /// @@ -398,7 +398,7 @@ pub trait BaseSign { /// Set the counterparty static channel data, including basepoints, /// `counterparty_selected`/`holder_selected_contest_delay` and funding outpoint. /// - /// This data is static, and will never change for a channel once set. For a given [`BaseSign`] + /// This data is static, and will never change for a channel once set. For a given [`EcdsaChannelSigner`] /// instance, LDK will call this method exactly once - either immediately after construction /// (not including if done via [`SignerProvider::read_chan_signer`]) or when the funding /// information has been generated. @@ -414,7 +414,7 @@ pub trait BaseSign { /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor -pub trait Sign: BaseSign + Writeable {} +pub trait Sign: EcdsaChannelSigner + Writeable {} /// Specifies the recipient of an invoice. /// @@ -511,7 +511,7 @@ pub trait SignerProvider { /// To derive a new `Signer`, a fresh `channel_keys_id` should be obtained through /// [`SignerProvider::generate_channel_keys_id`]. Otherwise, an existing `Signer` can be /// re-derived from its `channel_keys_id`, which can be obtained through its trait method - /// [`BaseSign::channel_keys_id`]. + /// [`EcdsaChannelSigner::channel_keys_id`]. fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer; /// Reads a [`Signer`] for this [`SignerProvider`] from the given input stream. @@ -620,38 +620,38 @@ impl InMemorySigner { /// Returns the counterparty's pubkeys. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn counterparty_pubkeys(&self) -> &ChannelPublicKeys { &self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().pubkeys } /// Returns the `contest_delay` value specified by our counterparty and applied on holder-broadcastable /// transactions, i.e., the amount of time that we have to wait to recover our funds if we /// broadcast a transaction. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn counterparty_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().selected_contest_delay } /// Returns the `contest_delay` value specified by us and applied on transactions broadcastable /// by our counterparty, i.e., the amount of time that they have to wait to recover their funds /// if they broadcast a transaction. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn holder_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().holder_selected_contest_delay } /// Returns whether the holder is the initiator. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn is_outbound(&self) -> bool { self.get_channel_parameters().is_outbound_from_holder } /// Funding outpoint /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn funding_outpoint(&self) -> &OutPoint { self.get_channel_parameters().funding_outpoint.as_ref().unwrap() } /// Returns a [`ChannelTransactionParameters`] for this channel, to be used when verifying or /// building transactions. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn get_channel_parameters(&self) -> &ChannelTransactionParameters { self.channel_parameters.as_ref().unwrap() } /// Returns whether anchors should be used. /// - /// Will panic if [`BaseSign::provide_channel_parameters`] has not been called before. + /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. pub fn opt_anchors(&self) -> bool { self.get_channel_parameters().opt_anchors.is_some() } @@ -725,7 +725,7 @@ impl InMemorySigner { } } -impl BaseSign for InMemorySigner { +impl EcdsaChannelSigner for InMemorySigner { fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey { let commitment_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx)).unwrap(); PublicKey::from_secret_key(secp_ctx, &commitment_secret) @@ -1452,8 +1452,8 @@ impl PhantomKeysManager { } } -// Ensure that BaseSign can have a vtable +// Ensure that EcdsaChannelSigner can have a vtable #[test] pub fn dyn_sign() { - let _signer: Box; + let _signer: Box; } diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index f526cc8aa..00d07d865 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -21,7 +21,7 @@ use bitcoin::hash_types::{Txid, BlockHash}; use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; -use crate::chain::keysinterface::{BaseSign, EntropySource, SignerProvider}; +use crate::chain::keysinterface::{EcdsaChannelSigner, EntropySource, SignerProvider}; use crate::ln::msgs::DecodeError; use crate::ln::PaymentPreimage; #[cfg(anchors)] diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index cd9f261b2..059aa05ab 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1635,7 +1635,7 @@ mod tests { use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; use bitcoin::secp256k1::{PublicKey, SecretKey, Secp256k1}; use crate::util::test_utils; - use crate::chain::keysinterface::{BaseSign, SignerProvider}; + use crate::chain::keysinterface::{EcdsaChannelSigner, SignerProvider}; use bitcoin::{Network, Txid}; use bitcoin::hashes::Hash; use crate::ln::PaymentHash; diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 062321606..b75016116 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -35,7 +35,7 @@ use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::{Sign, EntropySource, BaseSign, NodeSigner, Recipient, SignerProvider}; +use crate::chain::keysinterface::{Sign, EntropySource, EcdsaChannelSigner, SignerProvider, NodeSigner, Recipient}; use crate::util::events::ClosureReason; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use crate::util::logger::Logger; @@ -6901,7 +6901,7 @@ mod tests { use crate::ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight}; use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget}; - use crate::chain::keysinterface::{BaseSign, InMemorySigner, EntropySource, SignerProvider}; + use crate::chain::keysinterface::{EcdsaChannelSigner, InMemorySigner, EntropySource, SignerProvider}; use crate::chain::transaction::OutPoint; use crate::util::config::UserConfig; use crate::util::enforcing_trait_impls::EnforcingSigner; @@ -7406,7 +7406,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use bitcoin::hash_types::Txid; use bitcoin::secp256k1::Message; - use crate::chain::keysinterface::BaseSign; + use crate::chain::keysinterface::EcdsaChannelSigner; use crate::ln::PaymentPreimage; use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys}; use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 126056184..6ff4aeb14 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -17,7 +17,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::channelmonitor; use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use crate::chain::transaction::OutPoint; -use crate::chain::keysinterface::{BaseSign, EntropySource}; +use crate::chain::keysinterface::{EcdsaChannelSigner, EntropySource}; use crate::ln::{PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA}; diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 769d00ecf..f3e94b55d 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -10,7 +10,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::{chan_utils, msgs, PaymentPreimage}; -use crate::chain::keysinterface::{Sign, InMemorySigner, BaseSign}; +use crate::chain::keysinterface::{Sign, InMemorySigner, EcdsaChannelSigner}; use crate::prelude::*; use core::cmp; @@ -90,7 +90,7 @@ impl EnforcingSigner { } } -impl BaseSign for EnforcingSigner { +impl EcdsaChannelSigner for EnforcingSigner { fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey { self.inner.get_per_commitment_point(idx, secp_ctx) } From 30c45469e52105cbe979b5160a467658c8e964be Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Tue, 17 Jan 2023 20:33:54 -0800 Subject: [PATCH 2/4] Separate channel-type-agnostic methods into `ChannelSigner` trait. --- lightning/src/chain/keysinterface.rs | 98 ++++++++++++--------- lightning/src/chain/onchaintx.rs | 2 +- lightning/src/ln/chan_utils.rs | 2 +- lightning/src/ln/channel.rs | 4 +- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/enforcing_trait_impls.rs | 15 ++-- 6 files changed, 69 insertions(+), 54 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index c915562a2..bba7d7596 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -75,7 +75,7 @@ pub struct DelayedPaymentOutputDescriptor { /// The revocation point specific to the commitment transaction which was broadcast. Used to /// derive the witnessScript for this output. pub revocation_pubkey: PublicKey, - /// Arbitrary identification information returned by a call to [`EcdsaChannelSigner::channel_keys_id`]. + /// Arbitrary identification information returned by a call to [`ChannelSigner::channel_keys_id`]. /// This may be useful in re-deriving keys used in the channel to spend the output. pub channel_keys_id: [u8; 32], /// The value of the channel which this output originated from, possibly indirectly. @@ -107,7 +107,7 @@ pub struct StaticPaymentOutputDescriptor { pub outpoint: OutPoint, /// The output which is referenced by the given outpoint. pub output: TxOut, - /// Arbitrary identification information returned by a call to [`EcdsaChannelSigner::channel_keys_id`]. + /// Arbitrary identification information returned by a call to [`ChannelSigner::channel_keys_id`]. /// This may be useful in re-deriving keys used in the channel to spend the output. pub channel_keys_id: [u8; 32], /// The value of the channel which this transactions spends. @@ -172,15 +172,15 @@ pub enum SpendableOutputDescriptor { /// /// To derive the delayed payment key which is used to sign this input, you must pass the /// holder [`InMemorySigner::delayed_payment_base_key`] (i.e., the private key which corresponds to the - /// [`ChannelPublicKeys::delayed_payment_basepoint`] in [`EcdsaChannelSigner::pubkeys`]) and the provided + /// [`ChannelPublicKeys::delayed_payment_basepoint`] in [`ChannelSigner::pubkeys`]) and the provided /// [`DelayedPaymentOutputDescriptor::per_commitment_point`] to [`chan_utils::derive_private_key`]. The public key can be /// generated without the secret key using [`chan_utils::derive_public_key`] and only the - /// [`ChannelPublicKeys::delayed_payment_basepoint`] which appears in [`EcdsaChannelSigner::pubkeys`]. + /// [`ChannelPublicKeys::delayed_payment_basepoint`] which appears in [`ChannelSigner::pubkeys`]. /// /// To derive the [`DelayedPaymentOutputDescriptor::revocation_pubkey`] provided here (which is /// used in the witness script generation), you must pass the counterparty /// [`ChannelPublicKeys::revocation_basepoint`] (which appears in the call to - /// [`EcdsaChannelSigner::provide_channel_parameters`]) and the provided + /// [`ChannelSigner::provide_channel_parameters`]) and the provided /// [`DelayedPaymentOutputDescriptor::per_commitment_point`] to /// [`chan_utils::derive_public_revocation_key`]. /// @@ -191,7 +191,7 @@ pub enum SpendableOutputDescriptor { /// [`chan_utils::get_revokeable_redeemscript`]. DelayedPaymentOutput(DelayedPaymentOutputDescriptor), /// An output to a P2WPKH, spendable exclusively by our payment key (i.e., the private key - /// which corresponds to the `payment_point` in [`EcdsaChannelSigner::pubkeys`]). The witness + /// which corresponds to the `payment_point` in [`ChannelSigner::pubkeys`]). The witness /// in the spending input is, thus, simply: /// ```bitcoin /// @@ -212,18 +212,14 @@ impl_writeable_tlv_based_enum!(SpendableOutputDescriptor, (2, StaticPaymentOutput), ); -/// A trait to sign Lightning channel transactions as described in -/// [BOLT 3](https://github.com/lightning/bolts/blob/master/03-transactions.md). -/// -/// Signing services could be implemented on a hardware wallet and should implement signing -/// policies in order to be secure. Please refer to the [VLS Policy -/// Controls](https://gitlab.com/lightning-signer/validating-lightning-signer/-/blob/main/docs/policy-controls.md) -/// for an example of such policies. -pub trait EcdsaChannelSigner { +/// A trait to handle Lightning channel key material without concretizing the channel type or +/// the signature mechanism. +pub trait ChannelSigner { /// Gets the per-commitment point for a specific commitment number /// /// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards. fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey; + /// Gets the commitment secret for a specific commitment number as part of the revocation process /// /// An external signer implementation should error here if the commitment was already signed @@ -234,6 +230,7 @@ pub trait EcdsaChannelSigner { /// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards. // TODO: return a Result so we can signal a validation error fn release_commitment_secret(&self, idx: u64) -> [u8; 32]; + /// Validate the counterparty's signatures on the holder commitment transaction and HTLCs. /// /// This is required in order for the signer to make sure that releasing a commitment @@ -249,12 +246,35 @@ pub trait EcdsaChannelSigner { /// irrelevant or duplicate preimages. fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, preimages: Vec) -> Result<(), ()>; + /// Returns the holder's channel public keys and basepoints. fn pubkeys(&self) -> &ChannelPublicKeys; + /// Returns an arbitrary identifier describing the set of keys which are provided back to you in /// some [`SpendableOutputDescriptor`] types. This should be sufficient to identify this /// [`EcdsaChannelSigner`] object uniquely and lookup or re-derive its keys. fn channel_keys_id(&self) -> [u8; 32]; + + /// Set the counterparty static channel data, including basepoints, + /// `counterparty_selected`/`holder_selected_contest_delay` and funding outpoint. + /// + /// This data is static, and will never change for a channel once set. For a given [`ChannelSigner`] + /// instance, LDK will call this method exactly once - either immediately after construction + /// (not including if done via [`SignerProvider::read_chan_signer`]) or when the funding + /// information has been generated. + /// + /// channel_parameters.is_populated() MUST be true. + fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters); +} + +/// A trait to sign Lightning channel transactions as described in +/// [BOLT 3](https://github.com/lightning/bolts/blob/master/03-transactions.md). +/// +/// Signing services could be implemented on a hardware wallet and should implement signing +/// policies in order to be secure. Please refer to the [VLS Policy +/// Controls](https://gitlab.com/lightning-signer/validating-lightning-signer/-/blob/main/docs/policy-controls.md) +/// for an example of such policies. +pub trait EcdsaChannelSigner: ChannelSigner { /// Create a signature for a counterparty's commitment transaction and associated HTLC transactions. /// /// Note that if signing fails or is rejected, the channel will be force-closed. @@ -395,16 +415,6 @@ pub trait EcdsaChannelSigner { fn sign_channel_announcement_with_funding_key( &self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1 ) -> Result; - /// Set the counterparty static channel data, including basepoints, - /// `counterparty_selected`/`holder_selected_contest_delay` and funding outpoint. - /// - /// This data is static, and will never change for a channel once set. For a given [`EcdsaChannelSigner`] - /// instance, LDK will call this method exactly once - either immediately after construction - /// (not including if done via [`SignerProvider::read_chan_signer`]) or when the funding - /// information has been generated. - /// - /// channel_parameters.is_populated() MUST be true. - fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters); } /// A writeable signer. @@ -511,7 +521,7 @@ pub trait SignerProvider { /// To derive a new `Signer`, a fresh `channel_keys_id` should be obtained through /// [`SignerProvider::generate_channel_keys_id`]. Otherwise, an existing `Signer` can be /// re-derived from its `channel_keys_id`, which can be obtained through its trait method - /// [`EcdsaChannelSigner::channel_keys_id`]. + /// [`ChannelSigner::channel_keys_id`]. fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> Self::Signer; /// Reads a [`Signer`] for this [`SignerProvider`] from the given input stream. @@ -620,38 +630,38 @@ impl InMemorySigner { /// Returns the counterparty's pubkeys. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn counterparty_pubkeys(&self) -> &ChannelPublicKeys { &self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().pubkeys } /// Returns the `contest_delay` value specified by our counterparty and applied on holder-broadcastable /// transactions, i.e., the amount of time that we have to wait to recover our funds if we /// broadcast a transaction. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn counterparty_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().selected_contest_delay } /// Returns the `contest_delay` value specified by us and applied on transactions broadcastable /// by our counterparty, i.e., the amount of time that they have to wait to recover their funds /// if they broadcast a transaction. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn holder_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().holder_selected_contest_delay } /// Returns whether the holder is the initiator. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn is_outbound(&self) -> bool { self.get_channel_parameters().is_outbound_from_holder } /// Funding outpoint /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn funding_outpoint(&self) -> &OutPoint { self.get_channel_parameters().funding_outpoint.as_ref().unwrap() } /// Returns a [`ChannelTransactionParameters`] for this channel, to be used when verifying or /// building transactions. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn get_channel_parameters(&self) -> &ChannelTransactionParameters { self.channel_parameters.as_ref().unwrap() } /// Returns whether anchors should be used. /// - /// Will panic if [`EcdsaChannelSigner::provide_channel_parameters`] has not been called before. + /// Will panic if [`ChannelSigner::provide_channel_parameters`] has not been called before. pub fn opt_anchors(&self) -> bool { self.get_channel_parameters().opt_anchors.is_some() } @@ -725,7 +735,7 @@ impl InMemorySigner { } } -impl EcdsaChannelSigner for InMemorySigner { +impl ChannelSigner for InMemorySigner { fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey { let commitment_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx)).unwrap(); PublicKey::from_secret_key(secp_ctx, &commitment_secret) @@ -743,6 +753,18 @@ impl EcdsaChannelSigner for InMemorySigner { fn channel_keys_id(&self) -> [u8; 32] { self.channel_keys_id } + fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { + assert!(self.channel_parameters.is_none() || self.channel_parameters.as_ref().unwrap() == channel_parameters); + if self.channel_parameters.is_some() { + // The channel parameters were already set and they match, return early. + return; + } + assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); + self.channel_parameters = Some(channel_parameters.clone()); + } +} + +impl EcdsaChannelSigner for InMemorySigner { fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, _preimages: Vec, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { let trusted_tx = commitment_tx.trust(); let keys = trusted_tx.keys(); @@ -871,16 +893,6 @@ impl EcdsaChannelSigner for InMemorySigner { let msghash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]); Ok(sign(secp_ctx, &msghash, &self.funding_key)) } - - fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { - assert!(self.channel_parameters.is_none() || self.channel_parameters.as_ref().unwrap() == channel_parameters); - if self.channel_parameters.is_some() { - // The channel parameters were already set and they match, return early. - return; - } - assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); - self.channel_parameters = Some(channel_parameters.clone()); - } } const SERIALIZATION_VERSION: u8 = 1; diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 00d07d865..c14d4fa8f 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -21,7 +21,7 @@ use bitcoin::hash_types::{Txid, BlockHash}; use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; -use crate::chain::keysinterface::{EcdsaChannelSigner, EntropySource, SignerProvider}; +use crate::chain::keysinterface::{ChannelSigner, EntropySource, SignerProvider}; use crate::ln::msgs::DecodeError; use crate::ln::PaymentPreimage; #[cfg(anchors)] diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 059aa05ab..31881d05d 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1635,7 +1635,7 @@ mod tests { use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; use bitcoin::secp256k1::{PublicKey, SecretKey, Secp256k1}; use crate::util::test_utils; - use crate::chain::keysinterface::{EcdsaChannelSigner, SignerProvider}; + use crate::chain::keysinterface::{ChannelSigner, SignerProvider}; use bitcoin::{Network, Txid}; use bitcoin::hashes::Hash; use crate::ln::PaymentHash; diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b75016116..fb60a882a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -35,7 +35,7 @@ use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::{Sign, EntropySource, EcdsaChannelSigner, SignerProvider, NodeSigner, Recipient}; +use crate::chain::keysinterface::{Sign, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; use crate::util::events::ClosureReason; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use crate::util::logger::Logger; @@ -6901,7 +6901,7 @@ mod tests { use crate::ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight}; use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget}; - use crate::chain::keysinterface::{EcdsaChannelSigner, InMemorySigner, EntropySource, SignerProvider}; + use crate::chain::keysinterface::{ChannelSigner, InMemorySigner, EntropySource, SignerProvider}; use crate::chain::transaction::OutPoint; use crate::util::config::UserConfig; use crate::util::enforcing_trait_impls::EnforcingSigner; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 6ff4aeb14..f645b9df8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -17,7 +17,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::channelmonitor; use crate::chain::channelmonitor::{CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use crate::chain::transaction::OutPoint; -use crate::chain::keysinterface::{EcdsaChannelSigner, EntropySource}; +use crate::chain::keysinterface::{ChannelSigner, EcdsaChannelSigner, EntropySource}; use crate::ln::{PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA}; diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index f3e94b55d..30a7c39c1 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -10,7 +10,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::{chan_utils, msgs, PaymentPreimage}; -use crate::chain::keysinterface::{Sign, InMemorySigner, EcdsaChannelSigner}; +use crate::chain::keysinterface::{Sign, InMemorySigner, ChannelSigner, EcdsaChannelSigner}; use crate::prelude::*; use core::cmp; @@ -90,7 +90,7 @@ impl EnforcingSigner { } } -impl EcdsaChannelSigner for EnforcingSigner { +impl ChannelSigner for EnforcingSigner { fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey { self.inner.get_per_commitment_point(idx, secp_ctx) } @@ -114,8 +114,15 @@ impl EcdsaChannelSigner for EnforcingSigner { } fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() } + fn channel_keys_id(&self) -> [u8; 32] { self.inner.channel_keys_id() } + fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { + self.inner.provide_channel_parameters(channel_parameters) + } +} + +impl EcdsaChannelSigner for EnforcingSigner { fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx); @@ -228,10 +235,6 @@ impl EcdsaChannelSigner for EnforcingSigner { ) -> Result { self.inner.sign_channel_announcement_with_funding_key(msg, secp_ctx) } - - fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { - self.inner.provide_channel_parameters(channel_parameters) - } } impl Sign for EnforcingSigner {} From 394491115c2117cdf6afe98c286742ec49478011 Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Thu, 19 Jan 2023 15:51:38 -0800 Subject: [PATCH 3/4] Rename `Sign` to `WriteableEcdsaChannelSigner`. --- lightning/src/chain/chainmonitor.rs | 26 ++++++++++----------- lightning/src/chain/channelmonitor.rs | 26 ++++++++++----------- lightning/src/chain/keysinterface.rs | 14 +++++------ lightning/src/chain/mod.rs | 4 ++-- lightning/src/chain/onchaintx.rs | 8 +++---- lightning/src/chain/package.rs | 12 +++++----- lightning/src/ln/channel.rs | 8 +++---- lightning/src/ln/channelmanager.rs | 4 ++-- lightning/src/util/enforcing_trait_impls.rs | 4 ++-- lightning/src/util/persist.rs | 4 ++-- lightning/src/util/test_utils.rs | 2 +- 11 files changed, 56 insertions(+), 56 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 430f6bbac..3a2077209 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -31,7 +31,7 @@ use crate::chain::{ChannelMonitorUpdateStatus, Filter, WatchedOutput}; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Balance, MonitorEvent, TransactionOutputs, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::Sign; +use crate::chain::keysinterface::WriteableEcdsaChannelSigner; use crate::util::atomic_counter::AtomicCounter; use crate::util::logger::Logger; use crate::util::errors::APIError; @@ -68,7 +68,7 @@ impl MonitorUpdateId { pub(crate) fn from_monitor_update(update: &ChannelMonitorUpdate) -> Self { Self { contents: UpdateOrigin::OffChain(update.update_id) } } - pub(crate) fn from_new_monitor(monitor: &ChannelMonitor) -> Self { + pub(crate) fn from_new_monitor(monitor: &ChannelMonitor) -> Self { Self { contents: UpdateOrigin::OffChain(monitor.get_latest_update_id()) } } } @@ -93,7 +93,7 @@ impl MonitorUpdateId { /// [`ChannelMonitorUpdateStatus::PermanentFailure`], in which case the channel will likely be /// closed without broadcasting the latest state. See /// [`ChannelMonitorUpdateStatus::PermanentFailure`] for more details. -pub trait Persist { +pub trait Persist { /// Persist a new channel's data in response to a [`chain::Watch::watch_channel`] call. This is /// called by [`ChannelManager`] for new channels, or may be called directly, e.g. on startup. /// @@ -147,7 +147,7 @@ pub trait Persist { fn update_persisted_channel(&self, channel_id: OutPoint, update: Option<&ChannelMonitorUpdate>, data: &ChannelMonitor, update_id: MonitorUpdateId) -> ChannelMonitorUpdateStatus; } -struct MonitorHolder { +struct MonitorHolder { monitor: ChannelMonitor, /// The full set of pending monitor updates for this Channel. /// @@ -182,7 +182,7 @@ struct MonitorHolder { last_chain_persist_height: AtomicUsize, } -impl MonitorHolder { +impl MonitorHolder { fn has_pending_offchain_updates(&self, pending_monitor_updates_lock: &MutexGuard>) -> bool { pending_monitor_updates_lock.iter().any(|update_id| if let UpdateOrigin::OffChain(_) = update_id.contents { true } else { false }) @@ -197,12 +197,12 @@ impl MonitorHolder { /// /// Note that this holds a mutex in [`ChainMonitor`] and may block other events until it is /// released. -pub struct LockedChannelMonitor<'a, ChannelSigner: Sign> { +pub struct LockedChannelMonitor<'a, ChannelSigner: WriteableEcdsaChannelSigner> { lock: RwLockReadGuard<'a, HashMap>>, funding_txo: OutPoint, } -impl Deref for LockedChannelMonitor<'_, ChannelSigner> { +impl Deref for LockedChannelMonitor<'_, ChannelSigner> { type Target = ChannelMonitor; fn deref(&self) -> &ChannelMonitor { &self.lock.get(&self.funding_txo).expect("Checked at construction").monitor @@ -218,7 +218,7 @@ impl Deref for LockedChannelMonitor<'_, ChannelSigner> { /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [module-level documentation]: crate::chain::chainmonitor -pub struct ChainMonitor +pub struct ChainMonitor where C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -242,7 +242,7 @@ pub struct ChainMonitor ChainMonitor +impl ChainMonitor where C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -516,7 +516,7 @@ where C::Target: chain::Filter, } } -impl +impl chain::Listen for ChainMonitor where C::Target: chain::Filter, @@ -543,7 +543,7 @@ where } } -impl +impl chain::Confirm for ChainMonitor where C::Target: chain::Filter, @@ -592,7 +592,7 @@ where } } -impl +impl chain::Watch for ChainMonitor where C::Target: chain::Filter, T::Target: BroadcasterInterface, @@ -735,7 +735,7 @@ where C::Target: chain::Filter, } } -impl events::EventsProvider for ChainMonitor +impl events::EventsProvider for ChainMonitor where C::Target: chain::Filter, T::Target: BroadcasterInterface, F::Target: FeeEstimator, diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 59b60132f..34e5aac21 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -42,7 +42,7 @@ use crate::chain; use crate::chain::{BestBlock, WatchedOutput}; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, SignerProvider, EntropySource}; +use crate::chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource}; #[cfg(anchors)] use crate::chain::onchaintx::ClaimEvent; use crate::chain::onchaintx::OnchainTxHandler; @@ -706,14 +706,14 @@ impl Readable for IrrevocablyResolvedHTLC { /// the "reorg path" (ie disconnecting blocks until you find a common ancestor from both the /// returned block hash and the the current chain and then reconnecting blocks to get to the /// best chain) upon deserializing the object! -pub struct ChannelMonitor { +pub struct ChannelMonitor { #[cfg(test)] pub(crate) inner: Mutex>, #[cfg(not(test))] inner: Mutex>, } -pub(crate) struct ChannelMonitorImpl { +pub(crate) struct ChannelMonitorImpl { latest_update_id: u64, commitment_transaction_number_obscure_factor: u64, @@ -857,7 +857,7 @@ pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>); #[cfg(any(test, fuzzing, feature = "_test_utils"))] /// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying /// object -impl PartialEq for ChannelMonitor { +impl PartialEq for ChannelMonitor { fn eq(&self, other: &Self) -> bool { let inner = self.inner.lock().unwrap(); let other = other.inner.lock().unwrap(); @@ -868,7 +868,7 @@ impl PartialEq for ChannelMonitor { #[cfg(any(test, fuzzing, feature = "_test_utils"))] /// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying /// object -impl PartialEq for ChannelMonitorImpl { +impl PartialEq for ChannelMonitorImpl { fn eq(&self, other: &Self) -> bool { if self.latest_update_id != other.latest_update_id || self.commitment_transaction_number_obscure_factor != other.commitment_transaction_number_obscure_factor || @@ -912,7 +912,7 @@ impl PartialEq for ChannelMonitorImpl { } } -impl Writeable for ChannelMonitor { +impl Writeable for ChannelMonitor { fn write(&self, writer: &mut W) -> Result<(), Error> { self.inner.lock().unwrap().write(writer) } @@ -922,7 +922,7 @@ impl Writeable for ChannelMonitor { const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; -impl Writeable for ChannelMonitorImpl { +impl Writeable for ChannelMonitorImpl { fn write(&self, writer: &mut W) -> Result<(), Error> { write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); @@ -1090,7 +1090,7 @@ impl Writeable for ChannelMonitorImpl { } } -impl ChannelMonitor { +impl ChannelMonitor { /// For lockorder enforcement purposes, we need to have a single site which constructs the /// `inner` mutex, otherwise cases where we lock two monitors at the same time (eg in our /// PartialEq implementation) we may decide a lockorder violation has occurred. @@ -1521,7 +1521,7 @@ impl ChannelMonitor { } } -impl ChannelMonitorImpl { +impl ChannelMonitorImpl { /// Helper for get_claimable_balances which does the work for an individual HTLC, generating up /// to one `Balance` for the HTLC. fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, holder_commitment: bool, @@ -1684,7 +1684,7 @@ impl ChannelMonitorImpl { } } -impl ChannelMonitor { +impl ChannelMonitor { /// Gets the balances in this channel which are either claimable by us if we were to /// force-close the channel now or which are claimable on-chain (possibly awaiting /// confirmation). @@ -2082,7 +2082,7 @@ pub fn deliberately_bogus_accepted_htlc_witness() -> Vec> { vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into() } -impl ChannelMonitorImpl { +impl ChannelMonitorImpl { /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen /// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key). @@ -3664,7 +3664,7 @@ impl ChannelMonitorImpl { } } -impl chain::Listen for (ChannelMonitor, T, F, L) +impl chain::Listen for (ChannelMonitor, T, F, L) where T::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -3679,7 +3679,7 @@ where } } -impl chain::Confirm for (ChannelMonitor, T, F, L) +impl chain::Confirm for (ChannelMonitor, T, F, L) where T::Target: BroadcasterInterface, F::Target: FeeEstimator, diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index bba7d7596..0cd960c66 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -424,7 +424,7 @@ pub trait EcdsaChannelSigner: ChannelSigner { /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor -pub trait Sign: EcdsaChannelSigner + Writeable {} +pub trait WriteableEcdsaChannelSigner: EcdsaChannelSigner + Writeable {} /// Specifies the recipient of an invoice. /// @@ -505,8 +505,8 @@ pub trait NodeSigner { /// A trait that can return signer instances for individual channels. pub trait SignerProvider { - /// A type which implements [`Sign`] which will be returned by [`Self::derive_channel_signer`]. - type Signer : Sign; + /// A type which implements [`WriteableEcdsaChannelSigner`] which will be returned by [`Self::derive_channel_signer`]. + type Signer : WriteableEcdsaChannelSigner; /// Generates a unique `channel_keys_id` that can be used to obtain a [`Self::Signer`] through /// [`SignerProvider::derive_channel_signer`]. The `user_channel_id` is provided to allow @@ -526,7 +526,7 @@ pub trait SignerProvider { /// Reads a [`Signer`] for this [`SignerProvider`] from the given input stream. /// This is only called during deserialization of other objects which contain - /// [`Sign`]-implementing objects (i.e., [`ChannelMonitor`]s and [`ChannelManager`]s). + /// [`WriteableEcdsaChannelSigner`]-implementing objects (i.e., [`ChannelMonitor`]s and [`ChannelManager`]s). /// The bytes are exactly those which `::write()` writes, and /// contain no versioning scheme. You may wish to include your own version prefix and ensure /// you've read all of the provided bytes to ensure no corruption occurred. @@ -553,7 +553,7 @@ pub trait SignerProvider { } #[derive(Clone)] -/// A simple implementation of [`Sign`] that just keeps the private keys in memory. +/// A simple implementation of [`WriteableEcdsaChannelSigner`] that just keeps the private keys in memory. /// /// This implementation performs no policy checks and is insufficient by itself as /// a secure external signer. @@ -899,7 +899,7 @@ const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; -impl Sign for InMemorySigner {} +impl WriteableEcdsaChannelSigner for InMemorySigner {} impl Writeable for InMemorySigner { fn write(&self, writer: &mut W) -> Result<(), Error> { @@ -1064,7 +1064,7 @@ impl KeysManager { Err(_) => panic!("Your rng is busted"), } } - /// Derive an old [`Sign`] containing per-channel secrets based on a key derivation parameters. + /// Derive an old [`WriteableEcdsaChannelSigner`] containing per-channel secrets based on a key derivation parameters. pub fn derive_channel_keys(&self, channel_value_satoshis: u64, params: &[u8; 32]) -> InMemorySigner { let chan_id = u64::from_be_bytes(params[0..8].try_into().unwrap()); let mut unique_start = Sha256::engine(); diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 19218ed23..01eae4887 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -18,7 +18,7 @@ use bitcoin::network::constants::Network; use bitcoin::secp256k1::PublicKey; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, MonitorEvent}; -use crate::chain::keysinterface::Sign; +use crate::chain::keysinterface::WriteableEcdsaChannelSigner; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::prelude::*; @@ -291,7 +291,7 @@ pub enum ChannelMonitorUpdateStatus { /// multiple instances. /// /// [`PermanentFailure`]: ChannelMonitorUpdateStatus::PermanentFailure -pub trait Watch { +pub trait Watch { /// Watches a channel identified by `funding_txo` using `monitor`. /// /// Implementations are responsible for watching the chain for the funding transaction along diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index c14d4fa8f..7dcb8ff89 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -31,7 +31,7 @@ use crate::ln::chan_utils::{ChannelTransactionParameters, HolderCommitmentTransa use crate::chain::chaininterface::ConfirmationTarget; use crate::chain::chaininterface::{FeeEstimator, BroadcasterInterface, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER}; -use crate::chain::keysinterface::Sign; +use crate::chain::keysinterface::WriteableEcdsaChannelSigner; #[cfg(anchors)] use crate::chain::package::PackageSolvingData; use crate::chain::package::PackageTemplate; @@ -219,7 +219,7 @@ type PackageID = [u8; 32]; /// OnchainTxHandler receives claiming requests, aggregates them if it's sound, broadcast and /// do RBF bumping if possible. -pub struct OnchainTxHandler { +pub struct OnchainTxHandler { destination_script: Script, holder_commitment: HolderCommitmentTransaction, // holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment @@ -271,7 +271,7 @@ pub struct OnchainTxHandler { const SERIALIZATION_VERSION: u8 = 1; const MIN_SERIALIZATION_VERSION: u8 = 1; -impl OnchainTxHandler { +impl OnchainTxHandler { pub(crate) fn write(&self, writer: &mut W) -> Result<(), io::Error> { write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); @@ -415,7 +415,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP } } -impl OnchainTxHandler { +impl OnchainTxHandler { pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1) -> Self { OnchainTxHandler { destination_script, diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 227e5ccd1..5537a56f2 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -25,7 +25,7 @@ use crate::ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment}; use crate::ln::chan_utils; use crate::ln::msgs::DecodeError; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; -use crate::chain::keysinterface::Sign; +use crate::chain::keysinterface::WriteableEcdsaChannelSigner; #[cfg(anchors)] use crate::chain::onchaintx::ExternalHTLCClaim; use crate::chain::onchaintx::OnchainTxHandler; @@ -392,7 +392,7 @@ impl PackageSolvingData { _ => { mem::discriminant(self) == mem::discriminant(&input) } } } - fn finalize_input(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler) -> bool { + fn finalize_input(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler) -> bool { match self { PackageSolvingData::RevokedOutput(ref outp) => { let chan_keys = TxCreationKeys::derive_new(&onchain_handler.secp_ctx, &outp.per_commitment_point, &outp.counterparty_delayed_payment_base_key, &outp.counterparty_htlc_base_key, &onchain_handler.signer.pubkeys().revocation_basepoint, &onchain_handler.signer.pubkeys().htlc_basepoint); @@ -448,7 +448,7 @@ impl PackageSolvingData { } true } - fn get_finalized_tx(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler) -> Option { + fn get_finalized_tx(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler) -> Option { match self { PackageSolvingData::HolderHTLCOutput(ref outp) => { debug_assert!(!outp.opt_anchors()); @@ -657,7 +657,7 @@ impl PackageTemplate { inputs_weight + witnesses_weight + transaction_weight + output_weight } #[cfg(anchors)] - pub(crate) fn construct_malleable_package_with_external_funding( + pub(crate) fn construct_malleable_package_with_external_funding( &self, onchain_handler: &mut OnchainTxHandler, ) -> Option> { debug_assert!(self.requires_external_funding()); @@ -675,7 +675,7 @@ impl PackageTemplate { } htlcs } - pub(crate) fn finalize_malleable_package( + pub(crate) fn finalize_malleable_package( &self, onchain_handler: &mut OnchainTxHandler, value: u64, destination_script: Script, logger: &L ) -> Option where L::Target: Logger { debug_assert!(self.is_malleable()); @@ -703,7 +703,7 @@ impl PackageTemplate { log_debug!(logger, "Finalized transaction {} ready to broadcast", bumped_tx.txid()); Some(bumped_tx) } - pub(crate) fn finalize_untractable_package( + pub(crate) fn finalize_untractable_package( &self, onchain_handler: &mut OnchainTxHandler, logger: &L, ) -> Option where L::Target: Logger { debug_assert!(!self.is_malleable()); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fb60a882a..185fd59ba 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -35,7 +35,7 @@ use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator}; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::{OutPoint, TransactionData}; -use crate::chain::keysinterface::{Sign, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; +use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; use crate::util::events::ClosureReason; use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use crate::util::logger::Logger; @@ -498,7 +498,7 @@ pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5; // // Holder designates channel data owned for the benefice of the user client. // Counterparty designates channel data owned by the another channel participant entity. -pub(super) struct Channel { +pub(super) struct Channel { config: LegacyChannelConfig, // Track the previous `ChannelConfig` so that we can continue forwarding HTLCs that were @@ -832,7 +832,7 @@ macro_rules! secp_check { }; } -impl Channel { +impl Channel { /// Returns the value to use for `holder_max_htlc_value_in_flight_msat` as a percentage of the /// `channel_value_satoshis` in msat, set through /// [`ChannelHandshakeConfig::max_inbound_htlc_value_in_flight_percent_of_channel`] @@ -6133,7 +6133,7 @@ impl Readable for AnnouncementSigsState { } } -impl Writeable for Channel { +impl Writeable for Channel { fn write(&self, writer: &mut W) -> Result<(), io::Error> { // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been // called. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d08f8da55..03707e749 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -55,7 +55,7 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VA use crate::ln::outbound_payment; use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment}; use crate::ln::wire::Encode; -use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, Sign, SignerProvider}; +use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, WriteableEcdsaChannelSigner, SignerProvider}; use crate::util::config::{UserConfig, ChannelConfig}; use crate::util::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; use crate::util::events; @@ -452,7 +452,7 @@ pub(crate) enum MonitorUpdateCompletionAction { } /// State we hold per-peer. -pub(super) struct PeerState { +pub(super) struct PeerState { /// `temporary_channel_id` or `channel_id` -> `channel`. /// /// Holds all channels where the peer is the counterparty. Once a channel has been assigned a diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 30a7c39c1..c13907bb9 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -10,7 +10,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::{chan_utils, msgs, PaymentPreimage}; -use crate::chain::keysinterface::{Sign, InMemorySigner, ChannelSigner, EcdsaChannelSigner}; +use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, InMemorySigner, ChannelSigner, EcdsaChannelSigner}; use crate::prelude::*; use core::cmp; @@ -237,7 +237,7 @@ impl EcdsaChannelSigner for EnforcingSigner { } } -impl Sign for EnforcingSigner {} +impl WriteableEcdsaChannelSigner for EnforcingSigner {} impl Writeable for EnforcingSigner { fn write(&self, writer: &mut W) -> Result<(), Error> { diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs index 2e45685da..aa705f286 100644 --- a/lightning/src/util/persist.rs +++ b/lightning/src/util/persist.rs @@ -16,7 +16,7 @@ use crate::routing::scoring::WriteableScore; use crate::chain; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use crate::chain::chainmonitor::{Persist, MonitorUpdateId}; -use crate::chain::keysinterface::{EntropySource, NodeSigner, Sign, SignerProvider}; +use crate::chain::keysinterface::{EntropySource, NodeSigner, WriteableEcdsaChannelSigner, SignerProvider}; use crate::chain::transaction::OutPoint; use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate}; use crate::ln::channelmanager::ChannelManager; @@ -80,7 +80,7 @@ impl<'a, A: KVStorePersister, M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Dere } } -impl Persist for K { +impl Persist for K { // TODO: We really need a way for the persister to inform the user that its time to crash/shut // down once these start returning failure. // A PermanentFailure implies we should probably just shut down the node since we're diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 221cc1c62..7b4037bec 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -234,7 +234,7 @@ impl TestPersister { self.update_rets.lock().unwrap().push_back(next_ret); } } -impl chainmonitor::Persist for TestPersister { +impl chainmonitor::Persist for TestPersister { fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor, _id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus { if let Some(update_ret) = self.update_rets.lock().unwrap().pop_front() { return update_ret From 712c60e1c78481ba612ba6e5b0bc12cc9f10ba98 Mon Sep 17 00:00:00 2001 From: Arik Sosman Date: Thu, 19 Jan 2023 16:41:15 -0800 Subject: [PATCH 4/4] Make `Channel` and `ChannelManager` less particular about their `Signer` type. --- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/channelmanager.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 185fd59ba..d9ec57d5a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -498,7 +498,7 @@ pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5; // // Holder designates channel data owned for the benefice of the user client. // Counterparty designates channel data owned by the another channel participant entity. -pub(super) struct Channel { +pub(super) struct Channel { config: LegacyChannelConfig, // Track the previous `ChannelConfig` so that we can continue forwarding HTLCs that were diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 03707e749..82640ad15 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -55,7 +55,7 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VA use crate::ln::outbound_payment; use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment}; use crate::ln::wire::Encode; -use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, WriteableEcdsaChannelSigner, SignerProvider}; +use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner}; use crate::util::config::{UserConfig, ChannelConfig}; use crate::util::events::{Event, EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; use crate::util::events; @@ -452,7 +452,7 @@ pub(crate) enum MonitorUpdateCompletionAction { } /// State we hold per-peer. -pub(super) struct PeerState { +pub(super) struct PeerState { /// `temporary_channel_id` or `channel_id` -> `channel`. /// /// Holds all channels where the peer is the counterparty. Once a channel has been assigned a