Skip to content

Commit c245ca1

Browse files
tankyleotankyleo
committed
Add TxBuilder::build_commitment_transaction
1 parent d5e636a commit c245ca1

File tree

3 files changed

+192
-52
lines changed

3 files changed

+192
-52
lines changed

‎lightning/src/ln/chan_utils.rs

+15
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,21 @@ impl HTLCOutputInCommitment {
622622
&& self.cltv_expiry == other.cltv_expiry
623623
&& self.payment_hash == other.payment_hash
624624
}
625+
626+
pub(crate) fn is_dust(&self, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures) -> bool {
627+
let htlc_tx_fee_sat = if features.supports_anchors_zero_fee_htlc_tx() {
628+
0
629+
} else {
630+
let htlc_tx_weight = if self.offered {
631+
htlc_timeout_tx_weight(features)
632+
} else {
633+
htlc_success_tx_weight(features)
634+
};
635+
// As required by the spec, round down
636+
feerate_per_kw as u64 * htlc_tx_weight / 1000
637+
};
638+
self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee_sat
639+
}
625640
}
626641

627642
impl_writeable_tlv_based!(HTLCOutputInCommitment, {

‎lightning/src/ln/channel.rs

+24-50
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,7 @@ struct CommitmentData<'a> {
986986
}
987987

988988
/// A struct gathering stats on a commitment transaction, either local or remote.
989+
#[derive(Debug, PartialEq)]
989990
pub(crate) struct CommitmentStats {
990991
pub(crate) total_fee_sat: u64, // the total fee included in the transaction
991992
pub(crate) local_balance_before_fee_msat: u64, // local balance before fees *not* considering dust limits
@@ -3961,16 +3962,9 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39613962
let broadcaster_dust_limit_sat = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
39623963
let feerate_per_kw = self.get_commitment_feerate(funding, generated_by_local);
39633964

3964-
let stats = self.build_commitment_stats(funding, local, generated_by_local, None, None);
3965-
let CommitmentStats {
3966-
total_fee_sat,
3967-
local_balance_before_fee_msat,
3968-
remote_balance_before_fee_msat
3969-
} = stats;
3970-
39713965
let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
39723966
let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
3973-
let mut nondust_htlcs: Vec<HTLCOutputInCommitment> = Vec::with_capacity(num_htlcs);
3967+
let mut value_to_self_msat_offset = 0;
39743968

39753969
log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...",
39763970
commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number),
@@ -3993,13 +3987,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39933987
macro_rules! add_htlc_output {
39943988
($htlc: expr, $outbound: expr, $source: expr) => {
39953989
let htlc_in_tx = get_htlc_in_commitment!($htlc, $outbound == local);
3996-
htlcs_included.push((htlc_in_tx.clone(), $source));
3997-
if $htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, funding.get_channel_type()) {
3998-
log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
3999-
} else {
4000-
log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $htlc.state, $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
4001-
nondust_htlcs.push(htlc_in_tx);
4002-
}
3990+
htlcs_included.push((htlc_in_tx, $source));
40033991
}
40043992
}
40053993

@@ -4008,11 +3996,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
40083996

40093997
for htlc in self.pending_inbound_htlcs.iter() {
40103998
if htlc.state.included_in_commitment(generated_by_local) {
3999+
log_trace!(logger, " ...including inbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
40114000
add_htlc_output!(htlc, false, None);
40124001
} else {
40134002
log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
40144003
if let Some(preimage) = htlc.state.preimage() {
40154004
inbound_htlc_preimages.push(preimage);
4005+
value_to_self_msat_offset += htlc.amount_msat as i64;
40164006
}
40174007
}
40184008
};
@@ -4022,53 +4012,37 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
40224012
outbound_htlc_preimages.push(preimage);
40234013
}
40244014
if htlc.state.included_in_commitment(generated_by_local) {
4015+
log_trace!(logger, " ...including outbound {} HTLC {} (hash {}) with value {}", htlc.state, htlc.htlc_id, htlc.payment_hash, htlc.amount_msat);
40254016
add_htlc_output!(htlc, true, Some(&htlc.source));
40264017
} else {
40274018
log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state);
4019+
if htlc.state.preimage().is_some() {
4020+
value_to_self_msat_offset -= htlc.amount_msat as i64;
4021+
}
40284022
}
40294023
};
40304024

