Skip to content

Commit f4e40e1

Browse files
tankyleotankyleo
committed
Reorg return values of ChannelContext::build_commitment_transaction
We choose to include in `CommitmentStats` only fields that will be calculated or constructed by transaction builders. The other fields are created by channel, and placed in a new struct we call `CommitmentData`.
1 parent 218ecce commit f4e40e1

File tree

1 file changed

+77
-44
lines changed

1 file changed

+77
-44
lines changed

‎lightning/src/ln/channel.rs‎

Lines changed: 77 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -877,15 +877,20 @@ struct HTLCStats {
877877
on_holder_tx_outbound_holding_cell_htlcs_count: u32, // dust HTLCs *non*-included
878878
}
879879

880-
/// An enum gathering stats on commitment transaction, either local or remote.
881-
struct CommitmentStats<'a> {
880+
/// A struct gathering data on a commitment, either local or remote.
881+
struct CommitmentData<'a> {
882+
stats: CommitmentStats,
883+
htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
884+
outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
885+
inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
886+
}
887+
888+
/// A struct gathering stats on a commitment transaction, either local or remote.
889+
struct CommitmentStats {
882890
tx: CommitmentTransaction, // the transaction info
883891
total_fee_sat: u64, // the total fee included in the transaction
884-
htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
885892
local_balance_before_fee_msat: u64, // local balance before fees *not* considering dust limits
886893
remote_balance_before_fee_msat: u64, // remote balance before fees *not* considering dust limits
887-
outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
888-
inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
889894
}
890895

891896
/// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -2043,7 +2048,10 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
20432048
) -> Result<CommitmentTransaction, ChannelError> where L::Target: Logger {
20442049
let funding_script = self.funding().get_funding_redeemscript();
20452050

2046-
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;
2051+
let commitment_data = self.context().build_commitment_transaction(self.funding(),
2052+
holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(),
2053+
true, false, logger);
2054+
let initial_commitment_tx = commitment_data.stats.tx;
20472055
let trusted_tx = initial_commitment_tx.trust();
20482056
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
20492057
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.funding().get_value_satoshis());
@@ -2080,7 +2088,10 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
20802088
}
20812089
};
20822090
let context = self.context();
2083-
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;
2091+
let commitment_data = context.build_commitment_transaction(self.funding(),
2092+
context.cur_counterparty_commitment_transaction_number,
2093+
&context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
2094+
let counterparty_initial_commitment_tx = commitment_data.stats.tx;
20842095
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
20852096
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
20862097

@@ -3479,9 +3490,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
34793490
{
34803491
let funding_script = funding.get_funding_redeemscript();
34813492

3482-
let commitment_stats = self.build_commitment_transaction(funding, holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(), true, false, logger);
3493+
let commitment_data = self.build_commitment_transaction(funding,
3494+
holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(),
3495+
true, false, logger);
34833496
let commitment_txid = {
3484-
let trusted_tx = commitment_stats.tx.trust();
3497+
let trusted_tx = commitment_data.stats.tx.trust();
34853498
let bitcoin_tx = trusted_tx.built_transaction();
34863499
let sighash = bitcoin_tx.get_sighash_all(&funding_script, funding.get_value_satoshis());
34873500

@@ -3494,7 +3507,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
34943507
}
34953508
bitcoin_tx.txid
34963509
};
3497-
let mut htlcs_cloned: Vec<_> = commitment_stats.htlcs_included.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
3510+
let mut htlcs_cloned: Vec<_> = commitment_data.htlcs_included.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
34983511

34993512
// If our counterparty updated the channel fee in this commitment transaction, check that
35003513
// they can actually afford the new fee now.
@@ -3504,7 +3517,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
35043517
if update_fee {
35053518
debug_assert!(!funding.is_outbound());
35063519
let counterparty_reserve_we_require_msat = funding.holder_selected_channel_reserve_satoshis * 1000;
3507-
if commitment_stats.remote_balance_before_fee_msat < commitment_stats.total_fee_sat * 1000 + counterparty_reserve_we_require_msat {
3520+
if commitment_data.stats.remote_balance_before_fee_msat < commitment_data.stats.total_fee_sat * 1000 + counterparty_reserve_we_require_msat {
35083521
return Err(ChannelError::close("Funding remote cannot afford proposed new fee".to_owned()));
35093522
}
35103523
}
@@ -3520,14 +3533,14 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
35203533
&& info.next_holder_htlc_id == self.next_holder_htlc_id
35213534
&& info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
35223535
&& info.feerate == self.feerate_per_kw {
3523-
assert_eq!(commitment_stats.total_fee_sat, info.fee / 1000);
3536+
assert_eq!(commitment_data.stats.total_fee_sat, info.fee / 1000);
35243537
}
35253538
}
35263539
}
35273540
}
35283541

3529-
if msg.htlc_signatures.len() != commitment_stats.tx.htlcs().len() {
3530-
return Err(ChannelError::close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), commitment_stats.tx.htlcs().len())));
3542+
if msg.htlc_signatures.len() != commitment_data.stats.tx.htlcs().len() {
3543+
return Err(ChannelError::close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), commitment_data.stats.tx.htlcs().len())));
35313544
}
35323545

