From dd15ab03944cfb6e71dd11b806ace1b4fa225bef Mon Sep 17 00:00:00 2001 From: benthecarman Date: Thu, 12 Oct 2023 15:43:30 -0500 Subject: [PATCH] More flexible fee rate estimates --- fuzz/src/chanmon_consistency.rs | 7 +- lightning/src/chain/chaininterface.rs | 116 ++++++++++++++++++++++---- lightning/src/chain/onchaintx.rs | 4 +- lightning/src/chain/package.rs | 46 ++++------ lightning/src/ln/channel.rs | 34 ++++---- lightning/src/ln/channelmanager.rs | 35 ++++---- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/config.rs | 10 +-- lightning/src/util/test_utils.rs | 9 +- 9 files changed, 167 insertions(+), 96 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 2a1b9e9a7..3bdd78167 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -80,9 +80,10 @@ 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::HighPriority => MAX_FEE, - ConfirmationTarget::Background|ConfirmationTarget::MempoolMinimum => 253, - ConfirmationTarget::Normal => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE), + ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee => MAX_FEE * 10, + ConfirmationTarget::OnChainSweep => MAX_FEE, + ConfirmationTarget::ChannelCloseMinimum|ConfirmationTarget::AnchorChannelFee|ConfirmationTarget::MinAllowedAnchorChannelRemoteFee|ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => 253, + ConfirmationTarget::NonAnchorChannelFee => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE), } } } diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index 913fa007d..dfb90135c 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -48,21 +48,103 @@ pub trait BroadcasterInterface { /// estimation. #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] pub enum ConfirmationTarget { - /// We'd like a transaction to confirm in the future, but don't want to commit most of the fees - /// required to do so yet. The remaining fees will come via a Child-Pays-For-Parent (CPFP) fee - /// bump of the transaction. - /// - /// The feerate returned should be the absolute minimum feerate required to enter most node - /// mempools across the network. Note that if you are not able to obtain this feerate estimate, - /// you should likely use the furthest-out estimate allowed by your fee estimator. - MempoolMinimum, - /// We are happy with a transaction confirming slowly, at least within a day or so worth of - /// blocks. - Background, - /// We'd like a transaction to confirm without major delayed, i.e., within the next 12-24 blocks. - Normal, - /// We'd like a transaction to confirm in the next few blocks. - HighPriority, + /// 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 + /// fee - this should be a relatively high priority feerate. + OnChainSweep, + /// The highest feerate we will allow our channel counterparty to have in a non-anchor channel. + /// + /// This is the feerate on the transaction which we (or our counterparty) will broadcast in + /// order to close the channel unilaterally. Because our counterparty must ensure they can + /// always broadcast the latest state, this value being too low will cause immediate + /// force-closures. + /// + /// Allowing this value to be too high can allow our counterparty to burn our HTLC outputs to + /// dust, which can result in HTLCs failing or force-closures (when the dust HTLCs exceed + /// [`ChannelConfig::max_dust_htlc_exposure`]). + /// + /// Because most nodes use a feerate estimate which is based on a relatively high priority + /// transaction entering the current mempool, setting this to a small multiple of your current + /// high priority feerate estimate should suffice. + /// + /// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure + MaxAllowedNonAnchorChannelRemoteFee, + /// This is the lowest feerate we will allow our channel counterparty to have in an anchor + /// channel in order to close the channel if a channel party goes away. Because our counterparty + /// must ensure they can always broadcast the latest state, this value being too high will cause + /// immediate force-closures. + /// + /// This needs to be sufficient to get into the mempool when the channel needs to + /// be force-closed. Setting too low may result in force-closures. Because this is for anchor + /// channels, we can always bump the feerate later, the feerate here only needs to suffice to + /// enter the mempool. + /// + /// A good estimate is the expected mempool minimum at the time of force-closure. Obviously this + /// is not an estimate which is very easy to calculate because we do not know the future. Using + /// a simple long-term fee estimate or tracking of the mempool minimum is a good approach to + /// ensure you can always close the channel. A future change to Bitcoin's P2P network + /// (package relay) may obviate the need for this entirely. + MinAllowedAnchorChannelRemoteFee, + /// The lowest feerate we will allow our channel counterparty to have in a non-anchor channel. + /// This needs to be sufficient to get confirmed when the channel needs to be force-closed. + /// Setting too low may result in force-closures. + /// + /// This is the feerate on the transaction which we (or our counterparty) will broadcast in + /// order to close the channel if a channel party goes away. Because our counterparty must + /// ensure they can always broadcast the latest state, this value being too high will cause + /// immediate force-closures. + /// + /// This feerate represents the fee we pick now, which must be sufficient to enter a block at an + /// arbitrary time in the future. Obviously this is not an estimate which is very easy to + /// calculate. This can leave channels subject to being unable to close if feerates rise, and in + /// general you should prefer anchor channels to ensure you can increase the feerate when the + /// transactions need broadcasting. + /// + /// Do note some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), + /// causing occasional issues with feerate disagreements between an initiator that wants a + /// feerate of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. If your fee + /// estimator rounds subtracting 250 to your desired feerate here can help avoid this issue. + /// + /// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure + MinAllowedNonAnchorChannelRemoteFee, + /// This is the feerate on the transaction which we (or our counterparty) will broadcast in + /// order to close the channel if a channel party goes away. + /// + /// This needs to be sufficient to get into the mempool when the channel needs to + /// be force-closed. Setting too low may result in force-closures. Because this is for anchor + /// channels, it can be a low value as we can always bump the feerate later. + /// + /// A good estimate is the expected mempool minimum at the time of force-closure. Obviously this + /// is not an estimate which is very easy to calculate because we do not know the future. Using + /// a simple long-term fee estimate or tracking of the mempool minimum is a good approach to + /// ensure you can always close the channel. A future change to Bitcoin's P2P network + /// (package relay) may obviate the need for this entirely. + AnchorChannelFee, + /// Lightning is built around the ability to broadcast a transaction in the future to close our + /// channel and claim all pending funds. In order to do so, non-anchor channels are built with + /// transactions which we need to be able to broadcast at some point in the future. + /// + /// This feerate represents the fee we pick now, which must be sufficient to enter a block at an + /// arbitrary time in the future. Obviously this is not an estimate which is very easy to + /// calculate, so most lightning nodes use some relatively high-priority feerate using the + /// current mempool. This leaves channels subject to being unable to close if feerates rise, and + /// in general you should prefer anchor channels to ensure you can increase the feerate when the + /// transactions need broadcasting. + /// + /// Since this should represent the feerate of a channel close that does not need fee + /// bumping, this is also used as an upper bound for our attempted feerate when doing cooperative + /// closure of any channel. + NonAnchorChannelFee, + /// When cooperatively closing a channel, this is the minimum feerate we will accept. + /// Recommended at least within a day or so worth of blocks. + /// + /// This will also be used when initiating a cooperative close of a channel. When closing a + /// channel you can override this fee by using + /// [`ChannelManager::close_channel_with_feerate_and_script`]. + /// + /// [`ChannelManager::close_channel_with_feerate_and_script`]: crate::ln::channelmanager::ChannelManager::close_channel_with_feerate_and_script + ChannelCloseMinimum, } /// A trait which should be implemented to provide feerate information on a number of time @@ -135,7 +217,7 @@ mod tests { let test_fee_estimator = &TestFeeEstimator { sat_per_kw }; let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator); - assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), FEERATE_FLOOR_SATS_PER_KW); + assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), FEERATE_FLOOR_SATS_PER_KW); } #[test] @@ -144,6 +226,6 @@ mod tests { let test_fee_estimator = &TestFeeEstimator { sat_per_kw }; let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator); - assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), sat_per_kw); + assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), sat_per_kw); } } diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 5daa2463d..b6c643180 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -580,7 +580,7 @@ impl OnchainTxHandler if cached_request.is_malleable() { if cached_request.requires_external_funding() { let target_feerate_sat_per_1000_weight = cached_request.compute_package_feerate( - fee_estimator, ConfirmationTarget::HighPriority, force_feerate_bump + fee_estimator, ConfirmationTarget::OnChainSweep, force_feerate_bump ); if let Some(htlcs) = cached_request.construct_malleable_package_with_external_funding(self) { return Some(( @@ -631,7 +631,7 @@ impl OnchainTxHandler debug_assert_eq!(tx.txid(), self.holder_commitment.trust().txid(), "Holder commitment transaction mismatch"); - let conf_target = ConfirmationTarget::HighPriority; + let conf_target = ConfirmationTarget::OnChainSweep; let package_target_feerate_sat_per_1000_weight = cached_request .compute_package_feerate(fee_estimator, conf_target, force_feerate_bump); if let Some(input_amount_sat) = output.funding_amount { diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index c888118bc..3d29e5a55 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -24,7 +24,7 @@ use crate::ln::PaymentPreimage; use crate::ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment}; use crate::ln::chan_utils; use crate::ln::msgs::DecodeError; -use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; +use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT, compute_feerate_sat_per_1000_weight, fee_for_weight, FEERATE_FLOOR_SATS_PER_KW}; use crate::sign::WriteableEcdsaChannelSigner; use crate::chain::onchaintx::{ExternalHTLCClaim, OnchainTxHandler}; use crate::util::logger::Logger; @@ -1101,40 +1101,30 @@ impl Readable for PackageTemplate { } /// Attempt to propose a bumping fee for a transaction from its spent output's values and predicted -/// weight. We start with the highest priority feerate returned by the node's fee estimator then -/// fall-back to lower priorities until we have enough value available to suck from. +/// weight. We first try our [`OnChainSweep`] feerate, if it's not enough we try to sweep half of +/// the input amounts. /// /// If the proposed fee is less than the available spent output's values, we return the proposed -/// fee and the corresponding updated feerate. If the proposed fee is equal or more than the -/// available spent output's values, we return nothing +/// fee and the corresponding updated feerate. If fee is under [`FEERATE_FLOOR_SATS_PER_KW`], we +/// return nothing. +/// +/// [`OnChainSweep`]: crate::chain::chaininterface::ConfirmationTarget::OnChainSweep +/// [`FEERATE_FLOOR_SATS_PER_KW`]: crate::chain::chaininterface::MIN_RELAY_FEE_SAT_PER_1000_WEIGHT fn compute_fee_from_spent_amounts(input_amounts: u64, predicted_weight: usize, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Option<(u64, u64)> where F::Target: FeeEstimator, L::Target: Logger, { - let mut updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64; - let mut fee = updated_feerate * (predicted_weight as u64) / 1000; - if input_amounts <= fee { - updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal) as u64; - fee = updated_feerate * (predicted_weight as u64) / 1000; - if input_amounts <= fee { - updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background) as u64; - fee = updated_feerate * (predicted_weight as u64) / 1000; - if input_amounts <= fee { - log_error!(logger, "Failed to generate an on-chain punishment tx as even low priority fee ({} sat) was more than the entire claim balance ({} sat)", - fee, input_amounts); - None - } else { - log_warn!(logger, "Used low priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)", - input_amounts); - Some((fee, updated_feerate)) - } - } else { - log_warn!(logger, "Used medium priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)", - input_amounts); - Some((fee, updated_feerate)) - } + let sweep_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::OnChainSweep); + let fee_rate = cmp::min(sweep_feerate, compute_feerate_sat_per_1000_weight(input_amounts / 2, predicted_weight as u64)) as u64; + let fee = fee_rate * (predicted_weight as u64) / 1000; + + // if the fee rate is below the floor, we don't sweep + if fee_rate < FEERATE_FLOOR_SATS_PER_KW as u64 { + log_error!(logger, "Failed to generate an on-chain tx with fee ({} sat/kw) was less than the floor ({} sat/kw)", + fee_rate, FEERATE_FLOOR_SATS_PER_KW); + None } else { - Some((fee, updated_feerate)) + Some((fee, fee_rate)) } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a61a8de82..43ea7a65b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1168,7 +1168,7 @@ impl ChannelContext where SP::Target: SignerProvider { match self.config.options.max_dust_htlc_exposure { MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => { let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight( - ConfirmationTarget::HighPriority); + ConfirmationTarget::OnChainSweep); feerate_per_kw as u64 * multiplier }, MaxDustHTLCExposure::FixedLimitMsat(limit) => limit, @@ -2155,28 +2155,20 @@ impl Channel where // apply to channels supporting anchor outputs since HTLC transactions are pre-signed with a // zero fee, so their fee is no longer considered to determine dust limits. if !channel_type.supports_anchors_zero_fee_htlc_tx() { - let upper_limit = cmp::max(250 * 25, - fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10); + let upper_limit = + fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee) as u64; if feerate_per_kw as u64 > upper_limit { return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit))); } } - // We can afford to use a lower bound with anchors than previously since we can now bump - // fees when broadcasting our commitment. However, we must still make sure we meet the - // minimum mempool feerate, until package relay is deployed, such that we can ensure the - // commitment transaction propagates throughout node mempools on its own. let lower_limit_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() { - ConfirmationTarget::MempoolMinimum + ConfirmationTarget::MinAllowedAnchorChannelRemoteFee } else { - ConfirmationTarget::Background + ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee }; let lower_limit = fee_estimator.bounded_sat_per_1000_weight(lower_limit_conf_target); - // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing - // occasional issues with feerate disagreements between an initiator that wants a feerate - // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250 - // sat/kw before the comparison here. - if feerate_per_kw + 250 < lower_limit { + if feerate_per_kw < lower_limit { if let Some(cur_feerate) = cur_feerate_per_kw { if feerate_per_kw > cur_feerate { log_warn!(logger, @@ -2185,7 +2177,7 @@ impl Channel where return Ok(()); } } - return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit))); + return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit))); } Ok(()) } @@ -4155,8 +4147,10 @@ impl Channel where // Propose a range from our current Background feerate to our Normal feerate plus our // force_close_avoidance_max_fee_satoshis. // If we fail to come to consensus, we'll have to force-close. - let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background); - let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); + let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum); + // Use NonAnchorChannelFee because this should be an estimate for a channel close + // that we don't expect to need fee bumping + let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); let mut proposed_max_feerate = if self.context.is_outbound() { normal_feerate } else { u32::max_value() }; // The spec requires that (when the channel does not have anchors) we only send absolute @@ -5727,9 +5721,9 @@ impl OutboundV1Channel where SP::Target: SignerProvider { debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config))); let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() { - ConfirmationTarget::MempoolMinimum + ConfirmationTarget::AnchorChannelFee } else { - ConfirmationTarget::Normal + ConfirmationTarget::NonAnchorChannelFee }; let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target); @@ -6019,7 +6013,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { // whatever reason. if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() { self.context.channel_type.clear_anchors_zero_fee_htlc_tx(); - self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); + self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); assert!(!self.context.channel_transaction_parameters.channel_type_features.supports_anchors_nonzero_fee_htlc_tx()); } else if self.context.channel_type.supports_scid_privacy() { self.context.channel_type.clear_scid_privacy(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 755cb2c76..4ed5b63f2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2640,11 +2640,11 @@ where /// will be accepted on the given channel, and after additional timeout/the closing of all /// pending HTLCs, the channel will be closed on chain. /// - /// * If we are the channel initiator, we will pay between our [`Background`] and - /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee - /// estimate. + /// * If we are the channel initiator, we will pay between our [`ChannelCloseMinimum`] and + /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`NonAnchorChannelFee`] + /// fee estimate. /// * If our counterparty is the channel initiator, we will require a channel closing - /// transaction feerate of at least our [`Background`] feerate or the feerate which + /// transaction feerate of at least our [`ChannelCloseMinimum`] feerate or the feerate which /// would appear on a force-closure transaction, whichever is lower. We will allow our /// counterparty to pay as much fee as they'd like, however. /// @@ -2656,8 +2656,8 @@ where /// channel. /// /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis - /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background - /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal + /// [`ChannelCloseMinimum`]: crate::chain::chaininterface::ConfirmationTarget::ChannelCloseMinimum + /// [`NonAnchorChannelFee`]: crate::chain::chaininterface::ConfirmationTarget::NonAnchorChannelFee /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown pub fn close_channel(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey) -> Result<(), APIError> { self.close_channel_internal(channel_id, counterparty_node_id, None, None) @@ -2671,8 +2671,8 @@ where /// the channel being closed or not: /// * If we are the channel initiator, we will pay at least this feerate on the closing /// transaction. The upper-bound is set by - /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee - /// estimate (or `target_feerate_sat_per_1000_weight`, if it is greater). + /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`NonAnchorChannelFee`] + /// fee estimate (or `target_feerate_sat_per_1000_weight`, if it is greater). /// * If our counterparty is the channel initiator, we will refuse to accept a channel closure /// transaction feerate below `target_feerate_sat_per_1000_weight` (or the feerate which /// will appear on a force-closure transaction, whichever is lower). @@ -2690,8 +2690,7 @@ where /// channel. /// /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis - /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background - /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal + /// [`NonAnchorChannelFee`]: crate::chain::chaininterface::ConfirmationTarget::NonAnchorChannelFee /// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown pub fn close_channel_with_feerate_and_script(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option, shutdown_script: Option) -> Result<(), APIError> { self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script) @@ -4754,8 +4753,8 @@ where PersistenceNotifierGuard::optionally_notify(self, || { let mut should_persist = NotifyOption::SkipPersistNoEvents; - let normal_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); - let min_mempool_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MempoolMinimum); + let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); + let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); let per_peer_state = self.per_peer_state.read().unwrap(); for (_cp_id, peer_state_mutex) in per_peer_state.iter() { @@ -4765,9 +4764,9 @@ where |(chan_id, phase)| if let ChannelPhase::Funded(chan) = phase { Some((chan_id, chan)) } else { None } ) { let new_feerate = if chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - min_mempool_feerate + anchor_feerate } else { - normal_feerate + non_anchor_feerate }; let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate); if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; } @@ -4799,8 +4798,8 @@ where PersistenceNotifierGuard::optionally_notify(self, || { let mut should_persist = NotifyOption::SkipPersistNoEvents; - let normal_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal); - let min_mempool_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MempoolMinimum); + let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); + let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee); let mut handle_errors: Vec<(Result<(), _>, _)> = Vec::new(); let mut timed_out_mpp_htlcs = Vec::new(); @@ -4847,9 +4846,9 @@ where match phase { ChannelPhase::Funded(chan) => { let new_feerate = if chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - min_mempool_feerate + anchor_feerate } else { - normal_feerate + non_anchor_feerate }; let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate); if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 5e90ee7e9..c459f1cd7 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -10014,7 +10014,7 @@ fn accept_busted_but_better_fee() { MessageSendEvent::UpdateHTLCs { updates: msgs::CommitmentUpdate { ref update_fee, .. }, .. } => { nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_fee.as_ref().unwrap()); check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { - err: "Peer's feerate much too low. Actual: 1000. Our expected lower limit: 5000 (- 250)".to_owned() }, + err: "Peer's feerate much too low. Actual: 1000. Our expected lower limit: 5000".to_owned() }, [nodes[0].node.get_our_node_id()], 100000); check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 267774481..ef4111e4f 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -438,20 +438,20 @@ pub struct ChannelConfig { /// funder/initiator. /// /// When we are the funder, because we have to pay the channel closing fee, we bound the - /// acceptable fee by our [`Background`] and [`Normal`] fees, with the upper bound increased by + /// acceptable fee by our [`ChannelCloseMinimum`] and [`NonAnchorChannelFee`] fees, with the upper bound increased by /// this value. Because the on-chain fee we'd pay to force-close the channel is kept near our - /// [`Normal`] feerate during normal operation, this value represents the additional fee we're + /// [`NonAnchorChannelFee`] feerate during normal operation, this value represents the additional fee we're /// willing to pay in order to avoid waiting for our counterparty's to_self_delay to reclaim our /// funds. /// /// When we are not the funder, we require the closing transaction fee pay at least our - /// [`Background`] fee estimate, but allow our counterparty to pay as much fee as they like. + /// [`ChannelCloseMinimum`] fee estimate, but allow our counterparty to pay as much fee as they like. /// Thus, this value is ignored when we are not the funder. /// /// Default value: 1000 satoshis. /// - /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal - /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background + /// [`NonAnchorChannelFee`]: crate::chain::chaininterface::ConfirmationTarget::NonAnchorChannelFee + /// [`ChannelCloseMinimum`]: crate::chain::chaininterface::ConfirmationTarget::ChannelCloseMinimum pub force_close_avoidance_max_fee_satoshis: u64, /// If set, allows this channel's counterparty to skim an additional fee off this node's inbound /// HTLCs. Useful for liquidity providers to offload on-chain channel costs to end users. diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index bd66e4cae..45247be7c 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -94,8 +94,13 @@ pub struct TestFeeEstimator { pub sat_per_kw: Mutex, } impl chaininterface::FeeEstimator for TestFeeEstimator { - fn get_est_sat_per_1000_weight(&self, _confirmation_target: ConfirmationTarget) -> u32 { - *self.sat_per_kw.lock().unwrap() + fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32 { + match confirmation_target { + ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee => { + core::cmp::max(25 * 250, *self.sat_per_kw.lock().unwrap() * 10) + } + _ => *self.sat_per_kw.lock().unwrap(), + } } } -- 2.39.5