4031-
// We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater
4032-
// than or equal to the sum of `total_fee_sat` and `total_anchors_sat`.
4025+
// # Panics
40334026
//
4034-
// This is because when the remote party sends an `update_fee` message, we build the new
4035-
// commitment transaction *before* checking whether the remote party's balance is enough to
4036-
// cover the total fee and the anchors.
4037-
4038-
let (value_to_self, value_to_remote) = if funding.is_outbound() {
4039-
((local_balance_before_fee_msat / 1000).saturating_sub(total_fee_sat), remote_balance_before_fee_msat / 1000)
4040-
} else {
4041-
(local_balance_before_fee_msat / 1000, (remote_balance_before_fee_msat / 1000).saturating_sub(total_fee_sat))
4042-
};
4043-
4044-
let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote };
4045-
let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self };
4046-
4047-
if to_broadcaster_value_sat >= broadcaster_dust_limit_sat {
4048-
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat);
4049-
} else {
4050-
to_broadcaster_value_sat = 0;
4051-
}
4052-
4053-
if to_countersignatory_value_sat >= broadcaster_dust_limit_sat {
4054-
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat);
4055-
} else {
4056-
to_countersignatory_value_sat = 0;
4057-
}
4027+
// While we expect `value_to_self_msat_offset` to be negative in some cases, the local
4028+
// balance MUST remain greater than or equal to 0.
4029+
4030+
// TODO: When MSRV >= 1.66.0, use u64::checked_add_signed
4031+
let value_to_self_with_offset_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_offset).try_into().unwrap();
40584032

