From f0de37ae1fb43587015959bb31961fe5a7f4d173 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 22 Aug 2024 15:25:56 +0000 Subject: [PATCH] Add a new `ConfirmationTarget::MaximumFeeEstimate` When we broke `ConfirmationTarget` out into task-specific names, we left `MaxDustHTLCExposure::FeeRateMultiplier` as using the "when we broadcast feerate" as we were mostly concerned about the dust thresholds on outbound channels where we pick the fee and drive our own funds to dust. In 51bf78d604b28fe189171e1b976fe87cbb2614b7, that changed to include transaction fees on both inbound and outbound channels in our dust exposure amount, but we continued to use `ConfirmationTarget::OnChainSweep` for the fee estimator threshold. While the `MaxDustHTLCExposure::FeeRateMultiplier` value is quite conservative and shouldn't lead to force-closures unless feerate estimates disagree by something like 500 sat/vB (with only one HTLC active in a channel), this happened on Aug 22 when feerates spiked from 4 sat/vB to over 1000 sat/vB in one block. To avoid simple feerate estimate horizons causing this in the future, here we add a new `ConfirmationTarget::MaximumFeeEstimate` which is used for dust calculations. This allows users to split out the estimates they use for checking counterparty feerates from the estimates used for actual broadcasting. --- fuzz/src/chanmon_consistency.rs | 2 +- lightning/src/chain/chaininterface.rs | 6 ++++++ lightning/src/ln/channel.rs | 2 +- lightning/src/util/config.rs | 14 +++++++------- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 84cc4bf26..7c50a5125 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -97,7 +97,7 @@ impl FeeEstimator for FuzzEstimator { // always return a HighPriority feerate here which is >= the maximum Normal feerate and a // Background feerate which is <= the minimum Normal feerate. match conf_target { - ConfirmationTarget::OnChainSweep => MAX_FEE, + ConfirmationTarget::MaximumFeeEstimate | ConfirmationTarget::OnChainSweep => MAX_FEE, ConfirmationTarget::ChannelCloseMinimum | ConfirmationTarget::AnchorChannelFee | ConfirmationTarget::MinAllowedAnchorChannelRemoteFee diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index ba9e9d1fe..0db327470 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -49,6 +49,12 @@ pub trait BroadcasterInterface { /// estimation. #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] pub enum ConfirmationTarget { + /// The most aggressive (i.e. highest) feerate estimate available. + /// + /// This is used to sanity-check our counterparty's feerates and should be as conservative as + /// possible to ensure that we don't confuse a peer using a very conservative estimator for one + /// trying to burn channel balance to dust. + MaximumFeeEstimate, /// We have some funds available on chain which we need to spend prior to some expiry time at /// which point our counterparty may be able to steal them. Generally we have in the high tens /// to low hundreds of blocks to get our transaction on-chain, but we shouldn't risk too low a diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f4d55f916..96ca8bac3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2442,7 +2442,7 @@ impl ChannelContext where SP::Target: SignerProvider { fn get_dust_exposure_limiting_feerate(&self, fee_estimator: &LowerBoundedFeeEstimator, ) -> u32 where F::Target: FeeEstimator { - fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::OnChainSweep) + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaximumFeeEstimate) } pub fn get_max_dust_htlc_exposure_msat(&self, limiting_feerate_sat_per_kw: u32) -> u64 { diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index f369e793e..31c975c9c 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -382,9 +382,9 @@ pub enum MaxDustHTLCExposure { /// to this maximum the channel may be unable to send/receive HTLCs between the maximum dust /// exposure and the new minimum value for HTLCs to be economically viable to claim. FixedLimitMsat(u64), - /// This sets a multiplier on the [`ConfirmationTarget::OnChainSweep`] feerate (in sats/KW) to - /// determine the maximum allowed dust exposure. If this variant is used then the maximum dust - /// exposure in millisatoshis is calculated as: + /// This sets a multiplier on the [`ConfirmationTarget::MaximumFeeEstimate`] feerate (in + /// sats/KW) to determine the maximum allowed dust exposure. If this variant is used then the + /// maximum dust exposure in millisatoshis is calculated as: /// `feerate_per_kw * value`. For example, with our default value /// `FeeRateMultiplier(10_000)`: /// @@ -407,7 +407,7 @@ pub enum MaxDustHTLCExposure { /// by default this will be set to a [`Self::FixedLimitMsat`] of 5,000,000 msat. /// /// [`FeeEstimator`]: crate::chain::chaininterface::FeeEstimator - /// [`ConfirmationTarget::OnChainSweep`]: crate::chain::chaininterface::ConfirmationTarget::OnChainSweep + /// [`ConfirmationTarget::MaximumFeeEstimate`]: crate::chain::chaininterface::ConfirmationTarget::MaximumFeeEstimate FeeRateMultiplier(u64), } @@ -514,12 +514,12 @@ pub struct ChannelConfig { /// Note that when using [`MaxDustHTLCExposure::FeeRateMultiplier`] this maximum disagreement /// will scale linearly with increases (or decreases) in the our feerate estimates. Further, /// for anchor channels we expect our counterparty to use a relatively low feerate estimate - /// while we use [`ConfirmationTarget::OnChainSweep`] (which should be relatively high) and - /// feerate disagreement force-closures should only occur when theirs is higher than ours. + /// while we use [`ConfirmationTarget::MaximumFeeEstimate`] (which should be relatively high) + /// and feerate disagreement force-closures should only occur when theirs is higher than ours. /// /// Default value: [`MaxDustHTLCExposure::FeeRateMultiplier`] with a multiplier of `10_000` /// - /// [`ConfirmationTarget::OnChainSweep`]: crate::chain::chaininterface::ConfirmationTarget::OnChainSweep + /// [`ConfirmationTarget::MaximumFeeEstimate`]: crate::chain::chaininterface::ConfirmationTarget::MaximumFeeEstimate pub max_dust_htlc_exposure: MaxDustHTLCExposure, /// The additional fee we're willing to pay to avoid waiting for the counterparty's /// `to_self_delay` to reclaim funds. -- 2.39.5