Skip to content

Commit 9baac2f

Browse files
committed
More flexible fee rate estimates
1 parent 2c51080 commit 9baac2f

File tree

9 files changed

+169
-96
lines changed

9 files changed

+169
-96
lines changed

fuzz/src/chanmon_consistency.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,10 @@ impl FeeEstimator for FuzzEstimator {
8080
// always return a HighPriority feerate here which is >= the maximum Normal feerate and a
8181
// Background feerate which is <= the minimum Normal feerate.
8282
match conf_target {
83-
ConfirmationTarget::HighPriority => MAX_FEE,
84-
ConfirmationTarget::Background|ConfirmationTarget::MempoolMinimum => 253,
85-
ConfirmationTarget::Normal => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE),
83+
ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee => MAX_FEE * 10,
84+
ConfirmationTarget::OnChainSweep => MAX_FEE,
85+
ConfirmationTarget::ChannelCloseMinimum|ConfirmationTarget::AnchorChannelFee|ConfirmationTarget::MinAllowedAnchorChannelRemoteFee|ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => 253,
86+
ConfirmationTarget::NonAnchorChannelFee => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE),
8687
}
8788
}
8889
}

lightning/src/chain/chaininterface.rs

+101-17
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,105 @@ pub trait BroadcasterInterface {
4848
/// estimation.
4949
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
5050
pub enum ConfirmationTarget {
51-
/// We'd like a transaction to confirm in the future, but don't want to commit most of the fees
52-
/// required to do so yet. The remaining fees will come via a Child-Pays-For-Parent (CPFP) fee
53-
/// bump of the transaction.
54-
///
55-
/// The feerate returned should be the absolute minimum feerate required to enter most node
56-
/// mempools across the network. Note that if you are not able to obtain this feerate estimate,
57-
/// you should likely use the furthest-out estimate allowed by your fee estimator.
58-
MempoolMinimum,
59-
/// We are happy with a transaction confirming slowly, at least within a day or so worth of
60-
/// blocks.
61-
Background,
62-
/// We'd like a transaction to confirm without major delayed, i.e., within the next 12-24 blocks.
63-
Normal,
64-
/// We'd like a transaction to confirm in the next few blocks.
65-
HighPriority,
51+
/// We have some funds available on chain which we need to spend prior to some expiry time at
52+
/// which point our counterparty may be able to steal them. Generally we have in the high tens
53+
/// to low hundreds of blocks to get our transaction on-chain, but we shouldn't risk too low a
54+
/// fee - this should be a relatively high priority feerate.
55+
OnChainSweep,
56+
/// The highest feerate we will allow our channel counterparty to have in a non-anchor channel.
57+
///
58+
/// This is the feerate on the transaction which we (or our counterparty) will broadcast in
59+
/// order to close the channel unilaterally. Because our counterparty must ensure they can
60+
/// always broadcast the latest state, this value being too low will cause immediate
61+
/// force-closures.
62+
///
63+
/// Allowing this value to be too high can allow our counterparty to burn our HTLC outputs to
64+
/// dust, which can result in HTLCs failing or force-closures (when the dust HTLCs exceed
65+
/// [`ChannelConfig::max_dust_htlc_exposure`]).
66+
///
67+
/// Because most nodes use a feerate estimate which is based on a relatively high priority
68+
/// transaction entering the current mempool, setting this to a small multiple of your current
69+
/// high priority feerate estimate should suffice.
70+
///
71+
/// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure
72+
MaxAllowedNonAnchorChannelRemoteFee,
73+
/// This is the feerate on the transaction which we (or our counterparty) will broadcast in
74+
/// order to close the channel if a channel party goes away. Because our counterparty must
75+
/// ensure they can always broadcast the latest state, this value being too low will cause
76+
/// immediate force-closures.
77+
///
78+
/// This needs to be sufficient to get into the mempool when the channel needs to
79+
/// be force-closed. Setting too low may result in force-closures. Because this is for anchor
80+
/// channels, we can always bump the feerate later, the feerate here only needs to suffice to
81+
/// enter the mempool.
82+
///
83+
/// A good estimate is the expected mempool minimum at the time of force-closure. Obviously this
84+
/// is not an estimate which is very easy to calculate because we do not know the future. Using
85+
/// a simple long-term fee estimate or tracking of the mempool minimum is a good approach to
86+
/// ensure you can always close the channel. A future change to Bitcoin's P2P network
87+
/// (package relay) may obviate the need for this entirely.
88+
MinAllowedAnchorChannelRemoteFee,
89+
/// This is the feerate on the transaction which we (or our counterparty) will broadcast in
90+
/// order to close the channel if a channel party goes away. Because our counterparty must
91+
/// ensure they can always broadcast the latest state, this value being too low will cause
92+
/// immediate force-closures.
93+
///
94+
/// The lowest feerate we will allow our channel counterparty to have in a non-anchor channel.
95+
/// This needs to be sufficient to get confirmed when the channel needs to be force-closed.
96+
/// Setting too low may result in force-closures.
97+
///
98+
/// This feerate represents the fee we pick now, which must be sufficient to enter a block at an
99+
/// arbitrary time in the future. Obviously this is not an estimate which is very easy to
100+
/// calculate. This can leave channels subject to being unable to close if feerates rise, and in
101+
/// general you should prefer anchor channels to ensure you can increase the feerate when the
102+
/// transactions need broadcasting.
103+
///
104+
/// Do note some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw),
105+
/// causing occasional issues with feerate disagreements between an initiator that wants a
106+
/// feerate of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Adding 250 to your
107+
/// desired feerate here can help avoid this issue.
108+
///
109+
/// [`ChannelConfig::max_dust_htlc_exposure`]: crate::util::config::ChannelConfig::max_dust_htlc_exposure
110+
MinAllowedNonAnchorChannelRemoteFee,
111+
/// This is the feerate on the transaction which we (or our counterparty) will broadcast in
112+
/// order to close the channel if a channel party goes away. Because our counterparty must does
113+
/// not want to burn all their funds to fees, this value being too high can cause
114+
/// force-closures.
115+
///
116+
/// This needs to be sufficient to get into the mempool when the channel needs to
117+
/// be force-closed. Setting too low may result in force-closures. Because this is for anchor
118+
/// channels, it can be a low value as we can always bump the feerate later.
119+
///
120+
/// A good estimate is the expected mempool minimum at the time of force-closure. Obviously this
121+
/// is not an estimate which is very easy to calculate because we do not know the future. Using
122+
/// a simple long-term fee estimate or tracking of the mempool minimum is a good approach to
123+
/// ensure you can always close the channel. A future change to Bitcoin's P2P network
124+
/// (package relay) may obviate the need for this entirely.
125+
AnchorChannelFee,
126+
/// Lightning is built around the ability to broadcast a transaction in the future to close our
127+
/// channel and claim all pending funds. In order to do so, non-anchor channels are built with
128+
/// transactions which we need to be able to broadcast at some point in the future.
129+
///
130+
/// This feerate represents the fee we pick now, which must be sufficient to enter a block at an
131+
/// arbitrary time in the future. Obviously this is not an estimate which is very easy to
132+
/// calculate, so most lightning nodes use some relatively high-priority feerate using the
133+
/// current mempool. This leaves channels subject to being unable to close if feerates rise, and
134+
/// in general you should prefer anchor channels to ensure you can increase the feerate when the
135+
/// transactions need broadcasting.
136+
///
137+
/// Since this is a should represent the feerate of a channel close that does not need fee
138+
/// bumping. This is also used as an upperbound for our attempted feerate when doing cooperative
139+
/// closure of any channel.
140+
NonAnchorChannelFee,
141+
/// When cooperatively closing a channel, this is the minimum feerate we will accept.
142+
/// Recommended at least within a day or so worth of blocks.
143+
///
144+
/// This will also be used when initiating a cooperative close of a channel. When closing a
145+
/// channel you can override this fee by using
146+
/// [`ChannelManager::close_channel_with_feerate_and_script`].
147+
///
148+
/// [`ChannelManager::close_channel_with_feerate_and_script`]: crate::ln::channelmanager::ChannelManager::close_channel_with_feerate_and_script
149+
ChannelCloseMinimum,
66150
}
67151

68152
/// A trait which should be implemented to provide feerate information on a number of time
@@ -135,7 +219,7 @@ mod tests {
135219
let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
136220
let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator);
137221

138-
assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), FEERATE_FLOOR_SATS_PER_KW);
222+
assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), FEERATE_FLOOR_SATS_PER_KW);
139223
}
140224

