Force-close channels if their feerate gets stale without any update
For quite some time, LDK has force-closed channels if the peer
sends us a feerate update which is below our FeeEstimator's
concept of a channel lower-bound. This is intended to ensure that
channel feerates are always sufficient to get our commitment
transaction confirmed on-chain if we do need to force-close.
However, we've never checked our channel feerate regularly - if a peer is offline (or just uninterested in updating the channel feerate) and the prevailing feerates on-chain go up, we'll simply ignore it and allow our commitment transaction to sit around with a feerate too low to get confirmed.
Here we rectify this oversight by force-closing channels with stale
feerates, checking after each block. However, because fee
estimators are often buggy and force-closures piss off users, we
only do so rather conservatively. Specifically, we only force-close
if a channel's feerate is below the minimum FeeEstimator-provided
minimum across the last day.
Further, because fee estimators are often especially buggy on startup (and because peers haven't had a chance to update the channel feerates yet), we don't force-close channels until we have a full day of feerate lower-bound history.
This should reduce the incidence of force-closures substantially,
but it is expected this will still increase force-closures somewhat
substantially depending on the users' FeeEstimator.
Fixes #993 Previous attempt was #1056
I thought about just closing #993 entirely without addressing it as we really don't want to do this, we really want Bitcoin Core 28 to ship with package relay, users to use anchor channels, and never force-close a channel for too-low-feerate issues again.
However, if we don't do this there's kinda no point in force-closing for too-low-feerate at all - it means an honest node that just has a bad fee estimator will get force-closed on but a malicious node will just be mum as feerates go up and leave us high and dry.
Codecov Report
Attention: Patch coverage is 55.80524% with 118 lines in your changes missing coverage. Please review.
Project coverage is 89.81%. Comparing base (
1d0c6c6) to head (17b77e0).
| Files | Patch % | Lines |
|---|---|---|
| lightning/src/ln/channel.rs | 35.22% | 97 Missing and 6 partials :warning: |
| lightning/src/ln/channelmanager.rs | 70.58% | 14 Missing and 1 partial :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## main #3037 +/- ##
==========================================
- Coverage 89.84% 89.81% -0.04%
==========================================
Files 119 119
Lines 97811 97914 +103
Branches 97811 97914 +103
==========================================
+ Hits 87883 87942 +59
- Misses 7364 7396 +32
- Partials 2564 2576 +12
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Because this uses the existing FeeEstimator MinAllowedAnchorChannelRemoteFee and MinAllowedNonAnchorChannelRemoteFee values, setting them to 0/253 will disable this entirely including the existing force-closes from peers sending us low values. Are you anticipating a desire to disable this without disabling the checking when peers do send update_fee?
Because this uses the existing
FeeEstimatorMinAllowedAnchorChannelRemoteFeeandMinAllowedNonAnchorChannelRemoteFeevalues, setting them to 0/253 will disable this entirely including the existing force-closes from peers sending us low values. Are you anticipating a desire to disable this without disabling the checking when peers do sendupdate_fee?
Thoughts on this @benthecarman?
yeah i guess that is fine
Basically LGTM pending 2nd reviewer. Needs rebase as well.
Rebased and added a test.
Oops, the fuzz failure was not unrelated. The full_stack_target fuzzer has a sanity check to ensure we get notified if the fuzz format changes implying our existing corpus is less useful, which it did here.
Rebased to address conflicts with upstream and squashed the fixup.
Merging, the diff since @arik-so's ACK is mostly just a squash:
$ git range-diff -U1 7d2d04798daa9c78f183424f73f3fea6c8564573...9cc99b27e0b6471629d35df79518b7d282041e17 1d0c6c60c6802126b3b29d6a2aa026c1aa33db02...17b77e0bcf0fd6721ac820df07a92ff697b3f50f
1: eecf12b38 = 1: 73bc0f61b Add `ChannelError::close` constructor
2: 6aa0d3004 ! 2: 3e09d9937 Use `ChannelError::close` constructor when building a close variant
@@ lightning/src/ln/channel.rs: impl<SP: Deref> Channel<SP> where
for outp in closing_tx.trust().built_transaction().output.iter() {
- if !outp.script_pubkey.is_witness_program() && outp.value < MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS {
+ if !outp.script_pubkey.is_witness_program() && outp.value < Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS) {
- return Err(ChannelError::Close("Remote sent us a closing_signed with a dust output. Always use segwit closing scripts!".to_owned()));
3: 85fd47023 = 3: 93011c377 Allow `ChannelError` to specify the `ClosureReason`
4: 9f4d907b9 = 4: 88c291a9b Add a new `ClosureReason::PeerFeerateTooLow`
5: 3c7bfd7f8 ! 5: 66e6ee563 Skip fee reads in `full_stack_target` when connecting many blocks
@@ fuzz/src/full_stack.rs: use std::cell::RefCell;
+use std::sync::atomic::{AtomicU64,AtomicUsize,AtomicBool,Ordering};
- use bitcoin::bech32::u5;
+ use bech32::u5;
6: 898f6716a ! 6: 5a1cc288b Force-close channels if their feerate gets stale without any update
@@ Commit message
+ ## fuzz/src/full_stack.rs ##
+@@ fuzz/src/full_stack.rs: mod tests {
+ ext_from_hex("0c005e", &mut test);
+ // the funding transaction
+ ext_from_hex("020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0150c3000000000000220020ae0000000000000000000000000000000000000000000000000000000000000000000000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions, one per line
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // by now client should have sent a channel_ready (CHECK 3: SendChannelReady to 03000000 for chan 3d000000)
+
+ // inbound read from peer id 0 of len 18
+@@ fuzz/src/full_stack.rs: mod tests {
+ ext_from_hex("0c007d", &mut test);
+ // the commitment transaction for channel 3f00000000000000000000000000000000000000000000000000000000000000
+ ext_from_hex("02000000013a000000000000000000000000000000000000000000000000000000000000000000000000000000800258020000000000002200204b0000000000000000000000000000000000000000000000000000000000000014c0000000000000160014280000000000000000000000000000000000000005000020", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ //
+ // connect a block with one transaction of len 94
+ ext_from_hex("0c005e", &mut test);
+ // the HTLC timeout transaction
+ ext_from_hex("0200000001730000000000000000000000000000000000000000000000000000000000000000000000000000000001a701000000000000220020b20000000000000000000000000000000000000000000000000000000000000000000000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+ // connect a block with no transactions
+ ext_from_hex("0c0000", &mut test);
++ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
+
+ // process the now-pending HTLC forward
+ ext_from_hex("07", &mut test);
+
## lightning/src/ln/channel.rs ##
7: 518261f93 < -: --------- f
8: 9cc99b27e = 7: 17b77e0bc Add a test of stale-feerate-force-closure behavior
$ git show 518261f93
commit 518261f9382417923d1b5f3ad10b6c38ba2baa70
Author: Matt Corallo <[email protected]>
Date: Fri May 31 17:44:01 2024 +0000
f
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index f9db2540e..7ab980f34 100644
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -911,23 +911,38 @@ mod tests {
ext_from_hex("0022 ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679 3d00000000000000000000000000000000000000000000000000000000000000 0000 00000000000000000000000000000000000000000000000000000000000000210100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
// client should now respond with funding_signed (CHECK 2: type 35 to peer 03000000)
+
+
// connect a block with one transaction of len 94
ext_from_hex("0c005e", &mut test);
// the funding transaction
ext_from_hex("020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0150c3000000000000220020ae0000000000000000000000000000000000000000000000000000000000000000000000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions, one per line
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// by now client should have sent a channel_ready (CHECK 3: SendChannelReady to 03000000 for chan 3d000000)
// inbound read from peer id 0 of len 18
@@ -1297,21 +1312,28 @@ mod tests {
ext_from_hex("0c007d", &mut test);
// the commitment transaction for channel 3f00000000000000000000000000000000000000000000000000000000000000
ext_from_hex("02000000013a000000000000000000000000000000000000000000000000000000000000000000000000000000800258020000000000002200204b0000000000000000000000000000000000000000000000000000000000000014c0000000000000160014280000000000000000000000000000000000000005000020", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
//
// connect a block with one transaction of len 94
ext_from_hex("0c005e", &mut test);
// the HTLC timeout transaction
ext_from_hex("0200000001730000000000000000000000000000000000000000000000000000000000000000000000000000000001a701000000000000220020b20000000000000000000000000000000000000000000000000000000000000000000000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// connect a block with no transactions
ext_from_hex("0c0000", &mut test);
+ ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection
// process the now-pending HTLC forward
ext_from_hex("07", &mut test);