Skip to content

Commit

Permalink
Update signer docs to describe which methods are async
Browse files Browse the repository at this point in the history 
We should update the return types on the signing methods here as
well, but we should at least start by documenting which methods are
async and which are not.

Once we complete async support for `get_per_commitment_point`, we
can change the return types as most things in the channel signing
traits will be finalized.
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Dec 16, 2024
1 parent 47ca19d commit 6a8bf76
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 21 deletions.
54 changes: 41 additions & 13 deletions lightning/src/sign/ecdsa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ use crate::sign::{ChannelSigner, HTLCDescriptor};
/// 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.
///
/// Like [`ChannelSigner`], many of the methods allow errors to be returned to support async
/// signing. In such cases, the signing operation can be replayed by calling
/// [`ChannelManager::signer_unblocked`] or [`ChainMonitor::signer_unblocked`] (see individual
/// method documentation for which method should be called) once the result is ready, at which
/// point the channel operation will resume.
///
/// [`ChannelManager::signer_unblocked`]: crate::ln::channelmanager::ChannelManager::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
pub trait EcdsaChannelSigner: ChannelSigner {
/// Create a signature for a counterparty's commitment transaction and associated HTLC transactions.
///
Expand All @@ -39,8 +48,12 @@ pub trait EcdsaChannelSigner: ChannelSigner {
///
/// 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.
///
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelManager::signer_unblocked`] must be called.
///
/// [`ChannelManager::signer_unblocked`]: crate::ln::channelmanager::ChannelManager::signer_unblocked
fn sign_counterparty_commitment(
&self, commitment_tx: &CommitmentTransaction, inbound_htlc_preimages: Vec<PaymentPreimage>,
outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -58,18 +71,19 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
//
// TODO: Document the things someone using this interface should enforce before signing.
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_holder_commitment(
&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()>;
/// Same as [`sign_holder_commitment`], 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`] may enforce that we
/// only ever get called once.
///
/// This method is *not* async as it is intended only for testing purposes.
#[cfg(any(test, feature = "unsafe_revoked_tx_signing"))]
fn unsafe_sign_holder_commitment(
&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -92,9 +106,10 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_justice_revoked_output(
&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -121,9 +136,10 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_justice_revoked_htlc(
&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey,
htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -139,11 +155,12 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`EcdsaSighashType::All`]: bitcoin::sighash::EcdsaSighashType::All
/// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_holder_htlc_transaction(
&self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -169,9 +186,10 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_counterparty_htlc_transaction(
&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey,
htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>,
Expand All @@ -180,6 +198,12 @@ pub trait EcdsaChannelSigner: ChannelSigner {
///
/// Note that, due to rounding, there may be one "missing" satoshi, and either party may have
/// chosen to forgo their output as dust.
///
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelManager::signer_unblocked`] must be called.
///
/// [`ChannelManager::signer_unblocked`]: crate::ln::channelmanager::ChannelManager::signer_unblocked
fn sign_closing_transaction(
&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()>;
Expand All @@ -189,9 +213,10 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelMonitor::signer_unblocked`] must be called on its
/// monitor.
/// monitor or [`ChainMonitor::signer_unblocked`] called to attempt unblocking all monitors.
///
/// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked
/// [`ChainMonitor::signer_unblocked`]: crate::chain::chainmonitor::ChainMonitor::signer_unblocked
fn sign_holder_anchor_input(
&self, anchor_tx: &Transaction, input: usize, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()>;
Expand All @@ -201,9 +226,9 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// 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.
/// This method is *not* asynchronous, 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(
Expand All @@ -219,6 +244,9 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// spending the previous funding transaction's output
///
/// `input_value`: The value of the previous funding transaction output.
///
/// THis method is *not* asynchronous. If an `Err` is returned, the channel will be immediately
/// closed.
fn sign_splicing_funding_input(
&self, tx: &Transaction, input_index: usize, input_value: u64,
secp_ctx: &Secp256k1<secp256k1::All>,
Expand Down
35 changes: 27 additions & 8 deletions lightning/src/sign/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,18 +719,20 @@ impl HTLCDescriptor {
/// A trait to handle Lightning channel key material without concretizing the channel type or
/// the signature mechanism.
///
/// Several methods allow error types to be returned to support async signing. This feature
/// is not yet complete, and panics may occur in certain situations when returning errors
/// for these methods.
/// Several methods allow errors to be returned to support async signing. In such cases, the
/// signing operation can be replayed by calling [`ChannelManager::signer_unblocked`] once the
/// result is ready, at which point the channel operation will resume. Methods which allow for
/// async results are explicitly documented as such
///
/// [`ChannelManager::signer_unblocked`]: crate::ln::channelmanager::ChannelManager::signer_unblocked
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.
///
/// If the signer returns `Err`, then the user is responsible for either force-closing the channel
/// or calling `ChannelManager::signer_unblocked` once the signature is ready.
///
// TODO: link to `signer_unblocked` once the cfg flag is removed
/// This method is *not* asynchronous. This method is expected to always return `Ok`
/// immediately after we reconnect to peers, and returning an `Err` may lead to an immediate
/// `panic`. This method will be made asynchronous in a future release.
fn get_per_commitment_point(
&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<PublicKey, ()>;
Expand All @@ -743,7 +745,12 @@ pub trait ChannelSigner {
/// May be called more than once for the same index.
///
/// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
// TODO: return a Result so we can signal a validation error
///
/// An `Err` can be returned to signal that the signer is unavailable/cannot produce a valid
/// signature and should be retried later. Once the signer is ready to provide a signature after
/// previously returning an `Err`, [`ChannelManager::signer_unblocked`] must be called.
///
/// [`ChannelManager::signer_unblocked`]: crate::ln::channelmanager::ChannelManager::signer_unblocked
fn release_commitment_secret(&self, idx: u64) -> Result<[u8; 32], ()>;

/// Validate the counterparty's signatures on the holder commitment transaction and HTLCs.
Expand All @@ -759,6 +766,10 @@ pub trait ChannelSigner {
///
/// Note that all the relevant preimages will be provided, but there may also be additional
/// irrelevant or duplicate preimages.
///
/// This method is *not* asynchronous. If an `Err` is returned, the channel will be immediately
/// closed. If you wish to make this operation asynchronous, you should instead return `Ok(())`
/// and pause future signing operations until this validation completes.
fn validate_holder_commitment(
&self, holder_tx: &HolderCommitmentTransaction,
outbound_htlc_preimages: Vec<PaymentPreimage>,
Expand All @@ -768,14 +779,22 @@ pub trait ChannelSigner {
///
/// 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.
///
/// This method is *not* asynchronous. If an `Err` is returned, the channel will be immediately
/// closed. If you wish to make this operation asynchronous, you should instead return `Ok(())`
/// and pause future signing operations until this validation completes.
fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>;

/// Returns the holder's channel public keys and basepoints.
///
/// This method is *not* asynchronous. Instead, the value must be cached locally.
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.
///
/// This method is *not* asynchronous. Instead, the value must be cached locally.
fn channel_keys_id(&self) -> [u8; 32];

/// Set the counterparty static channel data, including basepoints,
Expand Down

0 comments on commit 6a8bf76

Please sign in to comment.