Skip to content

Commit d36fdab

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. We now always calculate dust exposure using a buffer from `msg.feerate_per_kw`, and not from `max(self.feerate_per_kw, msg.feerate_per_kw)`.
1 parent 3218db1 commit d36fdab

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
@@ -4338,16 +4338,21 @@ where
43384338
let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(
43394339
&fee_estimator, funding.get_channel_type(),
43404340
);
4341-
let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate);
4341+
// 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)
4342+
let include_counterparty_unknown_htlcs = false;
4343+
let next_local_commitment_stats = self.get_next_local_commitment_stats(funding, None, include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate);
4344+
let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, 0, msg.feerate_per_kw, dust_exposure_limiting_feerate);
4345+
43424346
let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
4343-
if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4347+
if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43444348
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)",
4345-
msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat)));
4349+
msg.feerate_per_kw, next_local_commitment_stats.dust_exposure_msat)));
43464350
}
4347-
if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4351+
if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43484352
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)",
4349-
msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat)));
4353+
msg.feerate_per_kw, next_remote_commitment_stats.dust_exposure_msat)));
43504354
}
4355+
43514356
Ok(())
43524357
}
43534358

0 commit comments

Comments
 (0)