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

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index 4b2cf17a2a989d72cbdaaba18305ee37afea1b0d..4591888e3f0aca100cd6dce88766c65514b01b28 100644 (file)
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -921,19 +921,32 @@ mod tests {
                ext_from_hex("0c005e", &mut test);
                // the funding transaction
                ext_from_hex("020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0150c3000000000000220020ae0000000000000000000000000000000000000000000000000000000000000000000000", &mut test);
                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);
                // 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("0c0000", &mut test);

+               ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection

                ext_from_hex("0c0000", &mut test);
                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("0c0000", &mut test);

+               ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection

                ext_from_hex("0c0000", &mut test);
                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("0c0000", &mut test);

+               ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection

                ext_from_hex("0c0000", &mut test);
                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("0c0000", &mut test);

+               ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection

                ext_from_hex("0c0000", &mut test);
                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("0c0000", &mut test);

+               ext_from_hex("00fd00fd", &mut test); // Two feerate requests during block connection

                ext_from_hex("0c0000", &mut test);
                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("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
                // 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


@@ -1303,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("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);
                //
                // 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);
                // 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);
                // 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);
                // 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);
                // 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);
                // 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);
 
                // process the now-pending HTLC forward
                ext_from_hex("07", &mut test);

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 488141fa8fa300fc65812209036aa99e591a7753..75d1c0949e4fbd846e98a6641beddb663866ceae 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5117,6 +5117,26 @@ impl<SP: Deref> Channel<SP> where
                }
        }
 
                }
        }
 

