Skip to content

Commit

Permalink
Separate out the methods that would apply to any channel into a `Chan…
Browse files Browse the repository at this point in the history
…nelSigner`, leaving the implementation-dependent ones in `EcdsaChannelSigner`.
  • Loading branch information
arik-so committed Jan 18, 2023
1 parent 454f6ba commit 897f881
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 40 deletions.
68 changes: 39 additions & 29 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,18 +212,12 @@ 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 {
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
Expand All @@ -234,6 +228,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
Expand All @@ -249,12 +244,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
/// [`BaseSign`] 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 [`BaseSign`]
/// 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.
Expand Down Expand Up @@ -395,16 +413,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 [`BaseSign`]
/// 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.
Expand Down Expand Up @@ -725,7 +733,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)
Expand All @@ -743,6 +751,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();
Expand Down Expand Up @@ -871,16 +891,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;
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/chan_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -6871,7 +6871,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;
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
15 changes: 9 additions & 6 deletions lightning/src/util/enforcing_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand All @@ -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);

Expand Down Expand Up @@ -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 {}
Expand Down

0 comments on commit 897f881

Please sign in to comment.