Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let ChannelSigner set to_remote, to_local, htlc tx scriptpubkeys #3454

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Let ChannelSigner set to_remote, to_local, htlc tx scriptpubkeys #3454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9604377
Let `ChannelSigner` set `to_remote` scriptpubkey
tankyleo Dec 7, 2024
ab43c1a
Use new `ChannelSigner` API in `HolderCommitmentTransaction::dummy`
tankyleo Dec 11, 2024
9dc7e3e
Add `InMemorySigner::overwrite_channel_parameters`
tankyleo Dec 11, 2024
1155fa0
Let `ChannelSigner` set `to_local` script pubkey
tankyleo Dec 13, 2024
d3181c5
Let `ChannelSigner` set htlc tx script pubkey
tankyleo Dec 13, 2024
0f0560a
Add `ChannelSigner::punish_revokeable_output`
tankyleo Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
102 changes: 45 additions & 57 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1338,7 +1337,7 @@ 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,
Expand All @@ -1347,10 +1346,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
best_block: BestBlock, counterparty_node_id: PublicKey, channel_id: ChannelId,
) -> ChannelMonitor<Signer> {

keys.provide_channel_parameters(channel_parameters);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I move the provide_channel_parameters call to here because there were some tests that built a ChannelMonitor without first calling provide_channel_parameters on the keys: Signer.

Further below in initial_commitment_signed, I delete the provide_channel_parameters call, as it is now called here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now need to call provide_channel_parameters before ChannelMonitor::new because ChannelSigner::get_counterparty_payment_script assumes that provide_channel_parameters has already been called - see the doc for ChannelSigner::get_counterparty_payment_script.


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 @@ -1675,8 +1674,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// 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
Expand All @@ -1692,12 +1691,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
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.
Expand All @@ -1707,10 +1706,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// to the commitment transaction being revoked, this will return a signed transaction, but
/// the signature will not be valid.
///
/// [`EcdsaChannelSigner::sign_justice_revoked_output`]: crate::sign::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<Transaction, ()> {
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<Transaction, ()> {
self.inner.lock().unwrap().punish_revokeable_output(justice_tx, input_idx, value, commitment_number)
}

pub(crate) fn get_min_seen_secret(&self) -> u64 {
Expand Down Expand Up @@ -3047,21 +3046,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// 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);
}
Expand Down Expand Up @@ -3100,7 +3099,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// 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(
Expand Down Expand Up @@ -3416,9 +3415,18 @@ 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 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, to_countersignatory_value, broadcaster_funding_key,
to_broadcaster_txout, counterparty_txout, broadcaster_funding_key,
countersignatory_funding_key, keys, feerate_per_kw, &mut nondust_htlcs,
channel_parameters)
}
Expand Down Expand Up @@ -3449,27 +3457,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}).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<Transaction, ()> {
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
Expand Down Expand Up @@ -3521,15 +3516,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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,
Expand Down Expand Up @@ -3639,16 +3630,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
} 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));
}
Expand Down Expand Up @@ -3738,11 +3722,15 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// 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<PackageTemplate>, Option<(ScriptBuf, PublicKey, RevocationKey)>) {
fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32, is_previous_tx: bool) -> (Vec<PackageTemplate>, 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 {
Expand Down Expand Up @@ -3809,7 +3797,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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,
Expand All @@ -3819,7 +3807,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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,
Expand Down
8 changes: 5 additions & 3 deletions lightning/src/chain/onchaintx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,8 +1210,9 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
.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 {
Expand Down Expand Up @@ -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};

Expand All @@ -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(),
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 2 additions & 8 deletions lightning/src/chain/package.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,15 +604,9 @@ impl PackageSolvingData {
fn finalize_input<Signer: EcdsaChannelSigner>(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler<Signer>) -> 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) => {
Expand Down
Loading
Loading