Skip to content

Commit 5208a28

Browse files
tankyleotankyleo
committed
Remove explicit per-commitment key derivations from channel objects
Channel objects should not impose a particular per-commitment derivation scheme on the builders of commitment transactions. This will make it easier to enable custom derivations in the future. Instead of building `TxCreationKeys` explicitly, the function `TrustedCommitmentTransaction::keys` should be used when the keys of the commitment transaction are needed. We do that in this commit.
1 parent ffa7bbf commit 5208a28

File tree

1 file changed

+21
-57
lines changed

1 file changed

+21
-57
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 21 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use crate::ln::script::{self, ShutdownScript};
4141
use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails};
4242
use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
4343
use crate::ln::chan_utils::{
44-
CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight,
44+
CounterpartyCommitmentSecrets, HTLCOutputInCommitment, htlc_success_tx_weight,
4545
htlc_timeout_tx_weight, ChannelPublicKeys, CommitmentTransaction,
4646
HolderCommitmentTransaction, ChannelTransactionParameters,
4747
CounterpartyChannelTransactionParameters, MAX_HTLCS,
@@ -2036,8 +2036,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
20362036
) -> Result<CommitmentTransaction, ChannelError> where L::Target: Logger {
20372037
let funding_script = self.funding().get_funding_redeemscript();
20382038

2039-
let keys = self.context().build_holder_transaction_keys(&self.funding(), holder_commitment_point.current_point());
2040-
let initial_commitment_tx = self.context().build_commitment_transaction(self.funding(), holder_commitment_point.transaction_number(), &keys, true, false, logger).tx;
2039+
let initial_commitment_tx = self.context().build_commitment_transaction(self.funding(), holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(), true, false, logger).tx;
20412040
let trusted_tx = initial_commitment_tx.trust();
20422041
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
20432042
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.funding().get_value_satoshis());
@@ -2074,8 +2073,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
20742073
}
20752074
};
20762075
let context = self.context();
2077-
let counterparty_keys = context.build_remote_transaction_keys(self.funding());
2078-
let counterparty_initial_commitment_tx = context.build_commitment_transaction(self.funding(), context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
2076+
let counterparty_initial_commitment_tx = context.build_commitment_transaction(self.funding(), context.cur_counterparty_commitment_transaction_number, &context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
20792077
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
20802078
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
20812079

@@ -3460,7 +3458,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
34603458
/// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
34613459
/// which peer generated this transaction and "to whom" this transaction flows.
34623460
#[inline]
3463-
fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, logger: &L) -> CommitmentStats
3461+
fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentStats
34643462
where L::Target: Logger
34653463
{
34663464
let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
@@ -3662,7 +3660,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
36623660
if local { funding.channel_transaction_parameters.as_holder_broadcastable() }
36633661
else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
36643662
let tx = CommitmentTransaction::new(commitment_number,
3665-
&keys.per_commitment_point,
3663+
&per_commitment_point,
36663664
value_to_a as u64,
36673665
value_to_b as u64,
36683666
feerate_per_kw,
@@ -3688,32 +3686,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
36883686
}
36893687
}
36903688

3691-
#[inline]
3692-
/// Creates a set of keys for build_commitment_transaction to generate a transaction which our
3693-
/// counterparty will sign (ie DO NOT send signatures over a transaction created by this to
3694-
/// our counterparty!)
3695-
/// The result is a transaction which we can revoke broadcastership of (ie a "local" transaction)
3696-
/// TODO Some magic rust shit to compile-time check this?
3697-
fn build_holder_transaction_keys(&self, funding: &FundingScope, per_commitment_point: PublicKey) -> TxCreationKeys {
3698-
let delayed_payment_base = &funding.get_holder_pubkeys().delayed_payment_basepoint;
3699-
let htlc_basepoint = &funding.get_holder_pubkeys().htlc_basepoint;
3700-
let counterparty_pubkeys = funding.get_counterparty_pubkeys();
3701-
3702-
TxCreationKeys::derive_new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint)
3703-
}
3704-
3705-
#[inline]
3706-
/// Creates a set of keys for build_commitment_transaction to generate a transaction which we
3707-
/// will sign and send to our counterparty.
3708-
/// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
3709-
fn build_remote_transaction_keys(&self, funding: &FundingScope) -> TxCreationKeys {
3710-
let revocation_basepoint = &funding.get_holder_pubkeys().revocation_basepoint;
3711-
let htlc_basepoint = &funding.get_holder_pubkeys().htlc_basepoint;
3712-
let counterparty_pubkeys = funding.get_counterparty_pubkeys();
3713-
3714-
TxCreationKeys::derive_new(&self.secp_ctx, &self.counterparty_cur_commitment_point.unwrap(), &counterparty_pubkeys.delayed_payment_basepoint, &counterparty_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint)
3715-
}
3716-
37173689
pub fn get_feerate_sat_per_1000_weight(&self) -> u32 {
37183690
self.feerate_per_kw
37193691
}
@@ -4497,9 +4469,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
44974469
SP::Target: SignerProvider,
44984470
L::Target: Logger
44994471
{
4500-
let counterparty_keys = self.build_remote_transaction_keys(funding);
45014472
let counterparty_initial_commitment_tx = self.build_commitment_transaction(
4502-
funding, self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
4473+
funding, self.cur_counterparty_commitment_transaction_number, &self.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
45034474
match self.holder_signer {
45044475
// TODO (taproot|arik): move match into calling method for Taproot
45054476
ChannelSignerType::Ecdsa(ref ecdsa) => {
@@ -5499,9 +5470,7 @@ impl<SP: Deref> FundedChannel<SP> where
54995470

55005471
let funding_script = self.funding.get_funding_redeemscript();
55015472

5502-
let keys = self.context.build_holder_transaction_keys(&self.funding, self.holder_commitment_point.current_point());
5503-
5504-
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &keys, true, false, logger);
5473+
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, false, logger);
55055474
let commitment_txid = {
55065475
let trusted_tx = commitment_stats.tx.trust();
55075476
let bitcoin_tx = trusted_tx.built_transaction();
@@ -5569,19 +5538,20 @@ impl<SP: Deref> FundedChannel<SP> where
55695538

55705539
let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len());
55715540
let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
5541+
let holder_keys = commitment_stats.tx.trust().keys();
55725542
for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() {
55735543
if let Some(_) = htlc.transaction_output_index {
55745544
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
55755545
self.funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, &self.context.channel_type,
5576-
&keys.broadcaster_delayed_payment_key, &keys.revocation_key);
5546+
&holder_keys.broadcaster_delayed_payment_key, &holder_keys.revocation_key);
55775547

5578-
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &keys);
5548+
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &holder_keys);
55795549
let htlc_sighashtype = if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All };
55805550
let htlc_sighash = hash_to_message!(&sighash::SighashCache::new(&htlc_tx).p2wsh_signature_hash(0, &htlc_redeemscript, htlc.to_bitcoin_amount(), htlc_sighashtype).unwrap()[..]);
55815551
log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.",
5582-
log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(keys.countersignatory_htlc_key.to_public_key().serialize()),
5552+
log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(holder_keys.countersignatory_htlc_key.to_public_key().serialize()),
55835553
encode::serialize_hex(&htlc_tx), log_bytes!(htlc_sighash[..]), encode::serialize_hex(&htlc_redeemscript), &self.context.channel_id());
5584-
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &keys.countersignatory_htlc_key.to_public_key()) {
5554+
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &holder_keys.countersignatory_htlc_key.to_public_key()) {
55855555
return Err(ChannelError::close("Invalid HTLC tx signature from peer".to_owned()));
55865556
}
55875557
if !separate_nondust_htlc_sources {
@@ -6263,8 +6233,7 @@ impl<SP: Deref> FundedChannel<SP> where
62636233
// Before proposing a feerate update, check that we can actually afford the new fee.
62646234
let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
62656235
let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw), dust_exposure_limiting_feerate);
6266-
let keys = self.context.build_holder_transaction_keys(&self.funding, self.holder_commitment_point.current_point());
6267-
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &keys, true, true, logger);
6236+
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, true, logger);
62686237
let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_stats.num_nondust_htlcs + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000;
62696238
let holder_balance_msat = commitment_stats.local_balance_msat - htlc_stats.outbound_holding_cell_msat;
62706239
if holder_balance_msat < buffer_fee_msat + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
@@ -6577,8 +6546,7 @@ impl<SP: Deref> FundedChannel<SP> where
65776546
self.holder_commitment_point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
65786547
}
65796548
let funding_signed = if self.context.signer_pending_funding && !self.funding.is_outbound() {
6580-
let counterparty_keys = self.context.build_remote_transaction_keys(&self.funding);
6581-
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number + 1, &counterparty_keys, false, false, logger).tx;
6549+
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number + 1, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
65826550
self.context.get_funding_signed_msg(&self.funding.channel_transaction_parameters, logger, counterparty_initial_commitment_tx)
65836551
} else { None };
65846552
// Provide a `channel_ready` message if we need to, but only if we're _not_ still pending
@@ -8514,8 +8482,7 @@ impl<SP: Deref> FundedChannel<SP> where
85148482
-> (Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, CommitmentTransaction)
85158483
where L::Target: Logger
85168484
{
8517-
let counterparty_keys = self.context.build_remote_transaction_keys(&self.funding);
8518-
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
8485+
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
85198486
let counterparty_commitment_tx = commitment_stats.tx;
85208487

85218488
#[cfg(any(test, fuzzing))]
@@ -8546,8 +8513,7 @@ impl<SP: Deref> FundedChannel<SP> where
85468513
#[cfg(any(test, fuzzing))]
85478514
self.build_commitment_no_state_update(logger);
85488515

8549-
let counterparty_keys = self.context.build_remote_transaction_keys(&self.funding);
8550-
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
8516+
let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
85518517
let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
85528518

85538519
match &self.context.holder_signer {
@@ -8575,6 +8541,7 @@ impl<SP: Deref> FundedChannel<SP> where
85758541
&counterparty_commitment_txid, encode::serialize_hex(&self.funding.get_funding_redeemscript()),
85768542
log_bytes!(signature.serialize_compact()[..]), &self.context.channel_id());
85778543

8544+
let counterparty_keys = commitment_stats.tx.trust().keys();
85788545
for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
85798546
log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}",
85808547
encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.funding.get_holder_selected_contest_delay(), htlc, &self.context.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
@@ -9030,8 +8997,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
90308997

