Skip to content

Commit a57c122

Browse files
committed
Limit inbound fee updates by dust exposure instead of our estimator
Inbound fee udpates are rather broken in lightning as they can impact the non-fundee despite the funder paying the fee, but only in the dust exposure it places on the fundee. At least lnd is fairly aggressively high in their (non-anchor) fee estimation, running the risk of force-closure. Further, because we relied on a fee estimator we don't have full control over, we were assuming our users' fees are particularly conservative, and thus were at a lot of risk to force-closures. This converts our fee limiting to use an absurd upper bound, focusing on whether we are over-exposed to in-flight dust when we receive an update_fee.
1 parent 2d0a8d5 commit a57c122

File tree

1 file changed

+28
-2
lines changed

1 file changed

+28
-2
lines changed

lightning/src/ln/channel.rs

+28-2
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,12 @@ impl<Signer: Sign> Channel<Signer> {
750750
if feerate_per_kw < lower_limit {
751751
return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
752752
}
753-
let upper_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 2;
753+
// We only bound the fee updates on the upper side to prevent completely absurd feerates,
754+
// always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
755+
// We generally don't care too much if they set the feerate to something very high, but it
756+
// could result in the channel being useless due to everything being dust.
757+
let upper_limit = cmp::max(250 * 25,
758+
fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
754759
if feerate_per_kw as u64 > upper_limit {
755760
return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
756761
}
@@ -3111,8 +3116,25 @@ impl<Signer: Sign> Channel<Signer> {
31113116
return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned()));
31123117
}
31133118
Channel::<Signer>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
3119+
let feerate_over_dust_buffer = msg.feerate_per_kw > self.get_dust_buffer_feerate();
3120+
31143121
self.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
31153122
self.update_time_counter += 1;
3123+
// If the feerate has increased over the previous dust buffer (note that
3124+
// `get_dust_buffer_feerate` considers the `pending_update_fee` status), check that we
3125+
// won't be pushed over our dust exposure limit by the feerate increase.
3126+
if feerate_over_dust_buffer {
3127+
let inbound_stats = self.get_inbound_pending_htlc_stats();
3128+
let outbound_stats = self.get_outbound_pending_htlc_stats();
3129+
if inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat > self.get_max_dust_htlc_exposure_msat() {
3130+
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)",
3131+
msg.feerate_per_kw, inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat)));
3132+
}
3133+
if inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat > self.get_max_dust_htlc_exposure_msat() {
3134+
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)",
3135+
msg.feerate_per_kw, inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat)));
3136+
}
3137+
}
31163138
Ok(())
31173139
}
31183140

@@ -3684,7 +3706,11 @@ impl<Signer: Sign> Channel<Signer> {
36843706
// whichever is higher. This ensures that we aren't suddenly exposed to significantly
36853707
// more dust balance if the feerate increases when we have several HTLCs pending
36863708
// which are near the dust limit.
3687-
cmp::max(2530, self.feerate_per_kw * 1250 / 1000)
3709+
let mut feerate_per_kw = self.feerate_per_kw;
3710+
if let Some((feerate, _)) = self.pending_update_fee {
3711+
feerate_per_kw = cmp::max(feerate_per_kw, feerate);
3712+
}
3713+
cmp::max(2530, feerate_per_kw * 1250 / 1000)
36883714
}
36893715

36903716
pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 {

0 commit comments

Comments
 (0)