Skip to content

Commit d3af49e

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 01e8ff5 commit d3af49e

File tree

1 file changed

+30
-2
lines changed

1 file changed

+30
-2
lines changed

lightning/src/ln/channel.rs

+30-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,27 @@ 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+
let holder_tx_dust_exposure = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat;
3130+
let counterparty_tx_dust_exposure = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat;
3131+
if holder_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
3132+
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)",
3133+
msg.feerate_per_kw, holder_tx_dust_exposure)));
3134+
}
3135+
if counterparty_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
3136+
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)",
3137+
msg.feerate_per_kw, counterparty_tx_dust_exposure)));
3138+
}
3139+
}
31163140
Ok(())
31173141
}
31183142

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

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

0 commit comments

Comments
 (0)