Skip to content

Commit

Permalink
Let ChannelSigner set to_remote scriptpubkey
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
tankyleo committed Dec 10, 2024
1 parent abf72a5 commit 06f5930
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 43 deletions.
14 changes: 9 additions & 5 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1331,19 +1331,19 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
ChannelMonitor { inner: Mutex::new(imp) }
}

pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_script: Option<ScriptBuf>,
pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, mut keys: Signer, shutdown_script: Option<ScriptBuf>,
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,
commitment_transaction_number_obscure_factor: u64,
initial_holder_commitment_tx: HolderCommitmentTransaction,
best_block: BestBlock, counterparty_node_id: PublicKey, channel_id: ChannelId,
) -> ChannelMonitor<Signer> {

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;
Expand Down Expand Up @@ -3398,9 +3398,13 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
&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)
}
Expand Down
60 changes: 34 additions & 26 deletions lightning/src/ln/chan_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<T>(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<T>(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);
Expand Down Expand Up @@ -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<BuiltCommitmentTransaction, ()> {
fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_funding_key: &PublicKey, countersignatory_funding_key: &PublicKey, to_countersignatory_spk: ScriptBuf) -> Result<BuiltCommitmentTransaction, ()> {
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();
Expand All @@ -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<T>(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<TxOut>, Vec<HTLCOutputInCommitment>), ()> {
let countersignatory_pubkeys = channel_parameters.countersignatory_pubkeys();
fn internal_build_outputs<T>(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<TxOut>, Vec<HTLCOutputInCommitment>), ()> {
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,
))
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1680,14 +1680,14 @@ impl CommitmentTransaction {
///
/// An external validating signer must call this method before signing
/// or using the built transaction.
pub fn verify<T: secp256k1::Signing + secp256k1::Verification>(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1<T>) -> Result<TrustedCommitmentTransaction, ()> {
pub fn verify<T: secp256k1::Signing + secp256k1::Verification>(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1<T>, to_countersignatory_spk: ScriptBuf) -> Result<TrustedCommitmentTransaction, ()> {
// 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(());
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
)
}
}
Expand Down
19 changes: 15 additions & 4 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1609,8 +1609,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> 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),
Expand Down Expand Up @@ -3165,9 +3164,14 @@ impl<SP: Deref> ChannelContext<SP> 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(),
Expand Down Expand Up @@ -10839,6 +10843,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;
Expand Down Expand Up @@ -10912,13 +10917,19 @@ 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)*);
};
}

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)*);
};
}
Expand Down
20 changes: 16 additions & 4 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,16 +766,22 @@ 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 channel_parameters = local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable();
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,
&mut htlcs,
&local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable()
&channel_parameters,
);
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap()
};
Expand Down Expand Up @@ -1517,15 +1523,21 @@ 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 channel_parameters = local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable();
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,
&mut vec![(accepted_htlc_info, ())],
&local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable()
&channel_parameters,
);
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap()
};
Expand Down
Loading

0 comments on commit 06f5930

Please sign in to comment.