Separate channel-type-agnostic methods into `ChannelSigner` trait.
authorArik Sosman <git@arik.io>
Wed, 18 Jan 2023 04:33:54 +0000 (20:33 -0800)
committerArik Sosman <git@arik.io>
Mon, 23 Jan 2023 23:48:26 +0000 (15:48 -0800)
lightning/src/chain/keysinterface.rs
lightning/src/chain/onchaintx.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/channel.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/enforcing_trait_impls.rs

index c915562a21fb62d289dcefab773bbd2066c6c074..bba7d7596b2098b8dd6cfef2f8efbbf0c079f2a1 100644 (file)
@@ -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
        /// <BIP 143 signature> <payment key>
@@ -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<secp256k1::All>) -> 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<PaymentPreimage>) -> 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<secp256k1::All>
        ) -> Result<Signature, ()>;
-       /// 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<secp256k1::All>) -> 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<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
                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;
index 00d07d86586a07a6a3ca9d50cb04330943ead68a..c14d4fa8fd5497edf42f79fb8438d0f586da2083 100644 (file)
@@ -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)]
index 059aa05abe0525a9b6f574296efbe20536553e7d..31881d05dfa1c1883e066f526d76dca20037df99 100644 (file)
@@ -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;
index b750161169670bde8918eabcc5b540a35b2d296b..fb60a882a33fd5faad1cb47634486572589aa2ae 100644 (file)
@@ -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;
index 6ff4aeb14e04035823ea344e4d0cde8fba5012ae..f645b9df8ac1aea9c9d31515e26c89a0d9bd8a14 100644 (file)
@@ -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};
index f3e94b55d07e73d2703456fe3fa8ebe19f64975e..30a7c39c1800521af126e32ccc130c680e43be54 100644 (file)
@@ -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<secp256k1::All>) -> 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<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
                self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);
 
@@ -228,10 +235,6 @@ impl EcdsaChannelSigner for EnforcingSigner {
        ) -> Result<Signature, ()> {
                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 {}