141225
#[test]
@@ -144,6 +228,6 @@ mod tests {
144228
let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
145229
let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator);
146230

147-
assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), sat_per_kw);
231+
assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), sat_per_kw);
148232
}
149233
}

lightning/src/chain/onchaintx.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
580580
if cached_request.is_malleable() {
581581
if cached_request.requires_external_funding() {
582582
let target_feerate_sat_per_1000_weight = cached_request.compute_package_feerate(
583-
fee_estimator, ConfirmationTarget::HighPriority, force_feerate_bump
583+
fee_estimator, ConfirmationTarget::OnChainSweep, force_feerate_bump
584584
);
585585
if let Some(htlcs) = cached_request.construct_malleable_package_with_external_funding(self) {
586586
return Some((
@@ -631,7 +631,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
631631
debug_assert_eq!(tx.txid(), self.holder_commitment.trust().txid(),
632632
"Holder commitment transaction mismatch");
633633

634-
let conf_target = ConfirmationTarget::HighPriority;
634+
let conf_target = ConfirmationTarget::OnChainSweep;
635635
let package_target_feerate_sat_per_1000_weight = cached_request
636636
.compute_package_feerate(fee_estimator, conf_target, force_feerate_bump);
637637
if let Some(input_amount_sat) = output.funding_amount {

lightning/src/chain/package.rs

+18-28
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use crate::ln::PaymentPreimage;
2424
use crate::ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment};
2525
use crate::ln::chan_utils;
2626
use crate::ln::msgs::DecodeError;
27-
use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
27+
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};
2828
use crate::sign::WriteableEcdsaChannelSigner;
2929
use crate::chain::onchaintx::{ExternalHTLCClaim, OnchainTxHandler};
3030
use crate::util::logger::Logger;
@@ -1101,40 +1101,30 @@ impl Readable for PackageTemplate {
11011101
}
11021102

11031103
/// Attempt to propose a bumping fee for a transaction from its spent output's values and predicted
1104-
/// weight. We start with the highest priority feerate returned by the node's fee estimator then
1105-
/// fall-back to lower priorities until we have enough value available to suck from.
1104+
/// weight. We start first try our [`OnChainSweep`] feerate, if it's not enough we try to sweep
1105+
/// half of the input amounts.
11061106
///
11071107
/// If the proposed fee is less than the available spent output's values, we return the proposed
1108-
/// fee and the corresponding updated feerate. If the proposed fee is equal or more than the
1109-
/// available spent output's values, we return nothing
1108+
/// fee and the corresponding updated feerate. If fee is under [`FEERATE_FLOOR_SATS_PER_KW`], we
1109+
/// return nothing.
1110+
///
1111+
/// [`OnChainSweep`]: crate::chain::chaininterface::ConfirmationTarget::OnChainSweep
1112+
/// [`FEERATE_FLOOR_SATS_PER_KW`]: crate::chain::chaininterface::MIN_RELAY_FEE_SAT_PER_1000_WEIGHT
11101113
fn compute_fee_from_spent_amounts<F: Deref, L: Deref>(input_amounts: u64, predicted_weight: usize, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L) -> Option<(u64, u64)>
11111114
where F::Target: FeeEstimator,
11121115
L::Target: Logger,
11131116
{
1114-
let mut updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64;
1115-
let mut fee = updated_feerate * (predicted_weight as u64) / 1000;
1116-
if input_amounts <= fee {
1117-
updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal) as u64;
1118-
fee = updated_feerate * (predicted_weight as u64) / 1000;
1119-
if input_amounts <= fee {
1120-
updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background) as u64;
1121-
fee = updated_feerate * (predicted_weight as u64) / 1000;
1122-
if input_amounts <= fee {
1123-
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)",
1124-
fee, input_amounts);
1125-
None
1126-
} else {
1127-
log_warn!(logger, "Used low priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
1128-
input_amounts);
1129-
Some((fee, updated_feerate))
1130-
}
1131-
} else {
1132-
log_warn!(logger, "Used medium priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
1133-
input_amounts);
1134-
Some((fee, updated_feerate))
1135-
}
1117+
let sweep_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::OnChainSweep);
1118+
let fee_rate = cmp::min(sweep_feerate, compute_feerate_sat_per_1000_weight(input_amounts / 2, predicted_weight as u64)) as u64;
1119+
let fee = fee_rate * (predicted_weight as u64) / 1000;
1120+
1121+
// if the fee rate is below the floor, we don't sweep
1122+
if fee_rate < FEERATE_FLOOR_SATS_PER_KW as u64 {
1123+
log_error!(logger, "Failed to generate an on-chain tx with fee ({} sat/kw) was less than the floor ({} sat/kw)",
1124+
fee_rate, FEERATE_FLOOR_SATS_PER_KW);
1125+
None
11361126
} else {
1137-
Some((fee, updated_feerate))
1127+
Some((fee, fee_rate))
11381128
}
11391129
}
11401130