+       pub fn check_for_stale_feerate<L: Logger>(&mut self, logger: &L, min_feerate: u32) -> Result<(), ClosureReason> {
+               if self.context.is_outbound() {
+                       // While its possible our fee is too low for an outbound channel because we've been
+                       // unable to increase the fee, we don't try to force-close directly here.
+                       return Ok(());
+               }
+               if self.context.feerate_per_kw < min_feerate {
+                       log_info!(logger,
+                               "Closing channel as feerate of {} is below required {} (the minimum required rate over the past day)",
+                               self.context.feerate_per_kw, min_feerate
+                       );
+                       Err(ClosureReason::PeerFeerateTooLow {
+                               peer_feerate_sat_per_kw: self.context.feerate_per_kw,
+                               required_feerate_sat_per_kw: min_feerate,
+                       })
+               } else {
+                       Ok(())
+               }
+       }
+

        pub fn update_fee<F: Deref, L: Deref>(&mut self, fee_estimator: &LowerBoundedFeeEstimator<F>, msg: &msgs::UpdateFee, logger: &L) -> Result<(), ChannelError>
                where F::Target: FeeEstimator, L::Target: Logger
        {
        pub fn update_fee<F: Deref, L: Deref>(&mut self, fee_estimator: &LowerBoundedFeeEstimator<F>, msg: &msgs::UpdateFee, logger: &L) -> Result<(), ChannelError>
                where F::Target: FeeEstimator, L::Target: Logger
        {

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index a2647c80cacb8bd01fc053ce6dfc0e061ccd7352..ef6089d3c5094173da88c7ec7c7b8600d253ff8a 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -962,6 +962,11 @@ pub(super) struct InboundChannelRequest {
 /// accepted. An unaccepted channel that exceeds this limit will be abandoned.
 const UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS: i32 = 2;
 
 /// accepted. An unaccepted channel that exceeds this limit will be abandoned.
 const UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS: i32 = 2;
 

+/// The number of blocks of historical feerate estimates we keep around and consider when deciding
+/// to force-close a channel for having too-low fees. Also the number of blocks we have to see
+/// after startup before we consider force-closing channels for having too-low fees.
+const FEERATE_TRACKING_BLOCKS: usize = 144;
+

 /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is
 /// actually ours and not some duplicate HTLC sent to us by a node along the route.
 ///
 /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is
 /// actually ours and not some duplicate HTLC sent to us by a node along the route.
 ///


@@ -2098,6 +2103,21 @@ where
        /// Tracks the message events that are to be broadcasted when we are connected to some peer.
        pending_broadcast_messages: Mutex<Vec<MessageSendEvent>>,
 
        /// Tracks the message events that are to be broadcasted when we are connected to some peer.
        pending_broadcast_messages: Mutex<Vec<MessageSendEvent>>,
 

+       /// We only want to force-close our channels on peers based on stale feerates when we're
+       /// confident the feerate on the channel is *really* stale, not just became stale recently.
+       /// Thus, we store the fee estimates we had as of the last [`FEERATE_TRACKING_BLOCKS`] blocks
+       /// (after startup completed) here, and only force-close when channels have a lower feerate
+       /// than we predicted any time in the last [`FEERATE_TRACKING_BLOCKS`] blocks.
+       ///
+       /// We only keep this in memory as we assume any feerates we receive immediately after startup
+       /// may be bunk (as they often are if Bitcoin Core crashes) and want to delay taking any
+       /// actions for a day anyway.
+       ///
+       /// The first element in the pair is the
+       /// [`ConfirmationTarget::MinAllowedAnchorChannelRemoteFee`] estimate, the second the
+       /// [`ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee`] estimate.
+       last_days_feerates: Mutex<VecDeque<(u32, u32)>>,
+

        entropy_source: ES,
        node_signer: NS,
        signer_provider: SP,
        entropy_source: ES,
        node_signer: NS,
        signer_provider: SP,


@@ -2875,6 +2895,8 @@ where
                        pending_offers_messages: Mutex::new(Vec::new()),
                        pending_broadcast_messages: Mutex::new(Vec::new()),
 
                        pending_offers_messages: Mutex::new(Vec::new()),
                        pending_broadcast_messages: Mutex::new(Vec::new()),
 

+                       last_days_feerates: Mutex::new(VecDeque::new()),
+

                        entropy_source,
                        node_signer,
                        signer_provider,
                        entropy_source,
                        node_signer,
                        signer_provider,


@@ -9173,7 +9195,38 @@ where
                                self, || -> NotifyOption { NotifyOption::DoPersist });
                *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
 
                                self, || -> NotifyOption { NotifyOption::DoPersist });
                *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
 

-               self.do_chain_event(Some(height), |channel| channel.best_block_updated(height, header.time, self.chain_hash, &self.node_signer, &self.default_configuration, &&WithChannelContext::from(&self.logger, &channel.context, None)));
+               let mut min_anchor_feerate = None;
+               let mut min_non_anchor_feerate = None;
+               if self.background_events_processed_since_startup.load(Ordering::Relaxed) {
+                       // If we're past the startup phase, update our feerate cache
+                       let mut last_days_feerates = self.last_days_feerates.lock().unwrap();
+                       if last_days_feerates.len() >= FEERATE_TRACKING_BLOCKS {
+                               last_days_feerates.pop_front();
+                       }
+                       let anchor_feerate = self.fee_estimator
+                               .bounded_sat_per_1000_weight(ConfirmationTarget::MinAllowedAnchorChannelRemoteFee);
+                       let non_anchor_feerate = self.fee_estimator
+                               .bounded_sat_per_1000_weight(ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee);
+                       last_days_feerates.push_back((anchor_feerate, non_anchor_feerate));
+                       if last_days_feerates.len() >= FEERATE_TRACKING_BLOCKS {
+                               min_anchor_feerate = last_days_feerates.iter().map(|(f, _)| f).min().copied();
+                               min_non_anchor_feerate = last_days_feerates.iter().map(|(_, f)| f).min().copied();
+                       }
+               }
+
+               self.do_chain_event(Some(height), |channel| {
+                       let logger = WithChannelContext::from(&self.logger, &channel.context, None);
+                       if channel.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               if let Some(feerate) = min_anchor_feerate {
+                                       channel.check_for_stale_feerate(&logger, feerate)?;
+                               }
+                       } else {
+                               if let Some(feerate) = min_non_anchor_feerate {
+                                       channel.check_for_stale_feerate(&logger, feerate)?;
+                               }
+                       }
+                       channel.best_block_updated(height, header.time, self.chain_hash, &self.node_signer, &self.default_configuration, &&WithChannelContext::from(&self.logger, &channel.context, None))
+               });

 
                macro_rules! max_time {
                        ($timestamp: expr) => {
 
                macro_rules! max_time {
                        ($timestamp: expr) => {


@@ -11992,6 +12045,8 @@ where
                        node_signer: args.node_signer,
                        signer_provider: args.signer_provider,
 
                        node_signer: args.node_signer,
                        signer_provider: args.signer_provider,
 

+                       last_days_feerates: Mutex::new(VecDeque::new()),
+

                        logger: args.logger,
                        default_configuration: args.default_config,
                };
                        logger: args.logger,
                        default_configuration: args.default_config,
                };