diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a9d15a062a8..f3abb77a849 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1579,6 +1579,15 @@ pub(super) struct FundingScope { #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a remote-generated commitment transaction counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, + + // We save these values so we can make sure `next_local_commit_tx_fee_msat` and + // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will + // be, by comparing the cached values to the fee of the transaction generated by + // `build_commitment_transaction`. + #[cfg(any(test, fuzzing))] + next_local_commitment_tx_fee_info_cached: Mutex>, + #[cfg(any(test, fuzzing))] + next_remote_commitment_tx_fee_info_cached: Mutex>, } impl FundingScope { @@ -1823,15 +1832,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// This can be used to rebroadcast the channel_announcement message later. announcement_sigs: Option<(Signature, Signature)>, - // We save these values so we can make sure `next_local_commit_tx_fee_msat` and - // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will - // be, by comparing the cached values to the fee of the tranaction generated by - // `build_commitment_transaction`. - #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex>, - #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex>, - /// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed /// they will not send a channel_reestablish until the channel locks in. Then, they will send a /// channel_ready *before* sending the channel_reestablish (which is clearly a violation of @@ -2470,6 +2470,11 @@ impl ChannelContext where SP::Target: SignerProvider { holder_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))), #[cfg(debug_assertions)] counterparty_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))), + + #[cfg(any(test, fuzzing))] + next_local_commitment_tx_fee_info_cached: Mutex::new(None), + #[cfg(any(test, fuzzing))] + next_remote_commitment_tx_fee_info_cached: Mutex::new(None), }; let channel_context = ChannelContext { user_id, @@ -2578,11 +2583,6 @@ impl ChannelContext where SP::Target: SignerProvider { announcement_sigs: None, - #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), - workaround_lnd_bug_4006: None, sent_message_awaiting_response: None, @@ -2705,6 +2705,11 @@ impl ChannelContext where SP::Target: SignerProvider { holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), #[cfg(debug_assertions)] counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), + + #[cfg(any(test, fuzzing))] + next_local_commitment_tx_fee_info_cached: Mutex::new(None), + #[cfg(any(test, fuzzing))] + next_remote_commitment_tx_fee_info_cached: Mutex::new(None), }; let channel_context = Self { user_id, @@ -2810,11 +2815,6 @@ impl ChannelContext where SP::Target: SignerProvider { announcement_sigs: None, - #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), - workaround_lnd_bug_4006: None, sent_message_awaiting_response: None, @@ -3868,9 +3868,9 @@ impl ChannelContext where SP::Target: SignerProvider { } let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered); - let mut max_reserved_commit_tx_fee_msat = 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(&funding, htlc_above_dust, Some(())); let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered); - let mut min_reserved_commit_tx_fee_msat = 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(&funding, 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; @@ -3899,7 +3899,7 @@ impl ChannelContext where SP::Target: SignerProvider { } let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered); - let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(Some(htlc_above_dust), None); + let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(&funding, Some(htlc_above_dust), None); let holder_selected_chan_reserve_msat = funding.holder_selected_channel_reserve_satoshis * 1000; let remote_balance_msat = (funding.channel_value_satoshis * 1000 - funding.value_to_self_msat) @@ -3996,7 +3996,9 @@ impl ChannelContext where SP::Target: SignerProvider { /// second allows for creating a buffer to ensure a further HTLC can always be accepted/added. /// /// Dust HTLCs are excluded. - fn next_local_commit_tx_fee_msat(&self, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>) -> u64 { + fn next_local_commit_tx_fee_msat( + &self, _funding: &FundingScope, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>, + ) -> u64 { let context = &self; assert!(context.is_outbound()); @@ -4085,7 +4087,7 @@ impl ChannelContext where SP::Target: SignerProvider { }, feerate: context.feerate_per_kw, }; - *context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); + *_funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); } res } @@ -4100,7 +4102,9 @@ impl ChannelContext where SP::Target: SignerProvider { /// second allows for creating a buffer to ensure a further HTLC can always be accepted/added. /// /// Dust HTLCs are excluded. - fn next_remote_commit_tx_fee_msat(&self, htlc: Option, fee_spike_buffer_htlc: Option<()>) -> u64 { + fn next_remote_commit_tx_fee_msat( + &self, _funding: &FundingScope, htlc: Option, fee_spike_buffer_htlc: Option<()>, + ) -> u64 { debug_assert!(htlc.is_some() || fee_spike_buffer_htlc.is_some(), "At least one of the options must be set"); let context = &self; @@ -4179,7 +4183,7 @@ impl ChannelContext where SP::Target: SignerProvider { }, feerate: context.feerate_per_kw, }; - *context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); + *_funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info); } res } @@ -5199,7 +5203,7 @@ impl FundedChannel where { let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else { let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - self.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations + self.context.next_remote_commit_tx_fee_msat(&self.funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations }; let anchor_outputs_value_msat = if !self.context.is_outbound() && self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 @@ -5222,7 +5226,7 @@ impl FundedChannel where if self.context.is_outbound() { // Check that they won't violate our local required channel reserve by adding this HTLC. let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(htlc_candidate, None); + let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(&self.funding, htlc_candidate, None); if self.funding.value_to_self_msat < self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); } @@ -5400,8 +5404,8 @@ impl FundedChannel where #[cfg(any(test, fuzzing))] { if self.context.is_outbound() { - let projected_commit_tx_info = self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap().take(); - *self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; + let projected_commit_tx_info = self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap().take(); + *self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; if let Some(info) = projected_commit_tx_info { let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len() + self.context.holding_cell_htlc_updates.len(); @@ -5770,8 +5774,8 @@ impl FundedChannel where #[cfg(any(test, fuzzing))] { - *self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; - *self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; + *self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; + *self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; } match &self.context.holder_signer { @@ -7411,7 +7415,7 @@ impl FundedChannel where // // A `None` `HTLCCandidate` is used as in this case because we're already accounting for // the incoming HTLC as it has been fully committed by both sides. - let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(None, Some(())); + let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(&self.funding, None, 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; } @@ -8353,8 +8357,8 @@ impl FundedChannel where #[cfg(any(test, fuzzing))] { if !self.context.is_outbound() { - let projected_commit_tx_info = self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); - *self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; + let projected_commit_tx_info = self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take(); + *self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; if let Some(info) = projected_commit_tx_info { let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len(); if info.total_pending_htlcs == total_pending_htlcs @@ -10375,6 +10379,11 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch holder_max_commitment_tx_output: Mutex::new((0, 0)), #[cfg(debug_assertions)] counterparty_max_commitment_tx_output: Mutex::new((0, 0)), + + #[cfg(any(test, fuzzing))] + next_local_commitment_tx_fee_info_cached: Mutex::new(None), + #[cfg(any(test, fuzzing))] + next_remote_commitment_tx_fee_info_cached: Mutex::new(None), }, context: ChannelContext { user_id, @@ -10470,11 +10479,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch announcement_sigs, - #[cfg(any(test, fuzzing))] - next_local_commitment_tx_fee_info_cached: Mutex::new(None), - #[cfg(any(test, fuzzing))] - next_remote_commitment_tx_fee_info_cached: Mutex::new(None), - workaround_lnd_bug_4006: None, sent_message_awaiting_response: None, @@ -10744,7 +10748,7 @@ mod tests { // Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass // the dust limit check. let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered); - let local_commit_tx_fee = node_a_chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None); + let local_commit_tx_fee = node_a_chan.context.next_local_commit_tx_fee_msat(&node_a_chan.funding, htlc_candidate, None); let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.context.get_channel_type()) * 1000; assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs); @@ -10753,7 +10757,7 @@ mod tests { node_a_chan.context.channel_transaction_parameters.is_outbound_from_holder = false; let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.context.get_channel_type()) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered); - let remote_commit_tx_fee = node_a_chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None); + let remote_commit_tx_fee = node_a_chan.context.next_remote_commit_tx_fee_msat(&node_a_chan.funding, Some(htlc_candidate), None); assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs); } @@ -10781,13 +10785,13 @@ mod tests { // counted as dust when it shouldn't be. let htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amt_above_timeout, HTLCInitiator::LocalOffered); - let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None); + let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None); assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_below_success, HTLCInitiator::RemoteOffered); - let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None); + let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None); assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); chan.context.channel_transaction_parameters.is_outbound_from_holder = false; @@ -10795,13 +10799,13 @@ mod tests { // If swapped: this HTLC would be counted as non-dust when it shouldn't be. let dust_htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis + 1) * 1000; let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_above_timeout, HTLCInitiator::LocalOffered); - let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None); + let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None); assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); // If swapped: this HTLC would be counted as dust when it shouldn't be. let htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis - 1) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amt_below_success, HTLCInitiator::RemoteOffered); - let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None); + let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None); assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); }