Only account for fee spike buffer multiple on non-anchor channels 2023-10-2674-fuzz-test
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Thu, 19 Oct 2023 16:29:21 +0000 (09:29 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Fri, 20 Oct 2023 18:04:42 +0000 (11:04 -0700)
Anchor outputs channels are no longer susceptible to fee spikes as they
now mostly target the dynamic minimum mempool fee and can contribute the
remainder of fees when closing.

fuzz/src/chanmon_consistency.rs
lightning/src/ln/channel.rs

index 767d7b4e1a7f58321798849fdde28bf46b9833d7..dddd97cae4554c8f22a09af98bf5d1098ea596fa 100644 (file)
@@ -1223,7 +1223,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
                        0x6d => { send_hop_payment(&nodes[2], &nodes[1], chan_b, &nodes[0], chan_a, 1, &mut payment_id, &mut payment_idx); },
 
                        0x80 => {
-                               let max_feerate = last_htlc_clear_fee_a * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_a;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_a.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_a.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
@@ -1232,7 +1235,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
                        0x81 => { fee_est_a.ret_val.store(253, atomic::Ordering::Release); nodes[0].maybe_update_chan_fees(); },
 
                        0x84 => {
-                               let max_feerate = last_htlc_clear_fee_b * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_b;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_b.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_b.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
@@ -1241,7 +1247,10 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
                        0x85 => { fee_est_b.ret_val.store(253, atomic::Ordering::Release); nodes[1].maybe_update_chan_fees(); },
 
                        0x88 => {
-                               let max_feerate = last_htlc_clear_fee_c * FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               let mut max_feerate = last_htlc_clear_fee_c;
+                               if !anchors {
+                                       max_feerate *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE as u32;
+                               }
                                if fee_est_c.ret_val.fetch_add(250, atomic::Ordering::AcqRel) + 250 > max_feerate {
                                        fee_est_c.ret_val.store(max_feerate, atomic::Ordering::Release);
                                }
index 9e95dd727a6bbbb7dc43de6593cd2ffe92be59f5..f3632b41b471098392bf357e3a9e7220995588ce 100644 (file)
@@ -1692,9 +1692,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        }
 
                        let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered);
-                       let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
+                       let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
                        let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered);
-                       let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+                       let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+                       if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               max_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                               min_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                       }
 
                        // We will first subtract the fee as if we were above-dust. Then, if the resulting
                        // value ends up being below dust, we have this fee available again. In that case,
@@ -2856,16 +2860,15 @@ impl<SP: Deref> Channel<SP> where
                        0
                };
                if !self.context.is_outbound() {
-                       // `2 *` and `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
-                       // the spec because in the spec, the fee spike buffer requirement doesn't exist on the
-                       // receiver's side, only on the sender's.
-                       // Note that when we eventually remove support for fee updates and switch to anchor output
-                       // fees, we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep
-                       // the extra htlc when calculating the next remote commitment transaction fee as we should
-                       // still be able to afford adding this HTLC plus one more future HTLC, regardless of being
-                       // sensitive to fee spikes.
+                       // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from
+                       // the spec because the fee spike buffer requirement doesn't exist on the receiver's
+                       // side, only on the sender's. Note that with anchor outputs we are no longer as
+                       // sensitive to fee spikes, so we need to account for them.
                        let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
-                       let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
+                       let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
+                       if !self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
+                               remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
+                       }
                        if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(self.context.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat {
                                // Note that if the pending_forward_status is not updated here, then it's because we're already failing
                                // the HTLC, i.e. its status is already set to failing.