Move ECDSA-specific signers into ecdsa.rs

arik-so · arik-so · commit c0263133809d · 2023-09-12T11:33:52.000-07:00
To separate out the logic in the `sign` module, which will start to be
convoluted with multiple signer types, we're splitting out each signer
type into its own submodule, following the taproot.rs example from a
previous commit.
diff --git 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::sign::WriteableEcdsaChannelSigner;
+use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::events;
 use crate::events::{Event, EventHandler};
 use crate::util::atomic_counter::AtomicCounter;
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
@@ -43,7 +43,7 @@ use crate::chain;
 use crate::chain::{BestBlock, WatchedOutput};
 use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::sign::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
+use crate::sign::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, ecdsa::WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
 use crate::chain::onchaintx::{ClaimEvent, OnchainTxHandler};
 use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
 use crate::chain::Filter;
@@ -1462,7 +1462,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// to the commitment transaction being revoked, this will return a signed transaction, but
 	/// the signature will not be valid.
 	///
-	/// [`EcdsaChannelSigner::sign_justice_revoked_output`]: crate::sign::EcdsaChannelSigner::sign_justice_revoked_output
+	/// [`EcdsaChannelSigner::sign_justice_revoked_output`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_justice_revoked_output
 	/// [`Persist`]: crate::chain::chainmonitor::Persist
 	pub fn sign_to_local_justice_tx(&self, justice_tx: Transaction, input_idx: usize, value: u64, commitment_number: u64) -> Result<Transaction, ()> {
 		self.inner.lock().unwrap().sign_to_local_justice_tx(justice_tx, input_idx, value, commitment_number)
diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs
@@ -17,7 +17,7 @@ use bitcoin::network::constants::Network;
 use bitcoin::secp256k1::PublicKey;
 
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, MonitorEvent};
-use crate::sign::WriteableEcdsaChannelSigner;
+use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::chain::transaction::{OutPoint, TransactionData};
 
 use crate::prelude::*;
diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
@@ -30,7 +30,7 @@ use crate::ln::chan_utils::{self, ChannelTransactionParameters, HTLCOutputInComm
 use crate::chain::ClaimId;
 use crate::chain::chaininterface::{ConfirmationTarget, FeeEstimator, BroadcasterInterface, LowerBoundedFeeEstimator};
 use crate::chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
-use crate::sign::WriteableEcdsaChannelSigner;
+use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::chain::package::{PackageSolvingData, PackageTemplate};
 use crate::util::logger::Logger;
 use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, UpgradableRequired, Writer, Writeable, VecWriter};
diff --git 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::sign::WriteableEcdsaChannelSigner;
+use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::chain::onchaintx::{ExternalHTLCClaim, OnchainTxHandler};
 use crate::util::logger::Logger;
 use crate::util::ser::{Readable, Writer, Writeable, RequiredWrapper};
diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs
@@ -26,7 +26,8 @@ use crate::ln::chan_utils::{
 use crate::ln::features::ChannelTypeFeatures;
 use crate::ln::PaymentPreimage;
 use crate::prelude::*;
-use crate::sign::{EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner};
+use crate::sign::SignerProvider;
+use crate::sign::ecdsa::{EcdsaChannelSigner, WriteableEcdsaChannelSigner};
 use crate::sync::Mutex;
 use crate::util::logger::Logger;
 
@@ -263,8 +264,8 @@ pub enum BumpTransactionEvent {
 	/// an empty `pending_htlcs`), confirmation of the commitment transaction can be considered to
 	/// be not urgent.
 	///
-	/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
-	/// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::EcdsaChannelSigner::sign_holder_anchor_input
+	/// [`EcdsaChannelSigner`]: crate::sign::ecdsa::EcdsaChannelSigner
+	/// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_holder_anchor_input
 	/// [`build_anchor_input_witness`]: crate::ln::chan_utils::build_anchor_input_witness
 	ChannelClose {
 		/// The unique identifier for the claim of the anchor output in the commitment transaction.
@@ -317,8 +318,8 @@ pub enum BumpTransactionEvent {
 	/// longer able to commit external confirmed funds to the HTLC transaction or the fee committed
 	/// to the HTLC transaction is greater in value than the HTLCs being claimed.
 	///
-	/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
-	/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::EcdsaChannelSigner::sign_holder_htlc_transaction
+	/// [`EcdsaChannelSigner`]: crate::sign::ecdsa::EcdsaChannelSigner
+	/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::ecdsa::EcdsaChannelSigner::sign_holder_htlc_transaction
 	/// [`HTLCDescriptor::tx_input_witness`]: HTLCDescriptor::tx_input_witness
 	HTLCResolution {
 		/// The unique identifier for the claim of the HTLCs in the confirmed commitment
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -35,7 +35,8 @@ use crate::chain::BestBlock;
 use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator};
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS, CLOSED_CHANNEL_UPDATE_ID};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::sign::{EcdsaChannelSigner, WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
+use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
+use crate::sign::ecdsa::{EcdsaChannelSigner, WriteableEcdsaChannelSigner};
 use crate::events::ClosureReason;
 use crate::routing::gossip::NodeId;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
@@ -8107,7 +8108,7 @@ mod tests {
 		use bitcoin::hashes::hex::FromHex;
 		use bitcoin::hash_types::Txid;
 		use bitcoin::secp256k1::Message;
-		use crate::sign::EcdsaChannelSigner;
+		use crate::sign::ecdsa::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/channelmanager.rs b/lightning/src/ln/channelmanager.rs
@@ -55,7 +55,8 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
 use crate::ln::outbound_payment;
 use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs};
 use crate::ln::wire::Encode;
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, WriteableEcdsaChannelSigner};
+use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider};
+use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
 use crate::util::wakers::{Future, Notifier};
 use crate::util::scid_utils::fake_scid;
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
@@ -17,7 +17,8 @@ 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::sign::{ChannelSigner, EcdsaChannelSigner, EntropySource, SignerProvider};
+use crate::sign::{ChannelSigner, EntropySource, SignerProvider};
+use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
 use crate::ln::{ChannelId, 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, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase};
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
@@ -9,7 +9,7 @@
 
 //! Further functional tests which test blockchain reorganizations.
 
-use crate::sign::EcdsaChannelSigner;
+use crate::sign::ecdsa::EcdsaChannelSigner;
 use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance};
 use crate::chain::transaction::OutPoint;
 use crate::chain::chaininterface::{LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs
@@ -0,0 +1,176 @@
+//! Defines ECDSA-specific signer types.
+
+use bitcoin::blockdata::transaction::Transaction;
+
+use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+use bitcoin::secp256k1::ecdsa::Signature;
+use bitcoin::secp256k1;
+
+use crate::util::ser::Writeable;
+use crate::events::bump_transaction::HTLCDescriptor;
+use crate::ln::PaymentPreimage;
+use crate::ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction, CommitmentTransaction, ClosingTransaction};
+use crate::ln::msgs::UnsignedChannelAnnouncement;
+
+use crate::prelude::*;
+use crate::sign::ChannelSigner;
+
+/// 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.
+	///
+	/// Policy checks should be implemented in this function, including checking the amount
+	/// sent to us and checking the HTLCs.
+	///
+	/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
+	/// A validating signer should ensure that an HTLC output is removed only when the matching
+	/// preimage is provided, or when the value to holder is restored.
+	///
+	/// Note that all the relevant preimages will be provided, but there may also be additional
+	/// irrelevant or duplicate preimages.
+	//
+	// TODO: Document the things someone using this interface should enforce before signing.
+	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction,
+		preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>
+	) -> Result<(Signature, Vec<Signature>), ()>;
+	/// Validate the counterparty's revocation.
+	///
+	/// This is required in order for the signer to make sure that the state has moved
+	/// forward and it is safe to sign the next counterparty commitment.
+	fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>;
+	/// Creates a signature for a holder's commitment transaction and its claiming HTLC transactions.
+	///
+	/// This will be called
+	/// - with a non-revoked `commitment_tx`.
+	/// - with the latest `commitment_tx` when we initiate a force-close.
+	/// - with the previous `commitment_tx`, just to get claiming HTLC
+	///   signatures, if we are reacting to a [`ChannelMonitor`]
+	///   [replica](https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas)
+	///   that decided to broadcast before it had been updated to the latest `commitment_tx`.
+	///
+	/// This may be called multiple times for the same transaction.
+	///
+	/// An external signer implementation should check that the commitment has not been revoked.
+	///
+	/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
+	// TODO: Document the things someone using this interface should enforce before signing.
+	fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction,
+		secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
+	/// Same as [`sign_holder_commitment_and_htlcs`], but exists only for tests to get access to
+	/// holder commitment transactions which will be broadcasted later, after the channel has moved
+	/// on to a newer state. Thus, needs its own method as [`sign_holder_commitment_and_htlcs`] may
+	/// enforce that we only ever get called once.
+	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
+	fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction,
+		secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
+	/// Create a signature for the given input in a transaction spending an HTLC transaction output
+	/// or a commitment transaction `to_local` output when our counterparty broadcasts an old state.
+	///
+	/// A justice transaction may claim multiple outputs at the same time if timelocks are
+	/// similar, but only a signature for the input at index `input` should be signed for here.
+	/// It may be called multiple times for same output(s) if a fee-bump is needed with regards
+	/// to an upcoming timelock expiration.
+	///
+	/// Amount is value of the output spent by this input, committed to in the BIP 143 signature.
+	///
+	/// `per_commitment_key` is revocation secret which was provided by our counterparty when they
+	/// revoked the state which they eventually broadcast. It's not a _holder_ secret key and does
+	/// not allow the spending of any funds by itself (you need our holder `revocation_secret` to do
+	/// so).
+	fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64,
+		per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>
+	) -> Result<Signature, ()>;
+	/// Create a signature for the given input in a transaction spending a commitment transaction
+	/// HTLC output when our counterparty broadcasts an old state.
+	///
+	/// A justice transaction may claim multiple outputs at the same time if timelocks are
+	/// similar, but only a signature for the input at index `input` should be signed for here.
+	/// It may be called multiple times for same output(s) if a fee-bump is needed with regards
+	/// to an upcoming timelock expiration.
+	///
+	/// `amount` is the value of the output spent by this input, committed to in the BIP 143
+	/// signature.
+	///
+	/// `per_commitment_key` is revocation secret which was provided by our counterparty when they
+	/// revoked the state which they eventually broadcast. It's not a _holder_ secret key and does
+	/// not allow the spending of any funds by itself (you need our holder revocation_secret to do
+	/// so).
+	///
+	/// `htlc` holds HTLC elements (hash, timelock), thus changing the format of the witness script
+	/// (which is committed to in the BIP 143 signatures).
+	fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64,
+		per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment,
+		secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+	/// Computes the signature for a commitment transaction's HTLC output used as an input within
+	/// `htlc_tx`, which spends the commitment transaction at index `input`. The signature returned
+	/// must be be computed using [`EcdsaSighashType::All`]. Note that this should only be used to
+	/// sign HTLC transactions from channels supporting anchor outputs after all additional
+	/// inputs/outputs have been added to the transaction.
+	///
+	/// [`EcdsaSighashType::All`]: bitcoin::blockdata::transaction::EcdsaSighashType::All
+	fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize,
+		htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1<secp256k1::All>
+	) -> Result<Signature, ()>;
+	/// Create a signature for a claiming transaction for a HTLC output on a counterparty's commitment
+	/// transaction, either offered or received.
+	///
+	/// Such a transaction may claim multiples offered outputs at same time if we know the
+	/// preimage for each when we create it, but only the input at index `input` should be
+	/// signed for here. It may be called multiple times for same output(s) if a fee-bump is
+	/// needed with regards to an upcoming timelock expiration.
+	///
+	/// `witness_script` is either an offered or received script as defined in BOLT3 for HTLC
+	/// outputs.
+	///
+	/// `amount` is value of the output spent by this input, committed to in the BIP 143 signature.
+	///
+	/// `per_commitment_point` is the dynamic point corresponding to the channel state
+	/// detected onchain. It has been generated by our counterparty and is used to derive
+	/// channel state keys, which are then included in the witness script and committed to in the
+	/// BIP 143 signature.
+	fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64,
+		per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment,
+		secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+	/// Create a signature for a (proposed) closing transaction.
+	///
+	/// Note that, due to rounding, there may be one "missing" satoshi, and either party may have
+	/// chosen to forgo their output as dust.
+	fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction,
+		secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+	/// Computes the signature for a commitment transaction's anchor output used as an
+	/// input within `anchor_tx`, which spends the commitment transaction, at index `input`.
+	fn sign_holder_anchor_input(
+		&self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1<secp256k1::All>,
+	) -> Result<Signature, ()>;
+	/// Signs a channel announcement message with our funding key proving it comes from one of the
+	/// channel participants.
+	///
+	/// Channel announcements also require a signature from each node's network key. Our node
+	/// signature is computed through [`NodeSigner::sign_gossip_message`].
+	///
+	/// Note that if this fails or is rejected, the channel will not be publicly announced and
+	/// our counterparty may (though likely will not) close the channel on us for violating the
+	/// protocol.
+	///
+	/// [`NodeSigner::sign_gossip_message`]: crate::sign::NodeSigner::sign_gossip_message
+	fn sign_channel_announcement_with_funding_key(
+		&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>
+	) -> Result<Signature, ()>;
+}
+
+/// A writeable signer.
+///
+/// There will always be two instances of a signer per channel, one occupied by the
+/// [`ChannelManager`] and another by the channel's [`ChannelMonitor`].
+///
+/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
+/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
+pub trait WriteableEcdsaChannelSigner: EcdsaChannelSigner + Writeable {}
diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs
diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