35333546
// Up to LDK 0.0.115, HTLC information was required to be duplicated in the
@@ -3547,10 +3560,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
35473560

35483561
let mut nondust_htlc_sources = Vec::with_capacity(htlcs_cloned.len());
35493562
let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
3550-
let holder_keys = commitment_stats.tx.trust().keys();
3563+
let holder_keys = commitment_data.stats.tx.trust().keys();
35513564
for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() {
35523565
if let Some(_) = htlc.transaction_output_index {
3553-
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.tx.feerate_per_kw(),
3566+
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_data.stats.tx.feerate_per_kw(),
35543567
funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, &self.channel_type,
35553568
&holder_keys.broadcaster_delayed_payment_key, &holder_keys.revocation_key);
35563569

@@ -3578,14 +3591,14 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
35783591
}
35793592

35803593
let holder_commitment_tx = HolderCommitmentTransaction::new(
3581-
commitment_stats.tx,
3594+
commitment_data.stats.tx,
35823595
msg.signature,
35833596
msg.htlc_signatures.clone(),
35843597
&funding.get_holder_pubkeys().funding_pubkey,
35853598
funding.counterparty_funding_pubkey()
35863599
);
35873600

3588-
self.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outbound_htlc_preimages)
3601+
self.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_data.outbound_htlc_preimages)
35893602
.map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
35903603