lightning/src/ln/channel.rs

+14-20
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
11681168
match self.config.options.max_dust_htlc_exposure {
11691169
MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
11701170
let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(
1171-
ConfirmationTarget::HighPriority);
1171+
ConfirmationTarget::OnChainSweep);
11721172
feerate_per_kw as u64 * multiplier
11731173
},
11741174
MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
@@ -2155,28 +2155,20 @@ impl<SP: Deref> Channel<SP> where
21552155
// apply to channels supporting anchor outputs since HTLC transactions are pre-signed with a
21562156
// zero fee, so their fee is no longer considered to determine dust limits.
21572157
if !channel_type.supports_anchors_zero_fee_htlc_tx() {
2158-
let upper_limit = cmp::max(250 * 25,
2159-
fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
2158+
let upper_limit =
2159+
fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee) as u64;
21602160
if feerate_per_kw as u64 > upper_limit {
21612161
return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
21622162
}
21632163
}
21642164

2165-
// We can afford to use a lower bound with anchors than previously since we can now bump
2166-
// fees when broadcasting our commitment. However, we must still make sure we meet the
2167-
// minimum mempool feerate, until package relay is deployed, such that we can ensure the
2168-
// commitment transaction propagates throughout node mempools on its own.
21692165
let lower_limit_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
2170-
ConfirmationTarget::MempoolMinimum
2166+
ConfirmationTarget::MinAllowedAnchorChannelRemoteFee
21712167
} else {
2172-
ConfirmationTarget::Background
2168+
ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee
21732169
};
21742170
let lower_limit = fee_estimator.bounded_sat_per_1000_weight(lower_limit_conf_target);
2175-
// Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
2176-
// occasional issues with feerate disagreements between an initiator that wants a feerate
2177-
// of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
2178-
// sat/kw before the comparison here.
2179-
if feerate_per_kw + 250 < lower_limit {
2171+
if feerate_per_kw < lower_limit {
21802172
if let Some(cur_feerate) = cur_feerate_per_kw {
21812173
if feerate_per_kw > cur_feerate {
21822174
log_warn!(logger,
@@ -2185,7 +2177,7 @@ impl<SP: Deref> Channel<SP> where
21852177
return Ok(());
21862178
}
21872179
}
2188-
return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
2180+
return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
21892181
}
21902182
Ok(())
21912183
}
@@ -4155,8 +4147,10 @@ impl<SP: Deref> Channel<SP> where
41554147
// Propose a range from our current Background feerate to our Normal feerate plus our
41564148
// force_close_avoidance_max_fee_satoshis.
41574149
// If we fail to come to consensus, we'll have to force-close.
4158-
let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background);
4159-
let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
4150+
let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum);
4151+
// Use NonAnchorChannelFee because this should be an estimate for a channel close
4152+
// that we don't expect to need fee bumping
4153+
let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
41604154
let mut proposed_max_feerate = if self.context.is_outbound() { normal_feerate } else { u32::max_value() };
41614155

41624156
// The spec requires that (when the channel does not have anchors) we only send absolute
@@ -5727,9 +5721,9 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
57275721
debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
57285722

57295723
let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
5730-
ConfirmationTarget::MempoolMinimum
5724+
ConfirmationTarget::AnchorChannelFee
57315725
} else {
5732-
ConfirmationTarget::Normal
5726+
ConfirmationTarget::NonAnchorChannelFee
57335727
};
57345728
let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target);
57355729

@@ -6019,7 +6013,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
60196013
// whatever reason.
60206014
if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
60216015
self.context.channel_type.clear_anchors_zero_fee_htlc_tx();
6022-
self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
6016+
self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
60236017
assert!(!self.context.channel_transaction_parameters.channel_type_features.supports_anchors_nonzero_fee_htlc_tx());
60246018
} else if self.context.channel_type.supports_scid_privacy() {
60256019
self.context.channel_type.clear_scid_privacy();

0 commit comments

Comments
 (0)