Skip to content

Commit 1c0fe1f

Browse files
committed
Improve prediction of commitment stats in validate_update_fee
`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs on the next commitment will be all the HTLCs in `ChannelContext.pending_inbound_htlcs`, and `ChannelContext.pending_outbound_htlcs`, as well as all the outbound HTLC adds in the holding cell. This is an overestimate: * Outbound HTLC removals which have been ACK'ed by the counterparty will certainly not be present in any *next* commitment, even though they remain in `pending_outbound_htlcs`. * Outbound HTLCs in the `RemoteRemoved` state, will not be present in the next *local* commitment. * Outbound HTLCs in the `LocalAnnounced` state have no guarantee that they were received by the counterparty before she sent the `update_fee`. * Outbound `update_add_htlc`'s in the holding cell are certainly not known by the counterparty, and we will reevaluate their addition to the channel when freeing the holding cell. * Inbound HTLCs in the `LocalRemoved` state will not be present in the next *remote* commitment. This commit stops using `get_pending_htlc_stats` in favor of the newly added `ChannelContext::get_next_{local, remote}_commitment_stats` methods, and fixes the issues described above.
1 parent b8ff94d commit 1c0fe1f

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

lightning/src/ln/channel.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4339,16 +4339,21 @@ where
43394339
let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(
43404340
&fee_estimator, funding.get_channel_type(),
43414341
);
4342-
let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate);
4342+
// Do not include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs)
4343+
let do_not_include_counterparty_unknown_htlcs = false;
4344+
let next_local_commitment_stats = self.get_next_local_commitment_stats(funding, None, do_not_include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate);
4345+
let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, do_not_include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate);
4346+
43434347
let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
4344-
if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4348+
if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43454349
return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)",
4346-
msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat)));
4350+
msg.feerate_per_kw, next_local_commitment_stats.dust_exposure_msat)));
43474351
}
4348-
if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4352+
if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43494353
return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)",
4350-
msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat)));
4354+
msg.feerate_per_kw, next_remote_commitment_stats.dust_exposure_msat)));
43514355
}
4356+
43524357
Ok(())
43534358
}
43544359

0 commit comments

Comments
 (0)