Skip to content

Commit

Permalink
Only account for fee spike buffer multiple on non-anchor channels
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wpaulino authored and TheBlueMatt committed Oct 19, 2023
1 parent 8e02036 commit aa1676f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
15 changes: 12 additions & 3 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,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);
}
Expand All @@ -1227,7 +1230,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);
}
Expand All @@ -1236,7 +1242,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);
}
Expand Down
25 changes: 14 additions & 11 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2859,16 +2863,15 @@ impl<SP: Deref> Channel<SP> where
}

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 - msg.amount_msat - self.context.holder_selected_channel_reserve_satoshis * 1000 - 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.
Expand Down

0 comments on commit aa1676f

Please sign in to comment.