From 960437752eee93396f375630b48ad2dccafcbfd4 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 7 Dec 2024 00:37:27 +0000 Subject: [PATCH 1/6] Let `ChannelSigner` set `to_remote` scriptpubkey This allows the `to_remote` output to easily be changed according to the features of the channel, or the evolution of the LN specification. `to_remote` could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required. Builders of `CommitmentTransaction` now ask a `ChannelSigner` for the appropriate `to_remote` script to use, and then pass the script to the `CommitmentTransaction` constructor. External signers now provide the expected `to_remote` script to the `verify` call of `CommitmentTransaction`. --- lightning/src/chain/channelmonitor.rs | 14 ++++-- lightning/src/ln/chan_utils.rs | 60 +++++++++++++---------- lightning/src/ln/channel.rs | 19 +++++-- lightning/src/ln/functional_tests.rs | 14 +++++- lightning/src/sign/mod.rs | 22 ++++++++- lightning/src/util/test_channel_signer.rs | 13 ++++- 6 files changed, 101 insertions(+), 41 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 3f6bdc3f256..9662e1d0668 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1338,7 +1338,7 @@ impl ChannelMonitor { ChannelMonitor { inner: Mutex::new(imp) } } - pub(crate) fn new(secp_ctx: Secp256k1, keys: Signer, shutdown_script: Option, + pub(crate) fn new(secp_ctx: Secp256k1, mut keys: Signer, shutdown_script: Option, on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, ScriptBuf), channel_parameters: &ChannelTransactionParameters, holder_pays_commitment_tx_fee: bool, funding_redeemscript: ScriptBuf, channel_value_satoshis: u64, @@ -1347,10 +1347,10 @@ impl ChannelMonitor { best_block: BestBlock, counterparty_node_id: PublicKey, channel_id: ChannelId, ) -> ChannelMonitor { + keys.provide_channel_parameters(channel_parameters); + assert!(commitment_transaction_number_obscure_factor <= (1 << 48)); - let counterparty_payment_script = chan_utils::get_counterparty_payment_script( - &channel_parameters.channel_type_features, &keys.pubkeys().payment_point - ); + let counterparty_payment_script = keys.get_counterparty_payment_script(true); let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap(); let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint; @@ -3416,9 +3416,13 @@ impl ChannelMonitorImpl { &broadcaster_keys, &countersignatory_keys, &self.onchain_tx_handler.secp_ctx); let channel_parameters = &self.onchain_tx_handler.channel_transaction_parameters.as_counterparty_broadcastable(); + let counterparty_txout = TxOut { + script_pubkey: self.counterparty_payment_script.clone(), + value: Amount::from_sat(to_countersignatory_value), + }; CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, - to_broadcaster_value, to_countersignatory_value, broadcaster_funding_key, + to_broadcaster_value, counterparty_txout, broadcaster_funding_key, countersignatory_funding_key, keys, feerate_per_kw, &mut nondust_htlcs, channel_parameters) } diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index ae76308f0ce..3896eab96c3 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -20,7 +20,6 @@ use bitcoin::sighash::EcdsaSighashType; use bitcoin::transaction::Version; use bitcoin::hashes::{Hash, HashEngine}; -use bitcoin::hashes::hash160::Hash as Hash160; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::ripemd160::Hash as Ripemd160; use bitcoin::hash_types::Txid; @@ -1135,7 +1134,13 @@ impl HolderCommitmentTransaction { for _ in 0..htlcs.len() { counterparty_htlc_sigs.push(dummy_sig); } - let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, 0, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters.as_counterparty_broadcastable()); + let channel_parameters = channel_parameters.as_counterparty_broadcastable(); + let counterparty_payment_script = get_counterparty_payment_script(&channel_parameters.channel_type_features(), &channel_parameters.countersignatory_pubkeys().payment_point); + let counterparty_txout = TxOut { + script_pubkey: counterparty_payment_script, + value: Amount::ZERO, + }; + let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, counterparty_txout, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters); htlcs.sort_by_key(|htlc| htlc.0.transaction_output_index); HolderCommitmentTransaction { inner, @@ -1445,12 +1450,12 @@ impl CommitmentTransaction { /// Only include HTLCs that are above the dust limit for the channel. /// /// This is not exported to bindings users due to the generic though we likely should expose a version without - pub fn new_with_auxiliary_htlc_data(commitment_number: u64, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, broadcaster_funding_key: PublicKey, countersignatory_funding_key: PublicKey, keys: TxCreationKeys, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> CommitmentTransaction { + pub fn new_with_auxiliary_htlc_data(commitment_number: u64, to_broadcaster_value_sat: u64, to_countersignatory_txout: TxOut, broadcaster_funding_key: PublicKey, countersignatory_funding_key: PublicKey, keys: TxCreationKeys, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> CommitmentTransaction { let to_broadcaster_value_sat = Amount::from_sat(to_broadcaster_value_sat); - let to_countersignatory_value_sat = Amount::from_sat(to_countersignatory_value_sat); + let to_countersignatory_value_sat = to_countersignatory_txout.value; // Sort outputs and populate output indices while keeping track of the auxiliary data - let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, htlcs_with_aux, channel_parameters, &broadcaster_funding_key, &countersignatory_funding_key).unwrap(); + let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_txout, htlcs_with_aux, channel_parameters, &broadcaster_funding_key, &countersignatory_funding_key).unwrap(); let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(commitment_number, channel_parameters); let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); @@ -1479,11 +1484,15 @@ impl CommitmentTransaction { self } - fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result { + fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey, to_countersignatory_spk: ScriptBuf) -> Result { let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(self.commitment_number, channel_parameters); + let to_countersignatory_txout = TxOut { + script_pubkey: to_countersignatory_spk, + value: self.to_countersignatory_value_sat, + }; let mut htlcs_with_aux = self.htlcs.iter().map(|h| (h.clone(), ())).collect(); - let (outputs, _) = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, self.to_countersignatory_value_sat, &mut htlcs_with_aux, channel_parameters, broadcaster_funding_key, countersignatory_funding_key)?; + let (outputs, _) = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, to_countersignatory_txout, &mut htlcs_with_aux, channel_parameters, broadcaster_funding_key, countersignatory_funding_key)?; let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); let txid = transaction.compute_txid(); @@ -1507,23 +1516,14 @@ impl CommitmentTransaction { // - initial sorting of outputs / HTLCs in the constructor, in which case T is auxiliary data the // caller needs to have sorted together with the HTLCs so it can keep track of the output index // - building of a bitcoin transaction during a verify() call, in which case T is just () - fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result<(Vec, Vec), ()> { - let countersignatory_pubkeys = channel_parameters.countersignatory_pubkeys(); + fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_txout: TxOut, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result<(Vec, Vec), ()> { let contest_delay = channel_parameters.contest_delay(); let mut txouts: Vec<(TxOut, Option<&mut HTLCOutputInCommitment>)> = Vec::new(); - if to_countersignatory_value_sat > Amount::ZERO { - let script = if channel_parameters.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - get_to_countersignatory_with_anchors_redeemscript(&countersignatory_pubkeys.payment_point).to_p2wsh() - } else { - ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_pubkeys.payment_point.serialize()).into()) - }; + if to_countersignatory_txout.value > Amount::ZERO { txouts.push(( - TxOut { - script_pubkey: script.clone(), - value: to_countersignatory_value_sat, - }, + to_countersignatory_txout.clone(), None, )) } @@ -1555,7 +1555,7 @@ impl CommitmentTransaction { )); } - if to_countersignatory_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { + if to_countersignatory_txout.value > Amount::ZERO || !htlcs_with_aux.is_empty() { let anchor_script = get_anchor_redeemscript(countersignatory_funding_key); txouts.push(( TxOut { @@ -1680,14 +1680,14 @@ impl CommitmentTransaction { /// /// An external validating signer must call this method before signing /// or using the built transaction. - pub fn verify(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1) -> Result { + pub fn verify(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1, to_countersignatory_spk: ScriptBuf) -> Result { // This is the only field of the key cache that we trust let per_commitment_point = self.keys.per_commitment_point; let keys = TxCreationKeys::from_channel_static_keys(&per_commitment_point, broadcaster_keys, countersignatory_keys, secp_ctx); if keys != self.keys { return Err(()); } - let tx = self.internal_rebuild_transaction(&keys, channel_parameters, &broadcaster_keys.funding_pubkey, &countersignatory_keys.funding_pubkey)?; + let tx = self.internal_rebuild_transaction(&keys, channel_parameters, &broadcaster_keys.funding_pubkey, &countersignatory_keys.funding_pubkey, to_countersignatory_spk)?; if self.built.transaction != tx.transaction || self.built.txid != tx.txid { return Err(()); } @@ -1894,11 +1894,11 @@ pub fn get_commitment_transaction_number_obscure_factor( mod tests { use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys}; use crate::chain; - use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; + use crate::ln::chan_utils::{get_counterparty_payment_script, 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::sign::{ChannelSigner, SignerProvider}; - use bitcoin::{Network, Txid, ScriptBuf, CompressedPublicKey}; + use bitcoin::{Amount, TxOut, Network, Txid, ScriptBuf, CompressedPublicKey}; use bitcoin::hashes::Hash; use bitcoin::hex::FromHex; use crate::types::payment::PaymentHash; @@ -1957,12 +1957,20 @@ mod tests { } fn build(&mut self, to_broadcaster_sats: u64, to_countersignatory_sats: u64) -> CommitmentTransaction { + let channel_parameters = self.channel_parameters.as_holder_broadcastable(); + let counterparty_payment_script = get_counterparty_payment_script(&channel_parameters.channel_type_features(), &channel_parameters.countersignatory_pubkeys().payment_point); + let counterparty_txout = TxOut { + script_pubkey: counterparty_payment_script, + value: Amount::from_sat(to_countersignatory_sats), + }; CommitmentTransaction::new_with_auxiliary_htlc_data( - self.commitment_number, to_broadcaster_sats, to_countersignatory_sats, + self.commitment_number, + to_broadcaster_sats, + counterparty_txout, self.holder_funding_pubkey.clone(), self.counterparty_funding_pubkey.clone(), self.keys.clone(), self.feerate_per_kw, - &mut self.htlcs_with_aux, &self.channel_parameters.as_holder_broadcastable() + &mut self.htlcs_with_aux, &channel_parameters, ) } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 18e009e38ea..a502bfce4ca 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -15,7 +15,7 @@ use bitcoin::sighash; use bitcoin::sighash::EcdsaSighashType; use bitcoin::consensus::encode; use bitcoin::absolute::LockTime; -use bitcoin::Weight; +use bitcoin::{TxOut, Weight}; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; @@ -1609,8 +1609,7 @@ trait InitialRemoteCommitmentReceiver where SP::Target: SignerProvide let funding_txo_script = funding_redeemscript.to_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&context.get_holder_pubkeys().payment_point, &context.get_counterparty_pubkeys().payment_point, context.is_outbound()); let shutdown_script = context.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); - let mut monitor_signer = signer_provider.derive_channel_signer(context.channel_value_satoshis, context.channel_keys_id); - monitor_signer.provide_channel_parameters(&context.channel_transaction_parameters); + let monitor_signer = signer_provider.derive_channel_signer(context.channel_value_satoshis, context.channel_keys_id); let channel_monitor = ChannelMonitor::new(context.secp_ctx.clone(), monitor_signer, shutdown_script, context.get_holder_selected_contest_delay(), &context.destination_script, (funding_txo, funding_txo_script), @@ -3165,9 +3164,14 @@ impl ChannelContext where SP::Target: SignerProvider { let channel_parameters = if local { self.channel_transaction_parameters.as_holder_broadcastable() } else { self.channel_transaction_parameters.as_counterparty_broadcastable() }; + let counterparty_payment_script = self.holder_signer.as_ref().get_counterparty_payment_script(!local); + let counterparty_txout = TxOut { + script_pubkey: counterparty_payment_script, + value: Amount::from_sat(value_to_b as u64), + }; let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, value_to_a as u64, - value_to_b as u64, + counterparty_txout, funding_pubkey_a, funding_pubkey_b, keys.clone(), @@ -10863,6 +10867,7 @@ mod tests { use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys}; use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint}; use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; + use crate::sign::type_resolver::ChannelSignerType; use crate::util::logger::Logger; use crate::sync::Arc; use core::str::FromStr; @@ -10936,6 +10941,9 @@ mod tests { macro_rules! test_commitment { ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => { chan.context.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::only_static_remote_key(); + let mut holder_signer = keys_provider.derive_channel_signer(chan.context.channel_value_satoshis, chan.context.channel_keys_id); + holder_signer.provide_channel_parameters(&chan.context.channel_transaction_parameters); + chan.context.holder_signer = ChannelSignerType::Ecdsa(holder_signer); test_commitment_common!($counterparty_sig_hex, $sig_hex, $tx_hex, &ChannelTypeFeatures::only_static_remote_key(), $($remain)*); }; } @@ -10943,6 +10951,9 @@ mod tests { macro_rules! test_commitment_with_anchors { ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => { chan.context.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + let mut holder_signer = keys_provider.derive_channel_signer(chan.context.channel_value_satoshis, chan.context.channel_keys_id); + holder_signer.provide_channel_parameters(&chan.context.channel_transaction_parameters); + chan.context.holder_signer = ChannelSignerType::Ecdsa(holder_signer); test_commitment_common!($counterparty_sig_hex, $sig_hex, $tx_hex, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), $($remain)*); }; } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index ac43efe4499..799ea9c5065 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -766,11 +766,16 @@ fn test_update_fee_that_funder_cannot_afford() { |phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None } ).flatten().unwrap(); let local_chan_signer = local_chan.get_signer(); + let counterparty_payment_script = local_chan_signer.as_ref().get_counterparty_payment_script(true); + let counterparty_txout = TxOut { + script_pubkey: counterparty_payment_script, + value: Amount::from_sat(channel_value - push_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000), + }; let mut htlcs: Vec<(HTLCOutputInCommitment, ())> = vec![]; let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( INITIAL_COMMITMENT_NUMBER - 1, push_sats, - channel_value - push_sats - commit_tx_fee_msat(non_buffer_feerate + 4, 0, &channel_type_features) / 1000, + counterparty_txout, local_funding, remote_funding, commit_tx_keys.clone(), non_buffer_feerate + 4, @@ -1521,10 +1526,15 @@ fn test_fee_spike_violation_fails_htlc() { |phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None } ).flatten().unwrap(); let local_chan_signer = local_chan.get_signer(); + let counterparty_payment_script = local_chan_signer.as_ref().get_counterparty_payment_script(true); + let counterparty_txout = TxOut { + script_pubkey: counterparty_payment_script, + value: Amount::from_sat(local_chan_balance), + }; let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( commitment_number, 95000, - local_chan_balance, + counterparty_txout, local_funding, remote_funding, commit_tx_keys.clone(), feerate_per_kw, diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 94d68288022..e8cb53bc067 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -42,8 +42,8 @@ use crate::chain::transaction::OutPoint; use crate::crypto::utils::{hkdf_extract_expand_twice, sign, sign_with_aux_rand}; use crate::ln::chan_utils; use crate::ln::chan_utils::{ - get_revokeable_redeemscript, make_funding_redeemscript, ChannelPublicKeys, - ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, + get_counterparty_payment_script, get_revokeable_redeemscript, make_funding_redeemscript, + ChannelPublicKeys, ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, HTLCOutputInCommitment, HolderCommitmentTransaction, }; use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI; @@ -789,6 +789,14 @@ pub trait ChannelSigner { /// /// channel_parameters.is_populated() MUST be true. fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters); + + /// Returns the scriptpubkey that should be placed in the `to_remote` output of commitment + /// transactions. Assumes the signer has already been given the channel parameters via + /// `provide_channel_parameters`. + /// + /// If `to_self` is set, return the `to_remote` script pubkey for the counterparty's commitment + /// transaction, otherwise, for the local party's. + fn get_counterparty_payment_script(&self, to_self: bool) -> ScriptBuf; } /// Specifies the recipient of an invoice. @@ -1373,6 +1381,16 @@ impl ChannelSigner for InMemorySigner { assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); self.channel_parameters = Some(channel_parameters.clone()); } + + fn get_counterparty_payment_script(&self, to_self: bool) -> ScriptBuf { + let params = if to_self { + self.channel_parameters.as_ref().unwrap().as_counterparty_broadcastable() + } else { + self.channel_parameters.as_ref().unwrap().as_holder_broadcastable() + }; + let payment_point = ¶ms.countersignatory_pubkeys().payment_point; + get_counterparty_payment_script(params.channel_type_features(), payment_point) + } } const MISSING_PARAMS_ERR: &'static str = diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index f3ef4dc1557..7dcb722ef86 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -25,6 +25,7 @@ use crate::sync::{Mutex, Arc}; use bitcoin::transaction::Transaction; use bitcoin::hashes::Hash; use bitcoin::sighash; +use bitcoin::ScriptBuf; use bitcoin::sighash::EcdsaSighashType; use bitcoin::secp256k1; @@ -212,6 +213,10 @@ impl ChannelSigner for TestChannelSigner { fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { self.inner.provide_channel_parameters(channel_parameters) } + + fn get_counterparty_payment_script(&self, to_self: bool) -> ScriptBuf { + self.inner.get_counterparty_payment_script(to_self) + } } impl EcdsaChannelSigner for TestChannelSigner { @@ -415,16 +420,20 @@ impl Writeable for TestChannelSigner { impl TestChannelSigner { fn verify_counterparty_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + let counterparty_spk = self.get_counterparty_payment_script(true); commitment_tx.verify( &self.inner.get_channel_parameters().unwrap().as_counterparty_broadcastable(), - self.inner.counterparty_pubkeys().unwrap(), self.inner.pubkeys(), secp_ctx + self.inner.counterparty_pubkeys().unwrap(), self.inner.pubkeys(), secp_ctx, + counterparty_spk, ).expect("derived different per-tx keys or built transaction") } fn verify_holder_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + let counterparty_spk = self.get_counterparty_payment_script(false); commitment_tx.verify( &self.inner.get_channel_parameters().unwrap().as_holder_broadcastable(), - self.inner.pubkeys(), self.inner.counterparty_pubkeys().unwrap(), secp_ctx + self.inner.pubkeys(), self.inner.counterparty_pubkeys().unwrap(), secp_ctx, + counterparty_spk, ).expect("derived different per-tx keys or built transaction") } } From ab43c1a2485a212d920e75b3e71b6c43d33adda2 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Wed, 11 Dec 2024 00:45:30 +0000 Subject: [PATCH 2/6] Use new `ChannelSigner` API in `HolderCommitmentTransaction::dummy` --- lightning/src/ln/chan_utils.rs | 38 ++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 3896eab96c3..46d13eb3e9f 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1104,24 +1104,25 @@ impl_writeable_tlv_based!(HolderCommitmentTransaction, { impl HolderCommitmentTransaction { #[cfg(test)] pub fn dummy(htlcs: &mut Vec<(HTLCOutputInCommitment, ())>) -> Self { + use crate::sign::{InMemorySigner, ChannelSigner}; let secp_ctx = Secp256k1::new(); - let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let dummy_sec = SecretKey::from_slice(&[42; 32]).unwrap(); + let dummy_key = PublicKey::from_secret_key(&secp_ctx, &dummy_sec); let dummy_sig = sign(&secp_ctx, &secp256k1::Message::from_digest([42; 32]), &SecretKey::from_slice(&[42; 32]).unwrap()); - let keys = TxCreationKeys { - per_commitment_point: dummy_key.clone(), - revocation_key: RevocationKey::from_basepoint(&secp_ctx, &RevocationBasepoint::from(dummy_key), &dummy_key), - broadcaster_htlc_key: HtlcKey::from_basepoint(&secp_ctx, &HtlcBasepoint::from(dummy_key), &dummy_key), - countersignatory_htlc_key: HtlcKey::from_basepoint(&secp_ctx, &HtlcBasepoint::from(dummy_key), &dummy_key), - broadcaster_delayed_payment_key: DelayedPaymentKey::from_basepoint(&secp_ctx, &DelayedPaymentBasepoint::from(dummy_key), &dummy_key), - }; - let channel_pubkeys = ChannelPublicKeys { - funding_pubkey: dummy_key.clone(), - revocation_basepoint: RevocationBasepoint::from(dummy_key), - payment_point: dummy_key.clone(), - delayed_payment_basepoint: DelayedPaymentBasepoint::from(dummy_key.clone()), - htlc_basepoint: HtlcBasepoint::from(dummy_key.clone()) - }; + let mut signer = InMemorySigner::new( + &secp_ctx, + dummy_sec, + dummy_sec, + dummy_sec, + dummy_sec, + dummy_sec, + [42; 32], + 0, + [42; 32], + [42; 32], + ); + let channel_pubkeys = signer.pubkeys(); let channel_parameters = ChannelTransactionParameters { holder_pubkeys: channel_pubkeys.clone(), holder_selected_contest_delay: 0, @@ -1130,17 +1131,18 @@ impl HolderCommitmentTransaction { funding_outpoint: Some(chain::transaction::OutPoint { txid: Txid::all_zeros(), index: 0 }), channel_type_features: ChannelTypeFeatures::only_static_remote_key(), }; + signer.provide_channel_parameters(&channel_parameters); + let keys = TxCreationKeys::from_channel_static_keys(&dummy_key.clone(), &signer.pubkeys(), signer.counterparty_pubkeys().unwrap(), &secp_ctx); let mut counterparty_htlc_sigs = Vec::new(); for _ in 0..htlcs.len() { counterparty_htlc_sigs.push(dummy_sig); } - let channel_parameters = channel_parameters.as_counterparty_broadcastable(); - let counterparty_payment_script = get_counterparty_payment_script(&channel_parameters.channel_type_features(), &channel_parameters.countersignatory_pubkeys().payment_point); + let counterparty_payment_script = signer.get_counterparty_payment_script(true); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, value: Amount::ZERO, }; - let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, counterparty_txout, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters); + let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, counterparty_txout, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters.as_counterparty_broadcastable()); htlcs.sort_by_key(|htlc| htlc.0.transaction_output_index); HolderCommitmentTransaction { inner, From 9dc7e3e5b77fe38e913f4d404d0e0bbd03debfbb Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Wed, 11 Dec 2024 00:46:57 +0000 Subject: [PATCH 3/6] Add `InMemorySigner::overwrite_channel_parameters` Builds of `CommitmentTransaction` now query a `ChannelSigner` for the script pubkey to use in the `to_remote` output of the commitment transaction. So we need to overwrite the `ChannelTransactionParameters` of a `ChannelSigner` anytime we want to build a new commitment transaction with a different set of features. This is feature is only used in tests, so we cfg-gate it behind the test flag. --- lightning/src/ln/chan_utils.rs | 30 ++++++++++++++--------- lightning/src/ln/channel.rs | 19 +++++++------- lightning/src/sign/mod.rs | 8 ++++++ lightning/src/util/test_channel_signer.rs | 5 ++++ 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 46d13eb3e9f..71bc786993c 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1896,7 +1896,7 @@ pub fn get_commitment_transaction_number_obscure_factor( mod tests { use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys}; use crate::chain; - use crate::ln::chan_utils::{get_counterparty_payment_script, get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment}; + 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::sign::{ChannelSigner, SignerProvider}; @@ -1906,6 +1906,7 @@ mod tests { use crate::types::payment::PaymentHash; use bitcoin::PublicKey as BitcoinPublicKey; use crate::types::features::ChannelTypeFeatures; + use crate::util::test_channel_signer::TestChannelSigner; #[allow(unused_imports)] use crate::prelude::*; @@ -1919,6 +1920,7 @@ mod tests { htlcs_with_aux: Vec<(HTLCOutputInCommitment, ())>, channel_parameters: ChannelTransactionParameters, counterparty_pubkeys: ChannelPublicKeys, + signer: TestChannelSigner, } impl TestCommitmentTxBuilder { @@ -1927,15 +1929,13 @@ mod tests { let seed = [42; 32]; let network = Network::Testnet; let keys_provider = test_utils::TestKeysInterface::new(&seed, network); - let signer = keys_provider.derive_channel_signer(3000, keys_provider.generate_channel_keys_id(false, 1_000_000, 0)); + let mut signer = keys_provider.derive_channel_signer(3000, keys_provider.generate_channel_keys_id(false, 1_000_000, 0)); let counterparty_signer = keys_provider.derive_channel_signer(3000, keys_provider.generate_channel_keys_id(true, 1_000_000, 1)); - let delayed_payment_base = &signer.pubkeys().delayed_payment_basepoint; let per_commitment_secret = SecretKey::from_slice(&>::from_hex("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); - let htlc_basepoint = &signer.pubkeys().htlc_basepoint; - let holder_pubkeys = signer.pubkeys(); + let holder_pubkeys = signer.pubkeys().clone(); let counterparty_pubkeys = counterparty_signer.pubkeys().clone(); - let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint); + let keys = TxCreationKeys::from_channel_static_keys(&per_commitment_point, &holder_pubkeys, &counterparty_pubkeys, &secp_ctx); let channel_parameters = ChannelTransactionParameters { holder_pubkeys: holder_pubkeys.clone(), holder_selected_contest_delay: 0, @@ -1944,6 +1944,7 @@ mod tests { funding_outpoint: Some(chain::transaction::OutPoint { txid: Txid::all_zeros(), index: 0 }), channel_type_features: ChannelTypeFeatures::only_static_remote_key(), }; + signer.provide_channel_parameters(&channel_parameters); let htlcs_with_aux = Vec::new(); Self { @@ -1955,12 +1956,12 @@ mod tests { htlcs_with_aux, channel_parameters, counterparty_pubkeys, + signer, } } fn build(&mut self, to_broadcaster_sats: u64, to_countersignatory_sats: u64) -> CommitmentTransaction { - let channel_parameters = self.channel_parameters.as_holder_broadcastable(); - let counterparty_payment_script = get_counterparty_payment_script(&channel_parameters.channel_type_features(), &channel_parameters.countersignatory_pubkeys().payment_point); + let counterparty_payment_script = self.signer.get_counterparty_payment_script(false); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, value: Amount::from_sat(to_countersignatory_sats), @@ -1972,9 +1973,14 @@ mod tests { self.holder_funding_pubkey.clone(), self.counterparty_funding_pubkey.clone(), self.keys.clone(), self.feerate_per_kw, - &mut self.htlcs_with_aux, &channel_parameters, + &mut self.htlcs_with_aux, &self.channel_parameters.as_holder_broadcastable(), ) } + + fn overwrite_channel_features(&mut self, channel_type_features: ChannelTypeFeatures) { + self.channel_parameters.channel_type_features = channel_type_features; + self.signer.overwrite_channel_parameters(&self.channel_parameters); + } } #[test] @@ -1987,7 +1993,7 @@ mod tests { assert_eq!(tx.built.transaction.output[1].script_pubkey, bitcoin::address::Address::p2wpkh(&CompressedPublicKey(builder.counterparty_pubkeys.payment_point), Network::Testnet).script_pubkey()); // Generate broadcaster and counterparty outputs as well as two anchors - builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + builder.overwrite_channel_features(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); let tx = builder.build(1000, 2000); assert_eq!(tx.built.transaction.output.len(), 4); assert_eq!(tx.built.transaction.output[3].script_pubkey, get_to_countersignatory_with_anchors_redeemscript(&builder.counterparty_pubkeys.payment_point).to_p2wsh()); @@ -2017,7 +2023,7 @@ mod tests { }; // Generate broadcaster output and received and offered HTLC outputs, w/o anchors - builder.channel_parameters.channel_type_features = ChannelTypeFeatures::only_static_remote_key(); + builder.overwrite_channel_features(ChannelTypeFeatures::only_static_remote_key()); builder.htlcs_with_aux = vec![(received_htlc.clone(), ()), (offered_htlc.clone(), ())]; let tx = builder.build(3000, 0); let keys = &builder.keys.clone(); @@ -2030,7 +2036,7 @@ mod tests { "0020215d61bba56b19e9eadb6107f5a85d7f99c40f65992443f69229c290165bc00d"); // Generate broadcaster output and received and offered HTLC outputs, with anchors - builder.channel_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + builder.overwrite_channel_features(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); builder.htlcs_with_aux = vec![(received_htlc.clone(), ()), (offered_htlc.clone(), ())]; let tx = builder.build(3000, 0); assert_eq!(tx.built.transaction.output.len(), 5); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a502bfce4ca..bad75813491 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10271,6 +10271,14 @@ mod tests { } } + #[cfg(ldk_test_vectors)] + impl crate::ln::channel::ChannelContext<&Keys> { + fn overwrite_channel_features(&mut self, channel_type_features: ChannelTypeFeatures) { + self.channel_transaction_parameters.channel_type_features = channel_type_features; + self.holder_signer.as_mut_ecdsa().unwrap().overwrite_channel_parameters(&self.channel_transaction_parameters); + } + } + #[cfg(ldk_test_vectors)] fn public_from_secret_hex(secp_ctx: &Secp256k1, hex: &str) -> PublicKey { assert!(cfg!(not(feature = "grind_signatures"))); @@ -10867,7 +10875,6 @@ mod tests { use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys}; use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint}; use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; - use crate::sign::type_resolver::ChannelSignerType; use crate::util::logger::Logger; use crate::sync::Arc; use core::str::FromStr; @@ -10940,20 +10947,14 @@ mod tests { macro_rules! test_commitment { ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => { - chan.context.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::only_static_remote_key(); - let mut holder_signer = keys_provider.derive_channel_signer(chan.context.channel_value_satoshis, chan.context.channel_keys_id); - holder_signer.provide_channel_parameters(&chan.context.channel_transaction_parameters); - chan.context.holder_signer = ChannelSignerType::Ecdsa(holder_signer); + chan.context.overwrite_channel_features(ChannelTypeFeatures::only_static_remote_key()); test_commitment_common!($counterparty_sig_hex, $sig_hex, $tx_hex, &ChannelTypeFeatures::only_static_remote_key(), $($remain)*); }; } macro_rules! test_commitment_with_anchors { ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => { - chan.context.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); - let mut holder_signer = keys_provider.derive_channel_signer(chan.context.channel_value_satoshis, chan.context.channel_keys_id); - holder_signer.provide_channel_parameters(&chan.context.channel_transaction_parameters); - chan.context.holder_signer = ChannelSignerType::Ecdsa(holder_signer); + chan.context.overwrite_channel_features(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); test_commitment_common!($counterparty_sig_hex, $sig_hex, $tx_hex, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(), $($remain)*); }; } diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index e8cb53bc067..6fe765c58c3 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -1328,6 +1328,14 @@ impl InMemorySigner { witness_script.as_bytes(), ])) } + + #[cfg(test)] + pub(crate) fn overwrite_channel_parameters( + &mut self, channel_parameters: &ChannelTransactionParameters, + ) { + assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated"); + self.channel_parameters = Some(channel_parameters.clone()); + } } impl EntropySource for InMemorySigner { diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 7dcb722ef86..af50b209bce 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -162,6 +162,11 @@ impl TestChannelSigner { fn is_signer_available(&self, signer_op: SignerOp) -> bool { !self.get_enforcement_state().disabled_signer_ops.contains(&signer_op) } + + #[cfg(test)] + pub(crate) fn overwrite_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters) { + self.inner.overwrite_channel_parameters(channel_parameters) + } } impl ChannelSigner for TestChannelSigner { From 1155fa0c1be112e8f9040a20c3b55b6b8a7a9fb2 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 13 Dec 2024 02:15:43 +0000 Subject: [PATCH 4/6] Let `ChannelSigner` set `to_local` script pubkey This allows the `to_local` output to easily be changed according to the features of the channel, or the evolution of the LN specification. `to_local` could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required. Builders of `CommitmentTransaction` now ask a `ChannelSigner` for the appropriate `to_local` script pubkey to use, and then pass it to the `CommitmentTransaction` constructor. External signers now provide the expected `to_local` script pubkey to the `verify` call of `CommitmentTransaction`. --- lightning/src/chain/channelmonitor.rs | 52 +++++++++++----------- lightning/src/ln/chan_utils.rs | 54 +++++++++++++---------- lightning/src/ln/channel.rs | 7 ++- lightning/src/ln/functional_tests.rs | 14 +++++- lightning/src/sign/mod.rs | 46 +++++++++++++++++-- lightning/src/util/test_channel_signer.rs | 12 ++++- 6 files changed, 126 insertions(+), 59 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 9662e1d0668..69da11f6dc8 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3047,21 +3047,21 @@ impl ChannelMonitorImpl { // holder commitment transactions. if self.broadcasted_holder_revokable_script.is_some() { let holder_commitment_tx = if self.current_holder_commitment_tx.txid == confirmed_spend_txid { - Some(&self.current_holder_commitment_tx) + Some((&self.current_holder_commitment_tx, false)) } else if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx { if prev_holder_commitment_tx.txid == confirmed_spend_txid { - Some(prev_holder_commitment_tx) + Some((prev_holder_commitment_tx, true)) } else { None } } else { None }; - if let Some(holder_commitment_tx) = holder_commitment_tx { + if let Some((holder_commitment_tx, is_previous_tx)) = holder_commitment_tx { // Assume that the broadcasted commitment transaction confirmed in the current best // block. Even if not, its a reasonable metric for the bump criteria on the HTLC // transactions. - let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx, self.best_block.height); + let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx, self.best_block.height, is_previous_tx); let conf_target = self.closure_conf_target(); self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height, self.best_block.height, broadcaster, conf_target, fee_estimator, logger); } @@ -3100,7 +3100,7 @@ impl ChannelMonitorImpl { // assuming it gets confirmed in the next block. Sadly, we have code which considers // "not yet confirmed" things as discardable, so we cannot do that here. let (mut new_outpoints, _) = self.get_broadcasted_holder_claims( - &self.current_holder_commitment_tx, self.best_block.height, + &self.current_holder_commitment_tx, self.best_block.height, false, ); let unsigned_commitment_tx = self.onchain_tx_handler.get_unsigned_holder_commitment_tx(); let new_outputs = self.get_broadcasted_holder_watch_outputs( @@ -3416,13 +3416,18 @@ impl ChannelMonitorImpl { &broadcaster_keys, &countersignatory_keys, &self.onchain_tx_handler.secp_ctx); let channel_parameters = &self.onchain_tx_handler.channel_transaction_parameters.as_counterparty_broadcastable(); + let to_broadcaster_spk = self.onchain_tx_handler.signer.get_revokeable_spk(false, commitment_number, their_per_commitment_point, &self.onchain_tx_handler.secp_ctx); + let to_broadcaster_txout = TxOut { + script_pubkey: to_broadcaster_spk, + value: Amount::from_sat(to_broadcaster_value), + }; let counterparty_txout = TxOut { script_pubkey: self.counterparty_payment_script.clone(), value: Amount::from_sat(to_countersignatory_value), }; CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, - to_broadcaster_value, counterparty_txout, broadcaster_funding_key, + to_broadcaster_txout, counterparty_txout, broadcaster_funding_key, countersignatory_funding_key, keys, feerate_per_kw, &mut nondust_htlcs, channel_parameters) } @@ -3525,15 +3530,11 @@ impl ChannelMonitorImpl { let secret = self.get_secret(commitment_number).unwrap(); let per_commitment_key = ignore_error!(SecretKey::from_slice(&secret)); let per_commitment_point = PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key); - let revocation_pubkey = RevocationKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, &self.holder_revocation_basepoint, &per_commitment_point,); - let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, &self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &PublicKey::from_secret_key(&self.onchain_tx_handler.secp_ctx, &per_commitment_key)); - - let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key); - let revokeable_p2wsh = revokeable_redeemscript.to_p2wsh(); + let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(false, commitment_number, &per_commitment_point, &self.onchain_tx_handler.secp_ctx); // First, process non-htlc outputs (to_holder & to_counterparty) for (idx, outp) in tx.output.iter().enumerate() { - if outp.script_pubkey == revokeable_p2wsh { + if outp.script_pubkey == revokeable_spk { let revk_outp = RevokedOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, outp.value, self.counterparty_commitment_params.on_counterparty_tx_csv, self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx()); let justice_package = PackageTemplate::build_package( commitment_txid, idx as u32, @@ -3643,16 +3644,9 @@ impl ChannelMonitorImpl { } else { return (claimable_outpoints, to_counterparty_output_info); }; if let Some(transaction) = tx { - let revocation_pubkey = RevocationKey::from_basepoint( - &self.onchain_tx_handler.secp_ctx, &self.holder_revocation_basepoint, &per_commitment_point); - - let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, &self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &per_commitment_point); - - let revokeable_p2wsh = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, - self.counterparty_commitment_params.on_counterparty_tx_csv, - &delayed_key).to_p2wsh(); + let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(false, commitment_number, &per_commitment_point, &self.onchain_tx_handler.secp_ctx); for (idx, outp) in transaction.output.iter().enumerate() { - if outp.script_pubkey == revokeable_p2wsh { + if outp.script_pubkey == revokeable_spk { to_counterparty_output_info = Some((idx.try_into().expect("Can't have > 2^32 outputs"), outp.value)); } @@ -3742,11 +3736,15 @@ impl ChannelMonitorImpl { // Returns (1) `PackageTemplate`s that can be given to the OnchainTxHandler, so that the handler can // broadcast transactions claiming holder HTLC commitment outputs and (2) a holder revokable // script so we can detect whether a holder transaction has been seen on-chain. - fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32) -> (Vec, Option<(ScriptBuf, PublicKey, RevocationKey)>) { + fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32, is_previous_tx: bool) -> (Vec, Option<(ScriptBuf, PublicKey, RevocationKey)>) { let mut claim_requests = Vec::with_capacity(holder_tx.htlc_outputs.len()); - - let redeemscript = chan_utils::get_revokeable_redeemscript(&holder_tx.revocation_key, self.on_holder_tx_csv, &holder_tx.delayed_payment_key); - let broadcasted_holder_revokable_script = Some((redeemscript.to_p2wsh(), holder_tx.per_commitment_point.clone(), holder_tx.revocation_key.clone())); + let commitment_number = if is_previous_tx { + self.current_holder_commitment_number + 1 + } else { + self.current_holder_commitment_number + }; + let revokeable_spk = self.onchain_tx_handler.signer.get_revokeable_spk(true, commitment_number, &holder_tx.per_commitment_point, &self.onchain_tx_handler.secp_ctx); + let broadcasted_holder_revokable_script = Some((revokeable_spk, holder_tx.per_commitment_point.clone(), holder_tx.revocation_key.clone())); for &(ref htlc, _, _) in holder_tx.htlc_outputs.iter() { if let Some(transaction_output_index) = htlc.transaction_output_index { @@ -3813,7 +3811,7 @@ impl ChannelMonitorImpl { if self.current_holder_commitment_tx.txid == commitment_txid { is_holder_tx = true; log_info!(logger, "Got broadcast of latest holder commitment tx {}, searching for available HTLCs to claim", commitment_txid); - let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height); + let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height, false); let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx); append_onchain_update!(res, to_watch); fail_unbroadcast_htlcs!(self, "latest holder", commitment_txid, tx, height, @@ -3823,7 +3821,7 @@ impl ChannelMonitorImpl { if holder_tx.txid == commitment_txid { is_holder_tx = true; log_info!(logger, "Got broadcast of previous holder commitment tx {}, searching for available HTLCs to claim", commitment_txid); - let res = self.get_broadcasted_holder_claims(holder_tx, height); + let res = self.get_broadcasted_holder_claims(holder_tx, height, true); let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx); append_onchain_update!(res, to_watch); fail_unbroadcast_htlcs!(self, "previous holder", commitment_txid, tx, height, block_hash, diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 71bc786993c..6b9a87695e1 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -1137,12 +1137,17 @@ impl HolderCommitmentTransaction { for _ in 0..htlcs.len() { counterparty_htlc_sigs.push(dummy_sig); } + let broadcaster_payment_script = signer.get_revokeable_spk(false, 0, &keys.per_commitment_point, &secp_ctx); + let broadcaster_txout = TxOut { + script_pubkey: broadcaster_payment_script, + value: Amount::ZERO, + }; let counterparty_payment_script = signer.get_counterparty_payment_script(true); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, value: Amount::ZERO, }; - let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, counterparty_txout, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters.as_counterparty_broadcastable()); + let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, broadcaster_txout, counterparty_txout, dummy_key.clone(), dummy_key.clone(), keys, 0, htlcs, &channel_parameters.as_counterparty_broadcastable()); htlcs.sort_by_key(|htlc| htlc.0.transaction_output_index); HolderCommitmentTransaction { inner, @@ -1452,12 +1457,12 @@ impl CommitmentTransaction { /// Only include HTLCs that are above the dust limit for the channel. /// /// This is not exported to bindings users due to the generic though we likely should expose a version without - pub fn new_with_auxiliary_htlc_data(commitment_number: u64, to_broadcaster_value_sat: u64, to_countersignatory_txout: TxOut, broadcaster_funding_key: PublicKey, countersignatory_funding_key: PublicKey, keys: TxCreationKeys, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> CommitmentTransaction { - let to_broadcaster_value_sat = Amount::from_sat(to_broadcaster_value_sat); + pub fn new_with_auxiliary_htlc_data(commitment_number: u64, to_broadcaster_txout: TxOut, to_countersignatory_txout: TxOut, broadcaster_funding_key: PublicKey, countersignatory_funding_key: PublicKey, keys: TxCreationKeys, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> CommitmentTransaction { + let to_broadcaster_value_sat = to_broadcaster_txout.value; let to_countersignatory_value_sat = to_countersignatory_txout.value; // Sort outputs and populate output indices while keeping track of the auxiliary data - let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_txout, htlcs_with_aux, channel_parameters, &broadcaster_funding_key, &countersignatory_funding_key).unwrap(); + let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_txout, to_countersignatory_txout, htlcs_with_aux, channel_parameters, &broadcaster_funding_key, &countersignatory_funding_key).unwrap(); let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(commitment_number, channel_parameters); let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); @@ -1486,15 +1491,19 @@ impl CommitmentTransaction { self } - fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey, to_countersignatory_spk: ScriptBuf) -> Result { + fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey, to_broadcaster_spk: ScriptBuf, to_countersignatory_spk: ScriptBuf) -> Result { let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(self.commitment_number, channel_parameters); + let to_broadcaster_txout = TxOut { + script_pubkey: to_broadcaster_spk, + value: self.to_broadcaster_value_sat, + }; let to_countersignatory_txout = TxOut { script_pubkey: to_countersignatory_spk, value: self.to_countersignatory_value_sat, }; let mut htlcs_with_aux = self.htlcs.iter().map(|h| (h.clone(), ())).collect(); - let (outputs, _) = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, to_countersignatory_txout, &mut htlcs_with_aux, channel_parameters, broadcaster_funding_key, countersignatory_funding_key)?; + let (outputs, _) = Self::internal_build_outputs(keys, to_broadcaster_txout, to_countersignatory_txout, &mut htlcs_with_aux, channel_parameters, broadcaster_funding_key, countersignatory_funding_key)?; let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs); let txid = transaction.compute_txid(); @@ -1518,9 +1527,7 @@ impl CommitmentTransaction { // - initial sorting of outputs / HTLCs in the constructor, in which case T is auxiliary data the // caller needs to have sorted together with the HTLCs so it can keep track of the output index // - building of a bitcoin transaction during a verify() call, in which case T is just () - fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_txout: TxOut, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result<(Vec, Vec), ()> { - let contest_delay = channel_parameters.contest_delay(); - + fn internal_build_outputs(keys: &TxCreationKeys, to_broadcaster_txout: TxOut, to_countersignatory_txout: TxOut, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey) -> Result<(Vec, Vec), ()> { let mut txouts: Vec<(TxOut, Option<&mut HTLCOutputInCommitment>)> = Vec::new(); if to_countersignatory_txout.value > Amount::ZERO { @@ -1530,23 +1537,15 @@ impl CommitmentTransaction { )) } - if to_broadcaster_value_sat > Amount::ZERO { - let redeem_script = get_revokeable_redeemscript( - &keys.revocation_key, - contest_delay, - &keys.broadcaster_delayed_payment_key, - ); + if to_broadcaster_txout.value > Amount::ZERO { txouts.push(( - TxOut { - script_pubkey: redeem_script.to_p2wsh(), - value: to_broadcaster_value_sat, - }, + to_broadcaster_txout.clone(), None, )); } if channel_parameters.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - if to_broadcaster_value_sat > Amount::ZERO || !htlcs_with_aux.is_empty() { + if to_broadcaster_txout.value > Amount::ZERO || !htlcs_with_aux.is_empty() { let anchor_script = get_anchor_redeemscript(broadcaster_funding_key); txouts.push(( TxOut { @@ -1682,14 +1681,14 @@ impl CommitmentTransaction { /// /// An external validating signer must call this method before signing /// or using the built transaction. - pub fn verify(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1, to_countersignatory_spk: ScriptBuf) -> Result { + pub fn verify(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1, to_broadcaster_spk: ScriptBuf, to_countersignatory_spk: ScriptBuf) -> Result { // This is the only field of the key cache that we trust let per_commitment_point = self.keys.per_commitment_point; let keys = TxCreationKeys::from_channel_static_keys(&per_commitment_point, broadcaster_keys, countersignatory_keys, secp_ctx); if keys != self.keys { return Err(()); } - let tx = self.internal_rebuild_transaction(&keys, channel_parameters, &broadcaster_keys.funding_pubkey, &countersignatory_keys.funding_pubkey, to_countersignatory_spk)?; + let tx = self.internal_rebuild_transaction(&keys, channel_parameters, &broadcaster_keys.funding_pubkey, &countersignatory_keys.funding_pubkey, to_broadcaster_spk, to_countersignatory_spk)?; if self.built.transaction != tx.transaction || self.built.txid != tx.txid { return Err(()); } @@ -1897,7 +1896,7 @@ mod tests { use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys}; use crate::chain; 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 bitcoin::secp256k1::{self, PublicKey, SecretKey, Secp256k1}; use crate::util::test_utils; use crate::sign::{ChannelSigner, SignerProvider}; use bitcoin::{Amount, TxOut, Network, Txid, ScriptBuf, CompressedPublicKey}; @@ -1921,6 +1920,7 @@ mod tests { channel_parameters: ChannelTransactionParameters, counterparty_pubkeys: ChannelPublicKeys, signer: TestChannelSigner, + secp_ctx: Secp256k1::, } impl TestCommitmentTxBuilder { @@ -1957,10 +1957,16 @@ mod tests { channel_parameters, counterparty_pubkeys, signer, + secp_ctx, } } fn build(&mut self, to_broadcaster_sats: u64, to_countersignatory_sats: u64) -> CommitmentTransaction { + let broadcaster_payment_script = self.signer.get_revokeable_spk(true, self.commitment_number, &self.keys.per_commitment_point, &self.secp_ctx); + let broadcaster_txout = TxOut { + script_pubkey: broadcaster_payment_script, + value: Amount::from_sat(to_broadcaster_sats), + }; let counterparty_payment_script = self.signer.get_counterparty_payment_script(false); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, @@ -1968,7 +1974,7 @@ mod tests { }; CommitmentTransaction::new_with_auxiliary_htlc_data( self.commitment_number, - to_broadcaster_sats, + broadcaster_txout, counterparty_txout, self.holder_funding_pubkey.clone(), self.counterparty_funding_pubkey.clone(), diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index bad75813491..6351a5e0fa1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3164,13 +3164,18 @@ impl ChannelContext where SP::Target: SignerProvider { let channel_parameters = if local { self.channel_transaction_parameters.as_holder_broadcastable() } else { self.channel_transaction_parameters.as_counterparty_broadcastable() }; + let broadcaster_payment_script = self.holder_signer.as_ref().get_revokeable_spk(local, commitment_number, &keys.per_commitment_point, &self.secp_ctx); + let broadcaster_txout = TxOut { + script_pubkey: broadcaster_payment_script, + value: Amount::from_sat(value_to_a as u64), + }; let counterparty_payment_script = self.holder_signer.as_ref().get_counterparty_payment_script(!local); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, value: Amount::from_sat(value_to_b as u64), }; let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, - value_to_a as u64, + broadcaster_txout, counterparty_txout, funding_pubkey_a, funding_pubkey_b, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 799ea9c5065..7de424ee345 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -766,6 +766,11 @@ fn test_update_fee_that_funder_cannot_afford() { |phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None } ).flatten().unwrap(); let local_chan_signer = local_chan.get_signer(); + let broadcaster_payment_script = local_chan_signer.as_ref().get_revokeable_spk(false, INITIAL_COMMITMENT_NUMBER - 1, &commit_tx_keys.per_commitment_point, &secp_ctx); + let broadcaster_txout = TxOut { + script_pubkey: broadcaster_payment_script, + value: Amount::from_sat(push_sats), + }; let counterparty_payment_script = local_chan_signer.as_ref().get_counterparty_payment_script(true); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, @@ -774,7 +779,7 @@ fn test_update_fee_that_funder_cannot_afford() { let mut htlcs: Vec<(HTLCOutputInCommitment, ())> = vec![]; let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( INITIAL_COMMITMENT_NUMBER - 1, - push_sats, + broadcaster_txout, counterparty_txout, local_funding, remote_funding, commit_tx_keys.clone(), @@ -1526,6 +1531,11 @@ fn test_fee_spike_violation_fails_htlc() { |phase| if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None } ).flatten().unwrap(); let local_chan_signer = local_chan.get_signer(); + let broadcaster_payment_script = local_chan_signer.as_ref().get_revokeable_spk(false, commitment_number, &commit_tx_keys.per_commitment_point, &secp_ctx); + let broadcaster_txout = TxOut { + script_pubkey: broadcaster_payment_script, + value: Amount::from_sat(95000), + }; let counterparty_payment_script = local_chan_signer.as_ref().get_counterparty_payment_script(true); let counterparty_txout = TxOut { script_pubkey: counterparty_payment_script, @@ -1533,7 +1543,7 @@ fn test_fee_spike_violation_fails_htlc() { }; let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data( commitment_number, - 95000, + broadcaster_txout, counterparty_txout, local_funding, remote_funding, commit_tx_keys.clone(), diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 6fe765c58c3..097b7c21486 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -65,6 +65,7 @@ use crate::crypto::chacha20::ChaCha20; use crate::io::{self, Error}; use crate::ln::msgs::DecodeError; use crate::prelude::*; +use crate::sign::chan_utils::TxCreationKeys; use crate::sign::ecdsa::EcdsaChannelSigner; #[cfg(taproot)] use crate::sign::taproot::TaprootChannelSigner; @@ -790,13 +791,28 @@ pub trait ChannelSigner { /// channel_parameters.is_populated() MUST be true. fn provide_channel_parameters(&mut self, channel_parameters: &ChannelTransactionParameters); - /// Returns the scriptpubkey that should be placed in the `to_remote` output of commitment - /// transactions. Assumes the signer has already been given the channel parameters via + /// Returns the script pubkey that should be placed in the `to_remote` output of commitment + /// transactions. + /// + /// Assumes the signer has already been given the channel parameters via /// `provide_channel_parameters`. /// - /// If `to_self` is set, return the `to_remote` script pubkey for the counterparty's commitment + /// If `to_self` is set, return the `to_remote` script pubkey for the remote party's commitment /// transaction, otherwise, for the local party's. fn get_counterparty_payment_script(&self, to_self: bool) -> ScriptBuf; + + /// Returns the script pubkey that should be placed in the `to_local` output of commitment + /// transactions, and in the output of second level HTLC transactions. + /// + /// Assumes the signer has already been given the channel parameters via + /// `provide_channel_parameters`. + /// + /// If `to_self` is set, return the revokeable script pubkey for local party's + /// commitment / htlc transaction, otherwise, for the remote party's. + fn get_revokeable_spk( + &self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey, + secp_ctx: &Secp256k1, + ) -> ScriptBuf; } /// Specifies the recipient of an invoice. @@ -1399,6 +1415,30 @@ impl ChannelSigner for InMemorySigner { let payment_point = ¶ms.countersignatory_pubkeys().payment_point; get_counterparty_payment_script(params.channel_type_features(), payment_point) } + + fn get_revokeable_spk( + &self, to_self: bool, _commitment_number: u64, per_commitment_point: &PublicKey, + secp_ctx: &Secp256k1, + ) -> ScriptBuf { + let params = if to_self { + self.channel_parameters.as_ref().unwrap().as_holder_broadcastable() + } else { + self.channel_parameters.as_ref().unwrap().as_counterparty_broadcastable() + }; + let contest_delay = params.contest_delay(); + let keys = TxCreationKeys::from_channel_static_keys( + per_commitment_point, + params.broadcaster_pubkeys(), + params.countersignatory_pubkeys(), + secp_ctx, + ); + get_revokeable_redeemscript( + &keys.revocation_key, + contest_delay, + &keys.broadcaster_delayed_payment_key, + ) + .to_p2wsh() + } } const MISSING_PARAMS_ERR: &'static str = diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index af50b209bce..ab483185f28 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -222,6 +222,10 @@ impl ChannelSigner for TestChannelSigner { fn get_counterparty_payment_script(&self, to_self: bool) -> ScriptBuf { self.inner.get_counterparty_payment_script(to_self) } + + fn get_revokeable_spk(&self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) -> ScriptBuf { + self.inner.get_revokeable_spk(to_self, commitment_number, per_commitment_point, secp_ctx) + } } impl EcdsaChannelSigner for TestChannelSigner { @@ -424,20 +428,24 @@ impl Writeable for TestChannelSigner { } impl TestChannelSigner { - fn verify_counterparty_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + fn verify_counterparty_commitment_tx<'a>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + let broadcaster_spk = self.get_revokeable_spk(false, commitment_tx.commitment_number(), &commitment_tx.per_commitment_point(), secp_ctx); let counterparty_spk = self.get_counterparty_payment_script(true); commitment_tx.verify( &self.inner.get_channel_parameters().unwrap().as_counterparty_broadcastable(), self.inner.counterparty_pubkeys().unwrap(), self.inner.pubkeys(), secp_ctx, + broadcaster_spk, counterparty_spk, ).expect("derived different per-tx keys or built transaction") } - fn verify_holder_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + fn verify_holder_commitment_tx<'a>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1) -> TrustedCommitmentTransaction<'a> { + let broadcaster_spk = self.get_revokeable_spk(true, commitment_tx.commitment_number(), &commitment_tx.per_commitment_point(), secp_ctx); let counterparty_spk = self.get_counterparty_payment_script(false); commitment_tx.verify( &self.inner.get_channel_parameters().unwrap().as_holder_broadcastable(), self.inner.pubkeys(), self.inner.counterparty_pubkeys().unwrap(), secp_ctx, + broadcaster_spk, counterparty_spk, ).expect("derived different per-tx keys or built transaction") } From d3181c5f9f629a86484f4738aa7457b5d1b9fe0e Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 13 Dec 2024 07:56:27 +0000 Subject: [PATCH 5/6] Let `ChannelSigner` set htlc tx script pubkey This allows the htlc tx output to easily be changed according to the features of the channel, or the evolution of the LN specification. The output could even be set to completely arbitrary scripts if compatibility with the formal LN spec is not required. Builders of htlc transactions now ask a `ChannelSigner` for the appropriate revokeable script pubkey to use, and then pass it to the htlc transaction constructors. --- lightning/src/chain/onchaintx.rs | 8 +++-- lightning/src/events/bump_transaction.rs | 16 ++++++---- lightning/src/ln/chan_utils.rs | 26 +++++++--------- lightning/src/ln/channel.rs | 12 ++++---- lightning/src/ln/monitor_tests.rs | 12 ++++++-- lightning/src/sign/mod.rs | 36 +++++++---------------- lightning/src/util/test_channel_signer.rs | 3 +- 7 files changed, 54 insertions(+), 59 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index cf7f8d7bfe0..c98121ba1e6 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -1210,8 +1210,9 @@ impl OnchainTxHandler { .find(|(_, htlc)| htlc.transaction_output_index.unwrap() == outp.vout) .unwrap(); let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[htlc_idx]; + let revokeable_spk = self.signer.get_revokeable_spk(true, holder_commitment.commitment_number(), &holder_commitment.per_commitment_point(), &self.secp_ctx); let mut htlc_tx = trusted_tx.build_unsigned_htlc_tx( - &self.channel_transaction_parameters.as_holder_broadcastable(), htlc_idx, preimage, + htlc_idx, preimage, revokeable_spk, ); let htlc_descriptor = HTLCDescriptor { @@ -1295,7 +1296,7 @@ mod tests { }; use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint}; use crate::ln::functional_test_utils::create_dummy_block; - use crate::sign::InMemorySigner; + use crate::sign::{ChannelSigner, InMemorySigner}; use crate::types::payment::{PaymentHash, PaymentPreimage}; use crate::util::test_utils::{TestBroadcaster, TestFeeEstimator, TestLogger}; @@ -1307,7 +1308,7 @@ mod tests { #[test] fn test_broadcast_height() { let secp_ctx = Secp256k1::new(); - let signer = InMemorySigner::new( + let mut signer = InMemorySigner::new( &secp_ctx, SecretKey::from_slice(&[41; 32]).unwrap(), SecretKey::from_slice(&[41; 32]).unwrap(), @@ -1356,6 +1357,7 @@ mod tests { funding_outpoint: Some(funding_outpoint), channel_type_features: ChannelTypeFeatures::only_static_remote_key(), }; + signer.provide_channel_parameters(&chan_params); // Create an OnchainTxHandler for a commitment containing HTLCs with CLTV expiries of 0, 1, // and 2 blocks. diff --git a/lightning/src/events/bump_transaction.rs b/lightning/src/events/bump_transaction.rs index 3acb2145e5b..e4f8574c74f 100644 --- a/lightning/src/events/bump_transaction.rs +++ b/lightning/src/events/bump_transaction.rs @@ -26,7 +26,7 @@ use crate::ln::chan_utils::{ }; use crate::prelude::*; use crate::sign::{ - ChannelDerivationParameters, HTLCDescriptor, SignerProvider, P2WPKH_WITNESS_WEIGHT + ChannelDerivationParameters, ChannelSigner, HTLCDescriptor, SignerProvider, P2WPKH_WITNESS_WEIGHT, }; use crate::sign::ecdsa::EcdsaChannelSigner; use crate::sync::Mutex; @@ -728,6 +728,7 @@ where output: vec![], }; let mut must_spend = Vec::with_capacity(htlc_descriptors.len()); + let mut signers_and_revokeable_spks = BTreeMap::new(); for htlc_descriptor in htlc_descriptors { let htlc_input = htlc_descriptor.unsigned_tx_input(); must_spend.push(Input { @@ -740,7 +741,13 @@ where }, }); htlc_tx.input.push(htlc_input); - let htlc_output = htlc_descriptor.tx_output(&self.secp); + let revokeable_spk = signers_and_revokeable_spks.entry(htlc_descriptor.channel_derivation_parameters.keys_id) + .or_insert_with(|| { + let signer = htlc_descriptor.derive_channel_signer(&self.signer_provider); + let revokeable_spk = signer.get_revokeable_spk(true, htlc_descriptor.per_commitment_number, &htlc_descriptor.per_commitment_point, &self.secp); + (signer, revokeable_spk) + }).1.clone(); + let htlc_output = htlc_descriptor.tx_output(revokeable_spk); htlc_tx.output.push(htlc_output); } @@ -789,10 +796,9 @@ where log_debug!(self.logger, "Signing HTLC transaction {}", htlc_psbt.unsigned_tx.compute_txid()); htlc_tx = self.utxo_source.sign_psbt(htlc_psbt)?; - let mut signers = BTreeMap::new(); for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() { - let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id) - .or_insert_with(|| htlc_descriptor.derive_channel_signer(&self.signer_provider)); + // Unwrap because we derived the corresponding signers for all htlc descriptors further above + let signer = &signers_and_revokeable_spks.get(&htlc_descriptor.channel_derivation_parameters.keys_id).unwrap().0; let htlc_sig = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?; let witness_script = htlc_descriptor.witness_script(&self.secp); htlc_tx.input[idx].witness = htlc_descriptor.tx_input_witness(&htlc_sig, &witness_script); diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 6b9a87695e1..23533ab1dce 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -704,13 +704,12 @@ pub(crate) fn make_funding_redeemscript_from_slices(broadcaster_funding_key: &[u /// /// Panics if htlc.transaction_output_index.is_none() (as such HTLCs do not appear in the /// commitment transaction). -pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, broadcaster_delayed_payment_key: &DelayedPaymentKey, revocation_key: &RevocationKey) -> Transaction { - let txins= vec![build_htlc_input(commitment_txid, htlc, channel_type_features)]; +pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, revokeable_spk: ScriptBuf) -> Transaction { + let txins = vec![build_htlc_input(commitment_txid, htlc, channel_type_features)]; let mut txouts: Vec = Vec::new(); txouts.push(build_htlc_output( - feerate_per_kw, contest_delay, htlc, channel_type_features, - broadcaster_delayed_payment_key, revocation_key + feerate_per_kw, htlc, channel_type_features, revokeable_spk, )); Transaction { @@ -734,7 +733,7 @@ pub(crate) fn build_htlc_input(commitment_txid: &Txid, htlc: &HTLCOutputInCommit } pub(crate) fn build_htlc_output( - feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, broadcaster_delayed_payment_key: &DelayedPaymentKey, revocation_key: &RevocationKey + feerate_per_kw: u32, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, revokeable_spk: ScriptBuf ) -> TxOut { let weight = if htlc.offered { htlc_timeout_tx_weight(channel_type_features) @@ -749,7 +748,7 @@ pub(crate) fn build_htlc_output( }; TxOut { - script_pubkey: get_revokeable_redeemscript(revocation_key, contest_delay, broadcaster_delayed_payment_key).to_p2wsh(), + script_pubkey: revokeable_spk, value: output_value, } } @@ -1740,8 +1739,7 @@ impl<'a> TrustedCommitmentTransaction<'a> { /// /// This function is only valid in the holder commitment context, it always uses EcdsaSighashType::All. pub fn get_htlc_sigs( - &self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, - entropy_source: &ES, secp_ctx: &Secp256k1, + &self, htlc_base_key: &SecretKey, entropy_source: &ES, secp_ctx: &Secp256k1, revokeable_spk: ScriptBuf, ) -> Result, ()> where ES::Target: EntropySource { let inner = self.inner; let keys = &inner.keys; @@ -1751,7 +1749,7 @@ impl<'a> TrustedCommitmentTransaction<'a> { for this_htlc in inner.htlcs.iter() { assert!(this_htlc.transaction_output_index.is_some()); - let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &self.channel_type_features, &keys.broadcaster_delayed_payment_key, &keys.revocation_key); + let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, &this_htlc, &self.channel_type_features, revokeable_spk.clone()); let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &self.channel_type_features, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key); @@ -1762,11 +1760,7 @@ impl<'a> TrustedCommitmentTransaction<'a> { } /// Builds the second-level holder HTLC transaction for the HTLC with index `htlc_index`. - pub(crate) fn build_unsigned_htlc_tx( - &self, channel_parameters: &DirectedChannelTransactionParameters, htlc_index: usize, - preimage: &Option, - ) -> Transaction { - let keys = &self.inner.keys; + pub(crate) fn build_unsigned_htlc_tx(&self, htlc_index: usize, preimage: &Option, revokeable_spk: ScriptBuf) -> Transaction { let this_htlc = &self.inner.htlcs[htlc_index]; assert!(this_htlc.transaction_output_index.is_some()); // if we don't have preimage for an HTLC-Success, we can't generate an HTLC transaction. @@ -1775,8 +1769,8 @@ impl<'a> TrustedCommitmentTransaction<'a> { if this_htlc.offered && preimage.is_some() { unreachable!(); } build_htlc_transaction( - &self.inner.built.txid, self.inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, - &self.channel_type_features, &keys.broadcaster_delayed_payment_key, &keys.revocation_key + &self.inner.built.txid, self.inner.feerate_per_kw, &this_htlc, + &self.channel_type_features, revokeable_spk, ) } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6351a5e0fa1..a2858d6d71a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5130,11 +5130,11 @@ impl Channel where let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len()); let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len()); + let revokeable_spk = self.context.holder_signer.as_ref().get_revokeable_spk(true, commitment_stats.tx.commitment_number(), &commitment_stats.tx.per_commitment_point(), &self.context.secp_ctx); for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() { if let Some(_) = htlc.transaction_output_index { let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw, - self.context.get_counterparty_selected_contest_delay().unwrap(), &htlc, &self.context.channel_type, - &keys.broadcaster_delayed_payment_key, &keys.revocation_key); + &htlc, &self.context.channel_type, revokeable_spk.clone()); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &keys); let htlc_sighashtype = if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All }; @@ -8103,6 +8103,7 @@ impl Channel where ).map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?; signature = res.0; htlc_signatures = res.1; + let revokeable_spk = ecdsa.get_revokeable_spk(false, commitment_stats.tx.commitment_number(), &commitment_stats.tx.per_commitment_point(), &self.context.secp_ctx); log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {} in channel {}", encode::serialize_hex(&commitment_stats.tx.trust().built_transaction().transaction), @@ -8111,7 +8112,7 @@ impl Channel where for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) { log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}", - encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.context.get_holder_selected_contest_delay(), htlc, &self.context.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)), + encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, htlc, &self.context.channel_type, revokeable_spk.clone())), encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &counterparty_keys)), log_bytes!(counterparty_keys.broadcaster_htlc_key.to_public_key().serialize()), log_bytes!(htlc_sig.serialize_compact()[..]), &self.context.channel_id()); @@ -11017,9 +11018,8 @@ mod tests { let remote_signature = Signature::from_der(&>::from_hex($counterparty_htlc_sig_hex).unwrap()[..]).unwrap(); let ref htlc = htlcs[$htlc_idx]; - let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw, - chan.context.get_counterparty_selected_contest_delay().unwrap(), - &htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key); + let revokeable_spk = signer.get_revokeable_spk(true, holder_commitment_tx.commitment_number(), &holder_commitment_tx.per_commitment_point(), &secp_ctx); + let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw, &htlc, $opt_anchors, revokeable_spk); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys); let htlc_sighashtype = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All }; let htlc_sighash = Message::from_digest(sighash::SighashCache::new(&htlc_tx).p2wsh_signature_hash(0, &htlc_redeemscript, htlc.to_bitcoin_amount(), htlc_sighashtype).unwrap().as_raw_hash().to_byte_array()); diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 9556e988b4e..22ae543277d 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -9,7 +9,9 @@ //! Further functional tests which test blockchain reorganizations. -use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor}; +use alloc::collections::BTreeMap; + +use crate::sign::{ecdsa::EcdsaChannelSigner, ChannelSigner, OutputSpender, SpendableOutputDescriptor}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep}; use crate::chain::transaction::OutPoint; use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight}; @@ -2901,6 +2903,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { }], }; let mut descriptors = Vec::with_capacity(4); + let mut revokeable_spks = BTreeMap::new(); for event in events { // We don't use the `BumpTransactionEventHandler` here because it does not support // creating one transaction from multiple `HTLCResolution` events. @@ -2909,7 +2912,12 @@ fn test_anchors_aggregated_revoked_htlc_tx() { for htlc_descriptor in &htlc_descriptors { assert!(!htlc_descriptor.htlc.offered); htlc_tx.input.push(htlc_descriptor.unsigned_tx_input()); - htlc_tx.output.push(htlc_descriptor.tx_output(&secp)); + let revokeable_spk = revokeable_spks.entry(htlc_descriptor.channel_derivation_parameters.keys_id) + .or_insert_with(|| { + let signer = htlc_descriptor.derive_channel_signer(&nodes[1].keys_manager); + signer.get_revokeable_spk(true, htlc_descriptor.per_commitment_number, &htlc_descriptor.per_commitment_point, &secp) + }).clone(); + htlc_tx.output.push(htlc_descriptor.tx_output(revokeable_spk)); } descriptors.append(&mut htlc_descriptors); htlc_tx.lock_time = tx_lock_time; diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 097b7c21486..33838155ae3 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -631,30 +631,12 @@ impl HTLCDescriptor { /// Returns the delayed output created as a result of spending the HTLC output in the commitment /// transaction. - pub fn tx_output( - &self, secp: &Secp256k1, - ) -> TxOut { - let channel_params = - self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable(); - let broadcaster_keys = channel_params.broadcaster_pubkeys(); - let counterparty_keys = channel_params.countersignatory_pubkeys(); - let broadcaster_delayed_key = DelayedPaymentKey::from_basepoint( - secp, - &broadcaster_keys.delayed_payment_basepoint, - &self.per_commitment_point, - ); - let counterparty_revocation_key = &RevocationKey::from_basepoint( - &secp, - &counterparty_keys.revocation_basepoint, - &self.per_commitment_point, - ); + pub fn tx_output(&self, revokeable_spk: ScriptBuf) -> TxOut { chan_utils::build_htlc_output( self.feerate_per_kw, - channel_params.contest_delay(), &self.htlc, - channel_params.channel_type_features(), - &broadcaster_delayed_key, - &counterparty_revocation_key, + &self.channel_derivation_parameters.transaction_parameters.channel_type_features, + revokeable_spk, ) } @@ -1468,19 +1450,21 @@ impl EcdsaChannelSigner for InMemorySigner { let commitment_txid = built_tx.txid; let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len()); + let revokeable_spk = self.get_revokeable_spk( + false, + trusted_tx.commitment_number(), + &trusted_tx.per_commitment_point(), + secp_ctx, + ); for htlc in commitment_tx.htlcs() { let channel_parameters = self.get_channel_parameters().expect(MISSING_PARAMS_ERR); - let holder_selected_contest_delay = - self.holder_selected_contest_delay().expect(MISSING_PARAMS_ERR); let chan_type = &channel_parameters.channel_type_features; let htlc_tx = chan_utils::build_htlc_transaction( &commitment_txid, commitment_tx.feerate_per_kw(), - holder_selected_contest_delay, htlc, chan_type, - &keys.broadcaster_delayed_payment_key, - &keys.revocation_key, + revokeable_spk.clone(), ); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, chan_type, &keys); let htlc_sighashtype = if chan_type.supports_anchors_zero_fee_htlc_tx() { diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index ab483185f28..88eddf6d675 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -308,7 +308,8 @@ impl EcdsaChannelSigner for TestChannelSigner { } } assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input()); - assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(secp_ctx)); + let revokeable_spk = self.get_revokeable_spk(true, htlc_descriptor.per_commitment_number, &htlc_descriptor.per_commitment_point, secp_ctx); + assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(revokeable_spk)); { let witness_script = htlc_descriptor.witness_script(secp_ctx); let sighash_type = if self.channel_type_features().supports_anchors_zero_fee_htlc_tx() { From 0f0560a0fdba054e1ab8ded2788bb09c05667a98 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sun, 15 Dec 2024 18:32:39 +0000 Subject: [PATCH 6/6] Add `ChannelSigner::punish_revokeable_output` All LN-Penalty channel signers need to be able to punish the counterparty in case they broadcast an old state. In this commit, we ask implementers of `ChannelSigner` to produce the full transaction with the given input finalized to punish the corresponding previous output. Consumers of the `ChannelSigner` trait can now be agnostic to the specific scripts used in revokeable outputs. We leave passing to the `ChannelSigner` all the previous `TxOut`'s needed to produce valid schnorr signatures under BIP 341 spending rules to a later patch. --- lightning/src/chain/chainmonitor.rs | 2 +- lightning/src/chain/channelmonitor.rs | 38 +++++--------- lightning/src/chain/package.rs | 10 +--- lightning/src/sign/mod.rs | 62 +++++++++++++++++++++++ lightning/src/util/test_channel_signer.rs | 7 +++ lightning/src/util/test_utils.rs | 2 +- 6 files changed, 85 insertions(+), 36 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index f1b9f729cb3..299370c90d5 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -96,7 +96,7 @@ use bitcoin::secp256k1::PublicKey; /// provided for bulding transactions for a watchtower: /// [`ChannelMonitor::initial_counterparty_commitment_tx`], /// [`ChannelMonitor::counterparty_commitment_txs_from_update`], -/// [`ChannelMonitor::sign_to_local_justice_tx`], [`TrustedCommitmentTransaction::revokeable_output_index`], +/// [`ChannelMonitor::punish_revokeable_output`], [`TrustedCommitmentTransaction::revokeable_output_index`], /// [`TrustedCommitmentTransaction::build_to_local_justice_tx`]. /// /// [`TrustedCommitmentTransaction::revokeable_output_index`]: crate::ln::chan_utils::TrustedCommitmentTransaction::revokeable_output_index diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 69da11f6dc8..be434d09d82 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -29,7 +29,6 @@ use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hash_types::{Txid, BlockHash}; -use bitcoin::ecdsa::Signature as BitcoinSignature; use bitcoin::secp256k1::{self, SecretKey, PublicKey, Secp256k1, ecdsa::Signature}; use crate::ln::channel::INITIAL_COMMITMENT_NUMBER; @@ -1675,8 +1674,8 @@ impl ChannelMonitor { /// This is provided so that watchtower clients in the persistence pipeline are able to build /// justice transactions for each counterparty commitment upon each update. It's intended to be /// used within an implementation of [`Persist::update_persisted_channel`], which is provided - /// with a monitor and an update. Once revoked, signing a justice transaction can be done using - /// [`Self::sign_to_local_justice_tx`]. + /// with a monitor and an update. Once revoked, punishing a revokeable output can be done using + /// [`Self::punish_revokeable_output`]. /// /// It is expected that a watchtower client may use this method to retrieve the latest counterparty /// commitment transaction(s), and then hold the necessary data until a later update in which @@ -1692,12 +1691,12 @@ impl ChannelMonitor { self.inner.lock().unwrap().counterparty_commitment_txs_from_update(update) } - /// Wrapper around [`EcdsaChannelSigner::sign_justice_revoked_output`] to make - /// signing the justice transaction easier for implementors of + /// Wrapper around [`ChannelSigner::punish_revokeable_output`] to make + /// punishing a revokeable output easier for implementors of /// [`chain::chainmonitor::Persist`]. On success this method returns the provided transaction - /// signing the input at `input_idx`. This method will only produce a valid signature for + /// finalizing the input at `input_idx`. This method will only produce a valid transaction for /// a transaction spending the `to_local` output of a commitment transaction, i.e. this cannot - /// be used for revoked HTLC outputs. + /// be used for revoked HTLC outputs of a commitment transaction. /// /// `Value` is the value of the output being spent by the input at `input_idx`, committed /// in the BIP 143 signature. @@ -1707,10 +1706,10 @@ impl ChannelMonitor { /// 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::ecdsa::EcdsaChannelSigner::sign_justice_revoked_output + /// [`ChannelSigner::punish_revokeable_output`]: crate::sign::ChannelSigner::punish_revokeable_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 { - self.inner.lock().unwrap().sign_to_local_justice_tx(justice_tx, input_idx, value, commitment_number) + pub fn punish_revokeable_output(&self, justice_tx: &Transaction, input_idx: usize, value: u64, commitment_number: u64) -> Result { + self.inner.lock().unwrap().punish_revokeable_output(justice_tx, input_idx, value, commitment_number) } pub(crate) fn get_min_seen_secret(&self) -> u64 { @@ -3458,27 +3457,14 @@ impl ChannelMonitorImpl { }).collect() } - fn sign_to_local_justice_tx( - &self, mut justice_tx: Transaction, input_idx: usize, value: u64, commitment_number: u64 + fn punish_revokeable_output( + &self, justice_tx: &Transaction, input_idx: usize, value: u64, commitment_number: u64 ) -> Result { let secret = self.get_secret(commitment_number).ok_or(())?; let per_commitment_key = SecretKey::from_slice(&secret).map_err(|_| ())?; let their_per_commitment_point = PublicKey::from_secret_key( &self.onchain_tx_handler.secp_ctx, &per_commitment_key); - - let revocation_pubkey = RevocationKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, - &self.holder_revocation_basepoint, &their_per_commitment_point); - let delayed_key = DelayedPaymentKey::from_basepoint(&self.onchain_tx_handler.secp_ctx, - &self.counterparty_commitment_params.counterparty_delayed_payment_base_key, &their_per_commitment_point); - let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, - self.counterparty_commitment_params.on_counterparty_tx_csv, &delayed_key); - - let sig = self.onchain_tx_handler.signer.sign_justice_revoked_output( - &justice_tx, input_idx, value, &per_commitment_key, &self.onchain_tx_handler.secp_ctx)?; - justice_tx.input[input_idx].witness.push_ecdsa_signature(&BitcoinSignature::sighash_all(sig)); - justice_tx.input[input_idx].witness.push(&[1u8]); - justice_tx.input[input_idx].witness.push(revokeable_redeemscript.as_bytes()); - Ok(justice_tx) + self.onchain_tx_handler.signer.punish_revokeable_output(justice_tx, input_idx, value, &per_commitment_key, &self.onchain_tx_handler.secp_ctx, &their_per_commitment_point) } /// Can only fail if idx is < get_min_seen_secret diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 53bba3a754b..d27b9acad61 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -604,15 +604,9 @@ impl PackageSolvingData { 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); - let witness_script = chan_utils::get_revokeable_redeemscript(&chan_keys.revocation_key, outp.on_counterparty_tx_csv, &chan_keys.broadcaster_delayed_payment_key); //TODO: should we panic on signer failure ? - if let Ok(sig) = onchain_handler.signer.sign_justice_revoked_output(&bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx) { - let mut ser_sig = sig.serialize_der().to_vec(); - ser_sig.push(EcdsaSighashType::All as u8); - bumped_tx.input[i].witness.push(ser_sig); - bumped_tx.input[i].witness.push(vec!(1)); - bumped_tx.input[i].witness.push(witness_script.clone().into_bytes()); + if let Ok(tx) = onchain_handler.signer.punish_revokeable_output(bumped_tx, i, outp.amount.to_sat(), &outp.per_commitment_key, &onchain_handler.secp_ctx, &outp.per_commitment_point) { + *bumped_tx = tx; } else { return false; } }, PackageSolvingData::RevokedHTLCOutput(ref outp) => { diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 33838155ae3..49663072125 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -795,6 +795,34 @@ pub trait ChannelSigner { &self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1, ) -> ScriptBuf; + + /// Finalize 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 the input at index `input` should be finalized here. + /// It may be called multiple times for the 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). + /// + /// 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. + /// + /// [`ChannelMonitor::signer_unblocked`]: crate::chain::channelmonitor::ChannelMonitor::signer_unblocked + /// + /// TODO(taproot): pass to the `ChannelSigner` all the `TxOut`'s spent by the justice transaction. + fn punish_revokeable_output( + &self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, + secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, + ) -> Result; } /// Specifies the recipient of an invoice. @@ -1421,6 +1449,40 @@ impl ChannelSigner for InMemorySigner { ) .to_p2wsh() } + + fn punish_revokeable_output( + &self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, + secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, + ) -> Result { + let params = self.channel_parameters.as_ref().unwrap().as_counterparty_broadcastable(); + let contest_delay = params.contest_delay(); + let keys = TxCreationKeys::from_channel_static_keys( + per_commitment_point, + params.broadcaster_pubkeys(), + params.countersignatory_pubkeys(), + secp_ctx, + ); + let witness_script = get_revokeable_redeemscript( + &keys.revocation_key, + contest_delay, + &keys.broadcaster_delayed_payment_key, + ); + let sig = EcdsaChannelSigner::sign_justice_revoked_output( + self, + justice_tx, + input, + amount, + per_commitment_key, + secp_ctx, + )?; + let mut justice_tx = justice_tx.clone(); + let mut ser_sig = sig.serialize_der().to_vec(); + ser_sig.push(EcdsaSighashType::All as u8); + justice_tx.input[input].witness.push(ser_sig); + justice_tx.input[input].witness.push(vec![1]); + justice_tx.input[input].witness.push(witness_script.into_bytes()); + Ok(justice_tx) + } } const MISSING_PARAMS_ERR: &'static str = diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index 88eddf6d675..c06f8fe7f60 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -226,6 +226,13 @@ impl ChannelSigner for TestChannelSigner { fn get_revokeable_spk(&self, to_self: bool, commitment_number: u64, per_commitment_point: &PublicKey, secp_ctx: &Secp256k1) -> ScriptBuf { self.inner.get_revokeable_spk(to_self, commitment_number, per_commitment_point, secp_ctx) } + + fn punish_revokeable_output( + &self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, + secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, + ) -> Result { + self.inner.punish_revokeable_output(justice_tx, input, amount, per_commitment_key, secp_ctx, per_commitment_point) + } } impl EcdsaChannelSigner for TestChannelSigner { diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 442a709866c..b9d8782b231 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -517,7 +517,7 @@ impl chainmonitor::Persist for while let Some(JusticeTxData { justice_tx, value, commitment_number }) = channel_state.front() { let input_idx = 0; let commitment_txid = justice_tx.input[input_idx].previous_output.txid; - match data.sign_to_local_justice_tx(justice_tx.clone(), input_idx, value.to_sat(), *commitment_number) { + match data.punish_revokeable_output(justice_tx, input_idx, value.to_sat(), *commitment_number) { Ok(signed_justice_tx) => { let dup = self.watchtower_state.lock().unwrap() .get_mut(&funding_txo).unwrap()