From 27fba2dcc02046fd467b1511a4cb59910042a90f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 19 Oct 2023 09:29:21 -0700 Subject: [PATCH] Only account for fee spike buffer multiple on non-anchor channels 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 | 15 ++++++++++++--- lightning/src/ln/channel.rs | 25 ++++++++++++++----------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 767d7b4e1..dddd97cae 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -1223,7 +1223,10 @@ pub fn do_test(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(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(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); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9e95dd727..f3632b41b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1692,9 +1692,13 @@ impl ChannelContext 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 Channel 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. -- 2.39.5