Skip to content

Force-close channels if their feerate gets stale without any update #3037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 11, 2024
Merged

Force-close channels if their feerate gets stale without any update #3037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
73bc0f6
Add `ChannelError::close` constructor
TheBlueMatt May 5, 2024
3e09d99
Use `ChannelError::close` constructor when building a close variant
TheBlueMatt May 5, 2024
93011c3
Allow `ChannelError` to specify the `ClosureReason`
TheBlueMatt May 6, 2024
88c291a
Add a new `ClosureReason::PeerFeerateTooLow`
TheBlueMatt May 5, 2024
66e6ee5
Skip fee reads in `full_stack_target` when connecting many blocks
TheBlueMatt May 31, 2024
5a1cc28
Force-close channels if their feerate gets stale without any update
TheBlueMatt May 5, 2024
17b77e0
Add a test of stale-feerate-force-closure behavior
TheBlueMatt May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion fuzz/src/full_stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ use std::cell::RefCell;
use std::convert::TryInto;
use std::cmp;
use std::sync::{Arc, Mutex};
use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering};
use std::sync::atomic::{AtomicU64,AtomicUsize,AtomicBool,Ordering};
use bech32::u5;

#[inline]
Expand All @@ -98,6 +98,7 @@ pub fn slice_to_be24(v: &[u8]) -> u32 {
struct InputData {
data: Vec<u8>,
read_pos: AtomicUsize,
halt_fee_est_reads: AtomicBool,
}
impl InputData {
fn get_slice(&self, len: usize) -> Option<&[u8]> {
Expand All @@ -124,6 +125,9 @@ struct FuzzEstimator {
}
impl FeeEstimator for FuzzEstimator {
fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 {
if self.input.halt_fee_est_reads.load(Ordering::Acquire) {
return 253;
}
//TODO: We should actually be testing at least much more than 64k...
match self.input.get_slice(2) {
Some(slice) => cmp::max(slice_to_be16(slice) as u32, 253),
Expand Down Expand Up @@ -446,6 +450,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger>) {
let input = Arc::new(InputData {
data: data.to_vec(),
read_pos: AtomicUsize::new(0),
halt_fee_est_reads: AtomicBool::new(false),
});
let fee_est = Arc::new(FuzzEstimator {
input: input.clone(),
Expand Down Expand Up @@ -703,10 +708,12 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger>) {
11 => {
let mut txn = broadcast.txn_broadcasted.lock().unwrap().split_off(0);
if !txn.is_empty() {
input.halt_fee_est_reads.store(true, Ordering::Release);
loss_detector.connect_block(&txn[..]);
for _ in 2..100 {
loss_detector.connect_block(&[]);
}
input.halt_fee_est_reads.store(false, Ordering::Release);
}
for tx in txn.drain(..) {
loss_detector.funding_txn.push(tx);
Expand Down Expand Up @@ -914,19 +921,32 @@ 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
Expand Down Expand Up @@ -1296,21 +1316,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);
Expand Down
24 changes: 24 additions & 0 deletions lightning/src/events/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,21 @@ pub enum ClosureReason {
FundingBatchClosure,
/// One of our HTLCs timed out in a channel, causing us to force close the channel.
HTLCsTimedOut,
/// Our peer provided a feerate which violated our required minimum (fetched from our
/// [`FeeEstimator`] either as [`ConfirmationTarget::MinAllowedAnchorChannelRemoteFee`] or
/// [`ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee`]).
///
/// [`FeeEstimator`]: crate::chain::chaininterface::FeeEstimator
/// [`ConfirmationTarget::MinAllowedAnchorChannelRemoteFee`]: crate::chain::chaininterface::ConfirmationTarget::MinAllowedAnchorChannelRemoteFee
/// [`ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee`]: crate::chain::chaininterface::ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee
PeerFeerateTooLow {
/// The feerate on our channel set by our peer.
peer_feerate_sat_per_kw: u32,
/// The required feerate we enforce, from our [`FeeEstimator`].
///
/// [`FeeEstimator`]: crate::chain::chaininterface::FeeEstimator
required_feerate_sat_per_kw: u32,
},
}

impl core::fmt::Display for ClosureReason {
Expand All @@ -355,6 +370,11 @@ impl core::fmt::Display for ClosureReason {
ClosureReason::CounterpartyCoopClosedUnfundedChannel => f.write_str("the peer requested the unfunded channel be closed"),
ClosureReason::FundingBatchClosure => f.write_str("another channel in the same funding batch closed"),
ClosureReason::HTLCsTimedOut => f.write_str("htlcs on the channel timed out"),
ClosureReason::PeerFeerateTooLow { peer_feerate_sat_per_kw, required_feerate_sat_per_kw } =>
f.write_fmt(format_args!(
"peer provided a feerate ({} sat/kw) which was below our lower bound ({} sat/kw)",
peer_feerate_sat_per_kw, required_feerate_sat_per_kw,
)),
}
}
}
Expand All @@ -373,6 +393,10 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
(17, CounterpartyInitiatedCooperativeClosure) => {},
(19, LocallyInitiatedCooperativeClosure) => {},
(21, HTLCsTimedOut) => {},
(23, PeerFeerateTooLow) => {
(0, peer_feerate_sat_per_kw, required),
(2, required_feerate_sat_per_kw, required),
},
);

/// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`].
Expand Down
Loading
Loading