90318998
/// Only allowed after [`FundingScope::channel_transaction_parameters`] is set.
90328999
fn get_funding_created_msg<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
9033-
let counterparty_keys = self.context.build_remote_transaction_keys(&self.funding);
9034-
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
9000+
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
90359001
let signature = match &self.context.holder_signer {
90369002
// TODO (taproot|arik): move match into calling method for Taproot
90379003
ChannelSignerType::Ecdsa(ecdsa) => {
@@ -11557,7 +11523,7 @@ mod tests {
1155711523
use bitcoin::secp256k1::Message;
1155811524
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ecdsa::EcdsaChannelSigner};
1155911525
use crate::types::payment::PaymentPreimage;
11560-
use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};
11526+
use crate::ln::channel::HTLCOutputInCommitment;
1156111527
use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint};
1156211528
use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
1156311529
use crate::util::logger::Logger;
@@ -11622,11 +11588,8 @@ mod tests {
1162211588
// We can't just use build_holder_transaction_keys here as the per_commitment_secret is not
1162311589
// derived from a commitment_seed, so instead we copy it here and call
1162411590
// build_commitment_transaction.
11625-
let delayed_payment_base = &chan.context.holder_signer.as_ref().pubkeys().delayed_payment_basepoint;
1162611591
let per_commitment_secret = SecretKey::from_slice(&<Vec<u8>>::from_hex("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
1162711592
let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
11628-
let htlc_basepoint = &chan.context.holder_signer.as_ref().pubkeys().htlc_basepoint;
11629-
let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint);
1163011593

1163111594
macro_rules! test_commitment {
1163211595
( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $($remain:tt)* ) => {
@@ -11647,7 +11610,7 @@ mod tests {
1164711610
$( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
1164811611
} ) => { {
1164911612
let (commitment_tx, htlcs): (_, Vec<HTLCOutputInCommitment>) = {
11650-
let mut commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &keys, true, false, &logger);
11613+
let mut commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
1165111614

1165211615
let htlcs = commitment_stats.htlcs_included.drain(..)
1165311616
.filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
@@ -11695,6 +11658,7 @@ mod tests {
1169511658
let remote_signature = Signature::from_der(&<Vec<u8>>::from_hex($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
1169611659

1169711660
let ref htlc = htlcs[$htlc_idx];
11661+
let keys = commitment_tx.trust().keys();
1169811662
let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,
1169911663
chan.funding.get_counterparty_selected_contest_delay().unwrap(),
1170011664
&htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);

0 commit comments

Comments
 (0)