35913604
Ok(LatestHolderCommitmentTXInfo {
@@ -3609,7 +3622,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
36093622
/// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
36103623
/// which peer generated this transaction and "to whom" this transaction flows.
36113624
#[inline]
3612-
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
3625+
fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentData
36133626
where L::Target: Logger
36143627
{
36153628
let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
@@ -3825,12 +3838,16 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38253838
htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
38263839
htlcs_included.append(&mut included_dust_htlcs);
38273840

3828-
CommitmentStats {
3841+
let stats = CommitmentStats {
38293842
tx,
38303843
total_fee_sat,
3831-
htlcs_included,
38323844
local_balance_before_fee_msat: value_to_self_msat,
38333845
remote_balance_before_fee_msat: value_to_remote_msat,
3846+
};
3847+
3848+
CommitmentData {
3849+
stats,
3850+
htlcs_included,
38343851
inbound_htlc_preimages,
38353852
outbound_htlc_preimages,
38363853
}
@@ -4618,8 +4635,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
46184635
SP::Target: SignerProvider,
46194636
L::Target: Logger
46204637
{
4621-
let counterparty_initial_commitment_tx = self.build_commitment_transaction(
4622-
funding, self.cur_counterparty_commitment_transaction_number, &self.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
4638+
let commitment_data = self.build_commitment_transaction(funding,
4639+
self.cur_counterparty_commitment_transaction_number,
4640+
&self.counterparty_cur_commitment_point.unwrap(), false, false, logger);
4641+
let counterparty_initial_commitment_tx = commitment_data.stats.tx;
46234642
match self.holder_signer {
46244643
// TODO (taproot|arik): move match into calling method for Taproot
46254644
ChannelSignerType::Ecdsa(ref ecdsa) => {
@@ -6332,9 +6351,11 @@ impl<SP: Deref> FundedChannel<SP> where
63326351
// Before proposing a feerate update, check that we can actually afford the new fee.
63336352
let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
63346353
let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw), dust_exposure_limiting_feerate);
6335-
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);
6336-
let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_stats.tx.htlcs().len() + 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;
6337-
let holder_balance_msat = commitment_stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat;
6354+
let commitment_data = self.context.build_commitment_transaction(&self.funding,
6355+
self.holder_commitment_point.transaction_number(),
6356+
&self.holder_commitment_point.current_point(), true, true, logger);
6357+
let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.stats.tx.htlcs().len() + 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;
6358+
let holder_balance_msat = commitment_data.stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat;
63386359
if holder_balance_msat < buffer_fee_msat + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
63396360
//TODO: auto-close after a number of failures?
63406361
log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw);
@@ -6645,7 +6666,10 @@ impl<SP: Deref> FundedChannel<SP> where
66456666
self.holder_commitment_point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
66466667
}
66476668
let funding_signed = if self.context.signer_pending_funding && !self.funding.is_outbound() {
6648-
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;
6669+
let commitment_data = self.context.build_commitment_transaction(&self.funding,
6670+
self.context.cur_counterparty_commitment_transaction_number + 1,
6671+
&self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
6672+
let counterparty_initial_commitment_tx = commitment_data.stats.tx;
66496673
self.context.get_funding_signed_msg(&self.funding.channel_transaction_parameters, logger, counterparty_initial_commitment_tx)
66506674
} else { None };
66516675
// Provide a `channel_ready` message if we need to, but only if we're _not_ still pending
@@ -8718,8 +8742,10 @@ impl<SP: Deref> FundedChannel<SP> where
87188742
-> (Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, CommitmentTransaction)
87198743
where L::Target: Logger
87208744
{
8721-
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);
8722-
let counterparty_commitment_tx = commitment_stats.tx;
8745+
let commitment_data = self.context.build_commitment_transaction(&self.funding,
8746+
self.context.cur_counterparty_commitment_transaction_number,
8747+
&self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
8748+
let counterparty_commitment_tx = commitment_data.stats.tx;
87238749

87248750
#[cfg(any(test, fuzzing))]
87258751
{
@@ -8739,7 +8765,7 @@ impl<SP: Deref> FundedChannel<SP> where
87398765
}
87408766
}
87418767

8742-
(commitment_stats.htlcs_included, counterparty_commitment_tx)
8768+
(commitment_data.htlcs_included, counterparty_commitment_tx)
87438769
}
87448770

87458771
/// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed
@@ -8749,8 +8775,10 @@ impl<SP: Deref> FundedChannel<SP> where
87498775
#[cfg(any(test, fuzzing))]
87508776
self.build_commitment_no_state_update(logger);
87518777

8752-
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);
8753-
let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
8778+
let commitment_data = self.context.build_commitment_transaction(&self.funding,
8779+
self.context.cur_counterparty_commitment_transaction_number,
8780+
&self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
8781+
let counterparty_commitment_tx = commitment_data.stats.tx;
87548782

87558783
match &self.context.holder_signer {
87568784
ChannelSignerType::Ecdsa(ecdsa) => {
@@ -8759,24 +8787,25 @@ impl<SP: Deref> FundedChannel<SP> where
87598787
{
87608788
let res = ecdsa.sign_counterparty_commitment(
87618789
&self.funding.channel_transaction_parameters,
8762-
&commitment_stats.tx,
8763-
commitment_stats.inbound_htlc_preimages,
8764-
commitment_stats.outbound_htlc_preimages,
8790+
&counterparty_commitment_tx,
8791+
commitment_data.inbound_htlc_preimages,
8792+
commitment_data.outbound_htlc_preimages,
87658793
&self.context.secp_ctx,
87668794
).map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
87678795
signature = res.0;
87688796
htlc_signatures = res.1;
87698797

8798+
let trusted_tx = counterparty_commitment_tx.trust();
87708799
log_trace!(logger, "Signed remote commitment tx {} (txid {}) with redeemscript {} -> {} in channel {}",
8771-
encode::serialize_hex(&commitment_stats.tx.trust().built_transaction().transaction),
8772-
&counterparty_commitment_txid, encode::serialize_hex(&self.funding.get_funding_redeemscript()),
8800+
encode::serialize_hex(&trusted_tx.built_transaction().transaction),
8801+
&trusted_tx.txid(), encode::serialize_hex(&self.funding.get_funding_redeemscript()),
87738802
log_bytes!(signature.serialize_compact()[..]), &self.context.channel_id());
87748803

8775-
let counterparty_keys = commitment_stats.tx.trust().keys();
8776-
debug_assert_eq!(htlc_signatures.len(), commitment_stats.tx.htlcs().len());
8777-
for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(commitment_stats.tx.htlcs()) {
8804+
let counterparty_keys = trusted_tx.keys();
8805+
debug_assert_eq!(htlc_signatures.len(), trusted_tx.htlcs().len());
8806+
for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(trusted_tx.htlcs()) {
87788807
log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}",
8779-
encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.tx.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)),
8808+
encode::serialize_hex(&chan_utils::build_htlc_transaction(&trusted_tx.txid(), trusted_tx.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)),
87808809
encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &counterparty_keys)),
87818810
log_bytes!(counterparty_keys.broadcaster_htlc_key.to_public_key().serialize()),
87828811
log_bytes!(htlc_sig.serialize_compact()[..]), &self.context.channel_id());
@@ -9230,7 +9259,10 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
92309259

92319260
/// Only allowed after [`FundingScope::channel_transaction_parameters`] is set.
92329261
fn get_funding_created_msg<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
9233-
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;
9262+
let commitment_data = self.context.build_commitment_transaction(&self.funding,
9263+
self.context.cur_counterparty_commitment_transaction_number,
9264+
&self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
9265+
let counterparty_initial_commitment_tx = commitment_data.stats.tx;
92349266
let signature = match &self.context.holder_signer {
92359267
// TODO (taproot|arik): move match into calling method for Taproot
92369268
ChannelSignerType::Ecdsa(ecdsa) => {
@@ -11881,8 +11913,9 @@ mod tests {
1188111913
( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $opt_anchors: expr, {
1188211914
$( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
1188311915
} ) => { {
11884-
let commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
11885-
let commitment_tx = commitment_stats.tx;
11916+
let commitment_data = chan.context.build_commitment_transaction(&chan.funding,
11917+
0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
11918+
let commitment_tx = commitment_data.stats.tx;
1188611919
let trusted_tx = commitment_tx.trust();
1188711920
let unsigned_tx = trusted_tx.built_transaction();
1188811921
let redeemscript = chan.funding.get_funding_redeemscript();

0 commit comments

Comments
 (0)