Skip to content

Commit 797cf5b

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 db6dd94 commit 797cf5b

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
@@ -4341,16 +4341,21 @@ where
43414341
let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(
43424342
&fee_estimator, funding.get_channel_type(),
43434343
);
4344-
let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate);
4344+
// 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)
4345+
let include_counterparty_unknown_htlcs = false;
4346+
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);
4347+
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);
4348+
43454349
let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
4346-
if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4350+
if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43474351
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)",
4348-
msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat)));
4352+
msg.feerate_per_kw, next_local_commitment_stats.dust_exposure_msat)));
43494353
}
4350-
if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat {
4354+
if next_remote_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
43514355
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)",
4352-
msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat)));
4356+
msg.feerate_per_kw, next_remote_commitment_stats.dust_exposure_msat)));
43534357
}
4358+
43544359
Ok(())
43554360
}
43564361

0 commit comments

Comments
 (0)