4059-
let channel_parameters =
4060-
if local { funding.channel_transaction_parameters.as_holder_broadcastable() }
4061-
else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
4062-
let tx = CommitmentTransaction::new(
4033+
let (tx, stats) = SpecTxBuilder {}.build_commitment_transaction(
4034+
local,
40634035
commitment_number,
40644036
per_commitment_point,
4065-
to_broadcaster_value_sat,
4066-
to_countersignatory_value_sat,
4067-
feerate_per_kw,
4068-
nondust_htlcs,
4069-
&channel_parameters,
4037+
&funding.channel_transaction_parameters,
40704038
&self.secp_ctx,
4039+
value_to_self_with_offset_msat,
4040+
htlcs_included.iter().map(|(htlc, _source)| htlc).cloned().collect(),
4041+
feerate_per_kw,
4042+
broadcaster_dust_limit_sat,
4043+
logger,
40714044
);
4045+
debug_assert_eq!(stats, self.build_commitment_stats(funding, local, generated_by_local, None, None));
40724046

40734047
// This populates the HTLC-source table with the indices from the HTLCs in the commitment
40744048
// transaction.

‎lightning/src/sign/tx_builder.rs

+153-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
//! Defines the `TxBuilder` trait, and the `SpecTxBuilder` type
22
3-
use crate::ln::chan_utils::commit_tx_fee_sat;
4-
use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI;
3+
use core::ops::Deref;
4+
5+
use bitcoin::secp256k1::{self, PublicKey, Secp256k1};
6+
7+
use crate::ln::chan_utils::{
8+
commit_tx_fee_sat, ChannelTransactionParameters, CommitmentTransaction, HTLCOutputInCommitment,
9+
};
10+
use crate::ln::channel::{CommitmentStats, ANCHOR_OUTPUT_VALUE_SATOSHI};
511
use crate::prelude::*;
612
use crate::types::features::ChannelTypeFeatures;
13+
use crate::util::logger::Logger;
714

815
pub(crate) trait TxBuilder {
916
fn commit_tx_fee_sat(
@@ -13,6 +20,14 @@ pub(crate) trait TxBuilder {
1320
&self, is_outbound_from_holder: bool, channel_type: &ChannelTypeFeatures,
1421
value_to_self_after_htlcs: u64, value_to_remote_after_htlcs: u64,
1522
) -> (u64, u64);
23+
fn build_commitment_transaction<L: Deref>(
24+
&self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey,
25+
channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1<secp256k1::All>,
26+
value_to_self_msat: u64, htlcs_in_tx: Vec<HTLCOutputInCommitment>, feerate_per_kw: u32,
27+
broadcaster_dust_limit_satoshis: u64, logger: &L,
28+
) -> (CommitmentTransaction, CommitmentStats)
29+
where
30+
L::Target: Logger;
1631
}
1732

1833
#[derive(Clone, Debug, Default)]
@@ -37,6 +52,13 @@ impl TxBuilder for SpecTxBuilder {
3752
let mut local_balance_before_fee_msat = value_to_self_after_htlcs;
3853
let mut remote_balance_before_fee_msat = value_to_remote_after_htlcs;
3954

55+
// We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater
56+
// than or equal to `total_anchors_sat`.
57+
//
58+
// This is because when the remote party sends an `update_fee` message, we build the new
59+
// commitment transaction *before* checking whether the remote party's balance is enough to
60+
// cover the total anchor sum.
61+
4062
if is_outbound_from_holder {
4163
local_balance_before_fee_msat =
4264
local_balance_before_fee_msat.saturating_sub(total_anchors_sat * 1000);
@@ -47,4 +69,133 @@ impl TxBuilder for SpecTxBuilder {
4769

4870
(local_balance_before_fee_msat, remote_balance_before_fee_msat)
4971
}
72+
fn build_commitment_transaction<L: Deref>(
73+
&self, local: bool, commitment_number: u64, per_commitment_point: &PublicKey,
74+
channel_parameters: &ChannelTransactionParameters, secp_ctx: &Secp256k1<secp256k1::All>,
75+
value_to_self_msat: u64, mut htlcs_in_tx: Vec<HTLCOutputInCommitment>, feerate_per_kw: u32,
76+
broadcaster_dust_limit_satoshis: u64, logger: &L,
77+
) -> (CommitmentTransaction, CommitmentStats)
78+
where
79+
L::Target: Logger,
80+
{
81+
let mut htlcs_from_local_balance_msat = 0;
82+
let mut htlcs_from_remote_balance_msat = 0;
83+
84+
// Trim dust htlcs
85+
htlcs_in_tx.retain(|htlc| {
86+
if htlc.offered == local {
87+
// This is an outbound htlc
88+
htlcs_from_local_balance_msat += htlc.amount_msat;
89+
} else {
90+
htlcs_from_remote_balance_msat += htlc.amount_msat;
91+
}
92+
if htlc.is_dust(
93+
feerate_per_kw,
94+
broadcaster_dust_limit_satoshis,
95+
&channel_parameters.channel_type_features,
96+
) {
97+
log_trace!(
98+
logger,
99+
" ...trimming {} HTLC with value {}sat, hash {}, due to dust limit {}",
100+
if htlc.offered == local { "outbound" } else { "inbound" },
101+
htlc.amount_msat / 1000,
102+
htlc.payment_hash,
103+
broadcaster_dust_limit_satoshis
104+
);
105+
false
106+
} else {
107+
true
108+
}
109+
});
110+
111+
// # Panics
112+
//
113+
// The value going to each party MUST be 0 or positive, even if all HTLCs pending in the
114+
// commitment clear by failure.
115+
116+
let (local_balance_before_fee_msat, remote_balance_before_fee_msat) = self
117+
.balances_excluding_tx_fee(
118+
channel_parameters.is_outbound_from_holder,
119+
&channel_parameters.channel_type_features,
120+
value_to_self_msat.checked_sub(htlcs_from_local_balance_msat).unwrap(),
121+
(channel_parameters.channel_value_satoshis * 1000)
122+
.checked_sub(value_to_self_msat)
123+
.unwrap()
124+
.checked_sub(htlcs_from_remote_balance_msat)
125+
.unwrap(),
126+
);
127+
let total_fee_sat = self.commit_tx_fee_sat(
128+
feerate_per_kw,
129+
htlcs_in_tx.len(),
130+
&channel_parameters.channel_type_features,
131+
);
132+
let stats = CommitmentStats {
133+
total_fee_sat,
134+
local_balance_before_fee_msat,
135+
remote_balance_before_fee_msat,
136+
};
137+
138+
// We MUST use saturating subs here, as the funder's balance is not guaranteed to be greater
139+
// than or equal to `total_fee_sat`.
140+
//
141+
// This is because when the remote party sends an `update_fee` message, we build the new
142+
// commitment transaction *before* checking whether the remote party's balance is enough to
143+
// cover the total fee.
144+
145+
let (value_to_self, value_to_remote) = if channel_parameters.is_outbound_from_holder {
146+
(
147+
(local_balance_before_fee_msat / 1000).saturating_sub(total_fee_sat),
148+
remote_balance_before_fee_msat / 1000,
149+
)
150+
} else {
151+
(
152+
local_balance_before_fee_msat / 1000,
153+
(remote_balance_before_fee_msat / 1000).saturating_sub(total_fee_sat),
154+
)
155+
};
156+
157+
let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote };
158+
let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self };
159+
160+
if to_broadcaster_value_sat >= broadcaster_dust_limit_satoshis {
161+
log_trace!(
162+
logger,
163+
" ...including {} output with value {}",
164+
if local { "to_local" } else { "to_remote" },
165+
to_broadcaster_value_sat
166+
);
167+
} else {
168+
to_broadcaster_value_sat = 0;
169+
}
170+
171+
if to_countersignatory_value_sat >= broadcaster_dust_limit_satoshis {
172+
log_trace!(
173+
logger,
174+
" ...including {} output with value {}",
175+
if local { "to_remote" } else { "to_local" },
176+
to_countersignatory_value_sat
177+
);
178+
} else {
179+
to_countersignatory_value_sat = 0;
180+
}
181+
182+
let directed_parameters = if local {
183+
channel_parameters.as_holder_broadcastable()
184+
} else {
185+
channel_parameters.as_counterparty_broadcastable()
186+
};
187+
188+
let tx = CommitmentTransaction::new(
189+
commitment_number,
190+
per_commitment_point,
191+
to_broadcaster_value_sat,
192+
to_countersignatory_value_sat,
193+
feerate_per_kw,
194+
htlcs_in_tx,
195+
&directed_parameters,
196+
secp_ctx,
197+
);
198+
199+
(tx, stats)
200+
}
50201
}

0 commit comments

Comments
 (0)