Skip to content

Commit ddb54fc

Browse files
committed
Stop including dust values in feerate affordability checks
When we or our counterparty are updating the fees on the channel, we currently check that the resulting balance is sufficient not only to meet the reserve threshold, but also not push it below dust. This isn't required in the BOLTs and may lead to spurious force-closures (which would be a bit safer, but reserve should always exceed the dust threshold). Worse, the current logic is broken - it compares the output value in *billionths of satoshis* to the dust limit in satoshis. Thus, the code is borderline dead anyway, but can overflow for channels with several million Bitcoin, causing the fuzzer to get mad (and lead to spurious force-closures for few-billion-dollar channels).
1 parent df1f981 commit ddb54fc

File tree

1 file changed

+4
-8
lines changed

1 file changed

+4
-8
lines changed

lightning/src/ln/channel.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -732,8 +732,8 @@ struct CommitmentStats<'a> {
732732
total_fee_sat: u64, // the total fee included in the transaction
733733
num_nondust_htlcs: usize, // the number of HTLC outputs (dust HTLCs *non*-included)
734734
htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
735-
local_balance_msat: u64, // local balance before fees but considering dust limits
736-
remote_balance_msat: u64, // remote balance before fees but considering dust limits
735+
local_balance_msat: u64, // local balance before fees *not* considering dust limits
736+
remote_balance_msat: u64, // remote balance before fees *not* considering dust limits
737737
outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
738738
inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
739739
}
@@ -1728,13 +1728,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
17281728
}
17291729
}
17301730

1731-
let mut value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
1731+
let value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
17321732
assert!(value_to_self_msat >= 0);
17331733
// Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie
17341734
// AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to
17351735
// "violate" their reserve value by couting those against it. Thus, we have to convert
17361736
// everything to i64 before subtracting as otherwise we can overflow.
1737-
let mut value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
1737+
let value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
17381738
assert!(value_to_remote_msat >= 0);
17391739

17401740
#[cfg(debug_assertions)]
@@ -1800,10 +1800,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
18001800
htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
18011801
htlcs_included.append(&mut included_dust_htlcs);
18021802

1803-
// For the stats, trimmed-to-0 the value in msats accordingly
1804-
value_to_self_msat = if (value_to_self_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_self_msat };
1805-
value_to_remote_msat = if (value_to_remote_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_remote_msat };
1806-
18071803
CommitmentStats {
18081804
tx,
18091805
feerate_per_kw,

0 commit comments

Comments
 (0)