From 6007acc50b60a220657859d650ed78711a96f995 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 20 Mar 2025 11:32:28 -0500 Subject: [PATCH 1/3] Move ChannelContext::get_channel_type to FundingScope While channel_type_features won't change during splicing, they may change when migrating existing channels to taproot in the future. --- lightning/src/ln/channel.rs | 126 +++++++++++----------- lightning/src/ln/channel_state.rs | 6 +- lightning/src/ln/channelmanager.rs | 14 +-- lightning/src/ln/dual_funding_tests.rs | 4 +- lightning/src/ln/functional_test_utils.rs | 2 +- 5 files changed, 77 insertions(+), 75 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d6fbd162388..967d0be7bf0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1723,6 +1723,11 @@ impl FundingScope { fn counterparty_funding_pubkey(&self) -> &PublicKey { &self.get_counterparty_pubkeys().funding_pubkey } + + /// Gets the channel's type + pub fn get_channel_type(&self) -> &ChannelTypeFeatures { + &self.channel_transaction_parameters.channel_type_features + } } /// Info about a pending splice, used in the pre-splice channel @@ -3119,11 +3124,6 @@ impl ChannelContext where SP::Target: SignerProvider { self.user_id } - /// Gets the channel's type - pub fn get_channel_type(&self) -> &ChannelTypeFeatures { - &self.channel_type - } - /// Gets the channel's `short_channel_id`. /// /// Will return `None` if the channel hasn't been confirmed yet. @@ -3222,7 +3222,7 @@ impl ChannelContext where SP::Target: SignerProvider { return Err(ChannelError::close("0 max_accepted_htlcs makes for a useless channel".to_owned())); } - let channel_type = &funding.channel_transaction_parameters.channel_type_features; + let channel_type = funding.get_channel_type(); if common_fields.max_accepted_htlcs > max_htlcs(channel_type) { return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", common_fields.max_accepted_htlcs, max_htlcs(channel_type)))); } @@ -3667,10 +3667,10 @@ impl ChannelContext where SP::Target: SignerProvider { ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => { if $outbound == local { // "offered HTLC output" let htlc_in_tx = get_htlc_in_commitment!($htlc, true); - let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_tx_fee = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { 0 } else { - feerate_per_kw as u64 * htlc_timeout_tx_weight(self.get_channel_type()) / 1000 + feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000 }; if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); @@ -3681,10 +3681,10 @@ impl ChannelContext where SP::Target: SignerProvider { } } else { let htlc_in_tx = get_htlc_in_commitment!($htlc, false); - let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_tx_fee = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { 0 } else { - feerate_per_kw as u64 * htlc_success_tx_weight(self.get_channel_type()) / 1000 + feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000 }; if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + htlc_tx_fee { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat); @@ -3796,8 +3796,8 @@ impl ChannelContext where SP::Target: SignerProvider { // commitment transaction *before* checking whether the remote party's balance is enough to // cover the total fee and the anchors. - let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &funding.channel_transaction_parameters.channel_type_features); - let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; + let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), funding.get_channel_type()); + let anchors_val = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; let (value_to_self, value_to_remote) = if funding.is_outbound() { ((value_to_self_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat), value_to_remote_msat / 1000) } else { @@ -3880,16 +3880,19 @@ impl ChannelContext where SP::Target: SignerProvider { } /// Returns a HTLCStats about pending htlcs - fn get_pending_htlc_stats(&self, outbound_feerate_update: Option, dust_exposure_limiting_feerate: u32) -> HTLCStats { + fn get_pending_htlc_stats( + &self, funding: &FundingScope, outbound_feerate_update: Option, + dust_exposure_limiting_feerate: u32, + ) -> HTLCStats { let context = self; - let uses_0_htlc_fee_anchors = self.get_channel_type().supports_anchors_zero_fee_htlc_tx(); + let uses_0_htlc_fee_anchors = funding.get_channel_type().supports_anchors_zero_fee_htlc_tx(); let dust_buffer_feerate = context.get_dust_buffer_feerate(outbound_feerate_update); let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if uses_0_htlc_fee_anchors { (0, 0) } else { - (dust_buffer_feerate as u64 * htlc_timeout_tx_weight(context.get_channel_type()) / 1000, - dust_buffer_feerate as u64 * htlc_success_tx_weight(context.get_channel_type()) / 1000) + (dust_buffer_feerate as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000, + dust_buffer_feerate as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000) }; let mut on_holder_tx_dust_exposure_msat = 0; @@ -3981,7 +3984,7 @@ impl ChannelContext where SP::Target: SignerProvider { } /// Returns information on all pending inbound HTLCs. - pub fn get_pending_inbound_htlc_details(&self) -> Vec { + pub fn get_pending_inbound_htlc_details(&self, funding: &FundingScope) -> Vec { let mut holding_cell_states = new_hash_map(); for holding_cell_update in self.holding_cell_htlc_updates.iter() { match holding_cell_update { @@ -4008,11 +4011,11 @@ impl ChannelContext where SP::Target: SignerProvider { } } let mut inbound_details = Vec::new(); - let htlc_success_dust_limit = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_success_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { 0 } else { let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(self.get_channel_type()) / 1000 + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 }; let holder_dust_limit_success_sat = htlc_success_dust_limit + self.holder_dust_limit_satoshis; for htlc in self.pending_inbound_htlcs.iter() { @@ -4031,13 +4034,13 @@ impl ChannelContext where SP::Target: SignerProvider { } /// Returns information on all pending outbound HTLCs. - pub fn get_pending_outbound_htlc_details(&self) -> Vec { + pub fn get_pending_outbound_htlc_details(&self, funding: &FundingScope) -> Vec { let mut outbound_details = Vec::new(); - let htlc_timeout_dust_limit = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_timeout_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { 0 } else { let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(self.get_channel_type()) / 1000 + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 }; let holder_dust_limit_timeout_sat = htlc_timeout_dust_limit + self.holder_dust_limit_satoshis; for htlc in self.pending_outbound_htlcs.iter() { @@ -4088,7 +4091,7 @@ impl ChannelContext where SP::Target: SignerProvider { // here. let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate); + let htlc_stats = context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); let outbound_capacity_msat = funding.value_to_self_msat .saturating_sub(htlc_stats.pending_outbound_htlcs_value_msat) @@ -4097,7 +4100,7 @@ impl ChannelContext where SP::Target: SignerProvider { let mut available_capacity_msat = outbound_capacity_msat; - let anchor_outputs_value_msat = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -4111,15 +4114,15 @@ impl ChannelContext where SP::Target: SignerProvider { // dependency. // This complicates the computation around dust-values, up to the one-htlc-value. let mut real_dust_limit_timeout_sat = context.holder_dust_limit_satoshis; - if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - real_dust_limit_timeout_sat += context.feerate_per_kw as u64 * htlc_timeout_tx_weight(context.get_channel_type()) / 1000; + if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + real_dust_limit_timeout_sat += context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000; } 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(&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(&funding, htlc_dust, Some(())); - if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + if !funding.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; } @@ -4142,8 +4145,8 @@ impl ChannelContext where SP::Target: SignerProvider { // If the channel is inbound (i.e. counterparty pays the fee), we need to make sure // sending a new HTLC won't reduce their balance below our reserve threshold. let mut real_dust_limit_success_sat = context.counterparty_dust_limit_satoshis; - if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - real_dust_limit_success_sat += context.feerate_per_kw as u64 * htlc_success_tx_weight(context.get_channel_type()) / 1000; + if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + real_dust_limit_success_sat += context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000; } let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered); @@ -4170,12 +4173,12 @@ impl ChannelContext where SP::Target: SignerProvider { let mut dust_exposure_dust_limit_msat = 0; let max_dust_htlc_exposure_msat = context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { (context.counterparty_dust_limit_satoshis, context.holder_dust_limit_satoshis) } else { let dust_buffer_feerate = context.get_dust_buffer_feerate(None) as u64; - (context.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(context.get_channel_type()) / 1000, - context.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(context.get_channel_type()) / 1000) + (context.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000, + context.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) }; if let Some(extra_htlc_dust_exposure) = htlc_stats.extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat { @@ -4245,11 +4248,11 @@ impl ChannelContext where SP::Target: SignerProvider { let context = &self; assert!(funding.is_outbound()); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { (0, 0) } else { - (context.feerate_per_kw as u64 * htlc_success_tx_weight(context.get_channel_type()) / 1000, - context.feerate_per_kw as u64 * htlc_timeout_tx_weight(context.get_channel_type()) / 1000) + (context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000, + context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) }; let real_dust_limit_success_sat = htlc_success_dust_limit + context.holder_dust_limit_satoshis; let real_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.holder_dust_limit_satoshis; @@ -4353,11 +4356,11 @@ impl ChannelContext where SP::Target: SignerProvider { let context = &self; assert!(!funding.is_outbound()); - let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { (0, 0) } else { - (context.feerate_per_kw as u64 * htlc_success_tx_weight(context.get_channel_type()) / 1000, - context.feerate_per_kw as u64 * htlc_timeout_tx_weight(context.get_channel_type()) / 1000) + (context.feerate_per_kw as u64 * htlc_success_tx_weight(funding.get_channel_type()) / 1000, + context.feerate_per_kw as u64 * htlc_timeout_tx_weight(funding.get_channel_type()) / 1000) }; let real_dust_limit_success_sat = htlc_success_dust_limit + context.counterparty_dust_limit_satoshis; let real_dust_limit_timeout_sat = htlc_timeout_dust_limit + context.counterparty_dust_limit_satoshis; @@ -5473,7 +5476,7 @@ impl FundedChannel where } let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate); + let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate); if htlc_stats.pending_inbound_htlcs + 1 > self.context.holder_max_accepted_htlcs as usize { return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.context.holder_max_accepted_htlcs))); } @@ -5517,7 +5520,7 @@ impl FundedChannel where let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); 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.funding.is_outbound() && self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if !self.funding.is_outbound() && self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -5530,7 +5533,7 @@ impl FundedChannel where } } - let anchor_outputs_value_msat = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -6348,11 +6351,12 @@ impl FundedChannel where // Before proposing a feerate update, check that we can actually afford the new fee. let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw), dust_exposure_limiting_feerate); - let commitment_data = self.context.build_commitment_transaction(&self.funding, - self.holder_commitment_point.transaction_number(), - &self.holder_commitment_point.current_point(), true, true, logger); - let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.stats.tx.htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000; + let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); + let commitment_data = self.context.build_commitment_transaction( + &self.funding, self.holder_commitment_point.transaction_number(), + &self.holder_commitment_point.current_point(), true, true, logger, + ); + let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.stats.tx.htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.funding.get_channel_type()) * 1000; let holder_balance_msat = commitment_data.stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat; if holder_balance_msat < buffer_fee_msat + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { //TODO: auto-close after a number of failures? @@ -6643,7 +6647,7 @@ impl FundedChannel where self.context.update_time_counter += 1; // Check that we won't be pushed over our dust exposure limit by the feerate increase. let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate); + let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", @@ -7613,7 +7617,7 @@ impl FundedChannel where } let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate); + let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); let on_counterparty_tx_dust_htlc_exposure_msat = htlc_stats.on_counterparty_tx_dust_exposure_msat; if on_counterparty_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { @@ -7622,11 +7626,11 @@ impl FundedChannel where on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); return Err(("Exceeded our total dust exposure limit on counterparty commitment tx", 0x1000|7)) } - let htlc_success_dust_limit = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_success_dust_limit = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { 0 } else { let dust_buffer_feerate = self.context.get_dust_buffer_feerate(None) as u64; - dust_buffer_feerate * htlc_success_tx_weight(self.context.get_channel_type()) / 1000 + dust_buffer_feerate * htlc_success_tx_weight(self.funding.get_channel_type()) / 1000 }; let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.context.holder_dust_limit_satoshis; if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { @@ -7638,7 +7642,7 @@ impl FundedChannel where } } - let anchor_outputs_value_msat = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -7667,7 +7671,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(&self.funding, None, Some(())); - if !self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + if !self.funding.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(self.funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { @@ -8756,7 +8760,7 @@ impl FundedChannel where && info.next_holder_htlc_id == self.context.next_holder_htlc_id && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id && info.feerate == self.context.feerate_per_kw { - let actual_fee = commit_tx_fee_sat(self.context.feerate_per_kw, counterparty_commitment_tx.htlcs().len(), self.context.get_channel_type()) * 1000; + let actual_fee = commit_tx_fee_sat(self.context.feerate_per_kw, counterparty_commitment_tx.htlcs().len(), self.funding.get_channel_type()) * 1000; assert_eq!(actual_fee, info.fee); } } @@ -11363,13 +11367,13 @@ mod tests { // 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(&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; + let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.funding.get_channel_type()) * 1000; assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs); // Finally, make sure that when Node A calculates the remote's commitment transaction fees, all // of the HTLCs are seen to be above the dust limit. node_a_chan.funding.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 remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.funding.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(&node_a_chan.funding, Some(htlc_candidate), None); assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs); @@ -11392,18 +11396,18 @@ mod tests { let config = UserConfig::default(); let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&fee_est, &&keys_provider, &&keys_provider, node_id, &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42, None, &logger).unwrap(); - let commitment_tx_fee_0_htlcs = commit_tx_fee_sat(chan.context.feerate_per_kw, 0, chan.context.get_channel_type()) * 1000; - let commitment_tx_fee_1_htlc = commit_tx_fee_sat(chan.context.feerate_per_kw, 1, chan.context.get_channel_type()) * 1000; + let commitment_tx_fee_0_htlcs = commit_tx_fee_sat(chan.context.feerate_per_kw, 0, chan.funding.get_channel_type()) * 1000; + let commitment_tx_fee_1_htlc = commit_tx_fee_sat(chan.context.feerate_per_kw, 1, chan.funding.get_channel_type()) * 1000; // If HTLC_SUCCESS_TX_WEIGHT and HTLC_TIMEOUT_TX_WEIGHT were swapped: then this HTLC would be // 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_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.funding.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(&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 dust_htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.funding.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(&chan.funding, htlc_candidate, None); assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs); @@ -11411,13 +11415,13 @@ mod tests { chan.funding.channel_transaction_parameters.is_outbound_from_holder = false; // 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 dust_htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.funding.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(&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_amt_below_success = ((253 * htlc_success_tx_weight(chan.funding.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(&chan.funding, Some(htlc_candidate), None); assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc); diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 2f7d08581ba..2da4eae97f2 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -510,7 +510,7 @@ impl ChannelDetails { // Note that accept_channel (or open_channel) is always the first message, so // `have_received_message` indicates that type negotiation has completed. channel_type: if context.have_received_message() { - Some(context.get_channel_type().clone()) + Some(funding.get_channel_type().clone()) } else { None }, @@ -540,8 +540,8 @@ impl ChannelDetails { inbound_htlc_maximum_msat: context.get_holder_htlc_maximum_msat(funding), config: Some(context.config()), channel_shutdown_state: Some(context.shutdown_state()), - pending_inbound_htlcs: context.get_pending_inbound_htlc_details(), - pending_outbound_htlcs: context.get_pending_outbound_htlc_details(), + pending_inbound_htlcs: context.get_pending_inbound_htlc_details(funding), + pending_outbound_htlcs: context.get_pending_outbound_htlc_details(funding), } } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index fab15bfea28..49a602f97e5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3175,7 +3175,7 @@ macro_rules! emit_channel_pending_event { counterparty_node_id: $channel.context.get_counterparty_node_id(), user_channel_id: $channel.context.get_user_id(), funding_txo: $channel.funding.get_funding_txo().unwrap().into_bitcoin_outpoint(), - channel_type: Some($channel.context.get_channel_type().clone()), + channel_type: Some($channel.funding.get_channel_type().clone()), }, None)); $channel.context.set_channel_pending_event_emitted(); } @@ -3190,7 +3190,7 @@ macro_rules! emit_channel_ready_event { channel_id: $channel.context.channel_id(), user_channel_id: $channel.context.get_user_id(), counterparty_node_id: $channel.context.get_counterparty_node_id(), - channel_type: $channel.context.get_channel_type().clone(), + channel_type: $channel.funding.get_channel_type().clone(), }, None)); $channel.context.set_channel_ready_event_emitted(); } @@ -4337,7 +4337,7 @@ where return Err(("Refusing to forward to a private channel based on our config.", 0x4000 | 10)); } if let HopConnector::ShortChannelId(outgoing_scid) = next_packet.outgoing_connector { - if chan.context.get_channel_type().supports_scid_privacy() && outgoing_scid != chan.context.outbound_scid_alias() { + if chan.funding.get_channel_type().supports_scid_privacy() && outgoing_scid != chan.context.outbound_scid_alias() { // `option_scid_alias` (referred to in LDK as `scid_privacy`) means // "refuse to forward unless the SCID alias was used", so we pretend // we don't have the channel here. @@ -6610,7 +6610,7 @@ where for (chan_id, chan) in peer_state.channel_by_id.iter_mut() .filter_map(|(chan_id, chan)| chan.as_funded_mut().map(|chan| (chan_id, chan))) { - let new_feerate = if chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let new_feerate = if chan.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { anchor_feerate } else { non_anchor_feerate @@ -6667,7 +6667,7 @@ where peer_state.channel_by_id.retain(|chan_id, chan| { match chan.as_funded_mut() { Some(funded_chan) => { - let new_feerate = if funded_chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let new_feerate = if funded_chan.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { anchor_feerate } else { non_anchor_feerate @@ -7962,7 +7962,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if accept_0conf { // This should have been correctly configured by the call to Inbound(V1/V2)Channel::new. debug_assert!(channel.context().minimum_depth().unwrap() == 0); - } else if channel.context().get_channel_type().requires_zero_conf() { + } else if channel.funding().get_channel_type().requires_zero_conf() { let send_msg_err_event = MessageSendEvent::HandleError { node_id: channel.context().get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage{ @@ -11593,7 +11593,7 @@ where 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 channel.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { if let Some(feerate) = min_anchor_feerate { channel.check_for_stale_feerate(&logger, feerate)?; } diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index 9b2247e1745..07d23763c99 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -133,11 +133,9 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) let per_peer_state = nodes[1].node.per_peer_state.read().unwrap(); let peer_state = per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap(); - let channel_context = - peer_state.channel_by_id.get(&tx_complete_msg.channel_id).unwrap().context(); let channel_funding = peer_state.channel_by_id.get(&tx_complete_msg.channel_id).unwrap().funding(); - (channel_funding.get_funding_txo(), channel_context.get_channel_type().clone()) + (channel_funding.get_funding_txo(), channel_funding.get_channel_type().clone()) }; channel.funding.channel_transaction_parameters = ChannelTransactionParameters { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 611fcd2b1ad..7ad18c420f4 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1029,7 +1029,7 @@ macro_rules! get_channel_type_features { let mut per_peer_state_lock; let mut peer_state_lock; let chan = get_channel_ref!($node, $counterparty_node, per_peer_state_lock, peer_state_lock, $channel_id); - chan.context().get_channel_type().clone() + chan.funding().get_channel_type().clone() } } } From b036a948d6e414988b30892d267cb789599205e7 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 20 Mar 2025 17:52:42 -0500 Subject: [PATCH 2/3] Remove redundant ChannelContext::channel_type FundingScope::get_channel_type can be used instead of ChannelContext::channel_type, which is redundant. --- lightning/src/ln/channel.rs | 91 +++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 967d0be7bf0..6aa34965d75 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1970,9 +1970,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// . sent_message_awaiting_response: Option, - /// This channel's type, as negotiated during channel open - channel_type: ChannelTypeFeatures, - // Our counterparty can offer us SCID aliases which they will map to this channel when routing // outbound payments. These can be used in invoice route hints to avoid explicitly revealing // the channel's funding UTXO. @@ -2719,7 +2716,6 @@ impl ChannelContext where SP::Target: SignerProvider { funding_tx_broadcast_safe_event_emitted: false, channel_ready_event_emitted: false, - channel_type, channel_keys_id, local_initiated_shutdown: None, @@ -2956,7 +2952,6 @@ impl ChannelContext where SP::Target: SignerProvider { funding_tx_broadcast_safe_event_emitted: false, channel_ready_event_emitted: false, - channel_type, channel_keys_id, blocked_monitor_updates: Vec::new(), @@ -3183,7 +3178,7 @@ impl ChannelContext where SP::Target: SignerProvider { } if let Some(ty) = &common_fields.channel_type { - if *ty != self.channel_type { + if ty != funding.get_channel_type() { return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned())); } } else if their_features.supports_channel_type() { @@ -3193,7 +3188,6 @@ impl ChannelContext where SP::Target: SignerProvider { if channel_type != ChannelTypeFeatures::only_static_remote_key() { return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned())); } - self.channel_type = channel_type.clone(); funding.channel_transaction_parameters.channel_type_features = channel_type; } @@ -3562,11 +3556,11 @@ impl ChannelContext where SP::Target: SignerProvider { for (idx, (htlc, mut source_opt)) in htlcs_cloned.drain(..).enumerate() { if let Some(_) = htlc.transaction_output_index { let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_data.stats.tx.feerate_per_kw(), - funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, &self.channel_type, + funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, funding.get_channel_type(), &holder_keys.broadcaster_delayed_payment_key, &holder_keys.revocation_key); - let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &self.channel_type, &holder_keys); - let htlc_sighashtype = if self.channel_type.supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All }; + let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, funding.get_channel_type(), &holder_keys); + let htlc_sighashtype = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { EcdsaSighashType::SinglePlusAnyoneCanPay } else { EcdsaSighashType::All }; let htlc_sighash = hash_to_message!(&sighash::SighashCache::new(&htlc_tx).p2wsh_signature_hash(0, &htlc_redeemscript, htlc.to_bitcoin_amount(), htlc_sighashtype).unwrap()[..]); log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.", log_bytes!(msg.htlc_signatures[idx].serialize_compact()[..]), log_bytes!(holder_keys.countersignatory_htlc_key.to_public_key().serialize()), @@ -3964,9 +3958,9 @@ impl ChannelContext where SP::Target: SignerProvider { .checked_sub(dust_exposure_limiting_feerate); let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| { let extra_htlc_dust_exposure = on_counterparty_tx_dust_exposure_msat - + chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, &self.channel_type) * 1000; + + chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()) * 1000; on_counterparty_tx_dust_exposure_msat - += chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, &self.channel_type) * 1000; + += chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, funding.get_channel_type()) * 1000; extra_htlc_dust_exposure }); @@ -4311,12 +4305,12 @@ impl ChannelContext where SP::Target: SignerProvider { } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, &context.channel_type) * 1000; + let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; #[cfg(any(test, fuzzing))] { let mut fee = res; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type) * 1000; + fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len() + context.holding_cell_htlc_updates.len(); @@ -4408,12 +4402,12 @@ impl ChannelContext where SP::Target: SignerProvider { } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, &context.channel_type) * 1000; + let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000; #[cfg(any(test, fuzzing))] if let Some(htlc) = &htlc { let mut fee = res; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type) * 1000; + fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len(); let commitment_tx_info = CommitmentTxInfoCached { @@ -4593,7 +4587,7 @@ impl ChannelContext where SP::Target: SignerProvider { { return Err(()); } - if self.channel_type == ChannelTypeFeatures::only_static_remote_key() { + if funding.get_channel_type() == &ChannelTypeFeatures::only_static_remote_key() { // We've exhausted our options return Err(()); } @@ -4606,16 +4600,16 @@ impl ChannelContext where SP::Target: SignerProvider { // checks whether the counterparty supports every feature, this would only happen if the // counterparty is advertising the feature, but rejecting channels proposing the feature for // whatever reason. - if self.channel_type.supports_anchors_zero_fee_htlc_tx() { - self.channel_type.clear_anchors_zero_fee_htlc_tx(); + let channel_type = &mut funding.channel_transaction_parameters.channel_type_features; + if channel_type.supports_anchors_zero_fee_htlc_tx() { + channel_type.clear_anchors_zero_fee_htlc_tx(); self.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee); - assert!(!funding.channel_transaction_parameters.channel_type_features.supports_anchors_nonzero_fee_htlc_tx()); - } else if self.channel_type.supports_scid_privacy() { - self.channel_type.clear_scid_privacy(); + assert!(!channel_type.supports_anchors_nonzero_fee_htlc_tx()); + } else if channel_type.supports_scid_privacy() { + channel_type.clear_scid_privacy(); } else { - self.channel_type = ChannelTypeFeatures::only_static_remote_key(); + *channel_type = ChannelTypeFeatures::only_static_remote_key(); } - funding.channel_transaction_parameters.channel_type_features = self.channel_type.clone(); Ok(()) } @@ -6641,7 +6635,7 @@ impl FundedChannel where if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { return Err(ChannelError::WarnAndDisconnect("Got fee update message while quiescent".to_owned())); } - FundedChannel::::check_remote_fee(&self.context.channel_type, fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?; + FundedChannel::::check_remote_fee(self.funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?; self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced)); self.context.update_time_counter += 1; @@ -8807,8 +8801,8 @@ impl FundedChannel where debug_assert_eq!(htlc_signatures.len(), trusted_tx.htlcs().len()); for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(trusted_tx.htlcs()) { log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}", - encode::serialize_hex(&chan_utils::build_htlc_transaction(&trusted_tx.txid(), trusted_tx.feerate_per_kw(), self.funding.get_holder_selected_contest_delay(), htlc, &self.context.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)), - encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &counterparty_keys)), + encode::serialize_hex(&chan_utils::build_htlc_transaction(&trusted_tx.txid(), trusted_tx.feerate_per_kw(), self.funding.get_holder_selected_contest_delay(), htlc, self.funding.get_channel_type(), &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)), + encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, self.funding.get_channel_type(), &counterparty_keys)), log_bytes!(counterparty_keys.broadcaster_htlc_key.to_public_key().serialize()), log_bytes!(htlc_sig.serialize_compact()[..]), &self.context.channel_id()); } @@ -9409,7 +9403,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), - channel_type: Some(self.context.channel_type.clone()), + channel_type: Some(self.funding.get_channel_type().clone()), }, push_msat: self.funding.get_value_satoshis() * 1000 - self.funding.value_to_self_msat, channel_reserve_satoshis: self.funding.holder_selected_channel_reserve_satoshis, @@ -9670,7 +9664,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), - channel_type: Some(self.context.channel_type.clone()), + channel_type: Some(self.funding.get_channel_type().clone()), }, channel_reserve_satoshis: self.funding.holder_selected_channel_reserve_satoshis, #[cfg(taproot)] @@ -9907,7 +9901,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), - channel_type: Some(self.context.channel_type.clone()), + channel_type: Some(self.funding.get_channel_type().clone()), }, funding_feerate_sat_per_1000_weight: self.context.feerate_per_kw, second_per_commitment_point, @@ -10077,7 +10071,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), - channel_type: Some(self.context.channel_type.clone()), + channel_type: Some(self.funding.get_channel_type().clone()), }, funding_satoshis: self.dual_funding_context.our_funding_satoshis, second_per_commitment_point, @@ -10450,8 +10444,8 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider // older clients fail to deserialize this channel at all. If the type is // only-static-remote-key, we simply consider it "default" and don't write the channel type // out at all. - let chan_type = if self.context.channel_type != ChannelTypeFeatures::only_static_remote_key() { - Some(&self.context.channel_type) } else { None }; + let chan_type = if self.funding.get_channel_type() != &ChannelTypeFeatures::only_static_remote_key() { + Some(self.funding.get_channel_type()) } else { None }; // The same logic applies for `holder_selected_channel_reserve_satoshis` values other than // the default, and when `holder_max_htlc_value_in_flight_msat` is configured to be set to @@ -10878,7 +10872,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel } } - let chan_features = channel_type.as_ref().unwrap(); + let chan_features = channel_type.unwrap(); if chan_features.supports_any_optional_bits() || chan_features.requires_unknown_bits_from(&our_supported_features) { // If the channel was written by a new version and negotiated with features we don't // understand yet, refuse to read it. @@ -10887,7 +10881,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel // ChannelTransactionParameters may have had an empty features set upon deserialization. // To account for that, we're proactively setting/overriding the field here. - channel_parameters.channel_type_features = chan_features.clone(); + channel_parameters.channel_type_features = chan_features; let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); @@ -11099,7 +11093,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel channel_pending_event_emitted: channel_pending_event_emitted.unwrap_or(true), channel_ready_event_emitted: channel_ready_event_emitted.unwrap_or(true), - channel_type: channel_type.unwrap(), channel_keys_id, local_initiated_shutdown, @@ -12316,8 +12309,8 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 2185; chan.context.holder_dust_limit_satoshis = 2001; - let cached_channel_type = chan.context.channel_type; - chan.context.channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + let cached_channel_type = chan.funding.get_channel_type().clone(); + chan.funding.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); test_commitment_with_anchors!("3044022040f63a16148cf35c8d3d41827f5ae7f7c3746885bb64d4d1b895892a83812b3e02202fcf95c2bf02c466163b3fa3ced6a24926fbb4035095a96842ef516e86ba54c0", "3045022100cd8479cfe1edb1e5a1d487391e0451a469c7171e51e680183f19eb4321f20e9b02204eab7d5a6384b1b08e03baa6e4d9748dfd2b5ab2bae7e39604a0d0055bbffdd5", @@ -12338,7 +12331,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 3702; chan.context.holder_dust_limit_satoshis = 546; - chan.context.channel_type = cached_channel_type.clone(); + chan.funding.channel_transaction_parameters.channel_type_features = cached_channel_type.clone(); test_commitment!("304502210092a587aeb777f869e7ff0d7898ea619ee26a3dacd1f3672b945eea600be431100220077ee9eae3528d15251f2a52b607b189820e57a6ccfac8d1af502b132ee40169", "3045022100e5efb73c32d32da2d79702299b6317de6fb24a60476e3855926d78484dd1b3c802203557cb66a42c944ef06e00bcc4da35a5bcb2f185aab0f8e403e519e1d66aaf75", @@ -12373,7 +12366,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 3687; chan.context.holder_dust_limit_satoshis = 3001; - chan.context.channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + chan.funding.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); test_commitment_with_anchors!("3045022100ad6c71569856b2d7ff42e838b4abe74a713426b37f22fa667a195a4c88908c6902202b37272b02a42dc6d9f4f82cab3eaf84ac882d9ed762859e1e75455c2c228377", "3045022100c970799bcb33f43179eb43b3378a0a61991cf2923f69b36ef12548c3df0e6d500220413dc27d2e39ee583093adfcb7799be680141738babb31cc7b0669a777a31f5d", @@ -12389,7 +12382,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 4914; chan.context.holder_dust_limit_satoshis = 546; - chan.context.channel_type = cached_channel_type.clone(); + chan.funding.channel_transaction_parameters.channel_type_features = cached_channel_type.clone(); test_commitment!("3045022100b4b16d5f8cc9fc4c1aff48831e832a0d8990e133978a66e302c133550954a44d022073573ce127e2200d316f6b612803a5c0c97b8d20e1e44dbe2ac0dd2fb8c95244", "3045022100d72638bc6308b88bb6d45861aae83e5b9ff6e10986546e13bce769c70036e2620220320be7c6d66d22f30b9fcd52af66531505b1310ca3b848c19285b38d8a1a8c19", @@ -12414,7 +12407,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 4894; chan.context.holder_dust_limit_satoshis = 4001; - chan.context.channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + chan.funding.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); test_commitment_with_anchors!("3045022100e784a66b1588575801e237d35e510fd92a81ae3a4a2a1b90c031ad803d07b3f3022021bc5f16501f167607d63b681442da193eb0a76b4b7fd25c2ed4f8b28fd35b95", "30450221009f16ac85d232e4eddb3fcd750a68ebf0b58e3356eaada45d3513ede7e817bf4c02207c2b043b4e5f971261975406cb955219fa56bffe5d834a833694b5abc1ce4cfd", @@ -12424,7 +12417,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 9651180; chan.context.holder_dust_limit_satoshis = 546; - chan.context.channel_type = cached_channel_type.clone(); + chan.funding.channel_transaction_parameters.channel_type_features = cached_channel_type.clone(); test_commitment!("304402200a8544eba1d216f5c5e530597665fa9bec56943c0f66d98fc3d028df52d84f7002201e45fa5c6bc3a506cc2553e7d1c0043a9811313fc39c954692c0d47cfce2bbd3", "3045022100e11b638c05c650c2f63a421d36ef8756c5ce82f2184278643520311cdf50aa200220259565fb9c8e4a87ccaf17f27a3b9ca4f20625754a0920d9c6c239d8156a11de", @@ -12442,7 +12435,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 6216010; chan.context.holder_dust_limit_satoshis = 4001; - chan.context.channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + chan.funding.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); test_commitment_with_anchors!("30450221008fd5dbff02e4b59020d4cd23a3c30d3e287065fda75a0a09b402980adf68ccda022001e0b8b620cd915ddff11f1de32addf23d81d51b90e6841b2cb8dcaf3faa5ecf", "30450221009ad80792e3038fe6968d12ff23e6888a565c3ddd065037f357445f01675d63f3022018384915e5f1f4ae157e15debf4f49b61c8d9d2b073c7d6f97c4a68caa3ed4c1", @@ -12452,7 +12445,7 @@ mod tests { chan.funding.value_to_self_msat = 6993000000; // 7000000000 - 7000000 chan.context.feerate_per_kw = 9651936; chan.context.holder_dust_limit_satoshis = 546; - chan.context.channel_type = cached_channel_type; + chan.funding.channel_transaction_parameters.channel_type_features = cached_channel_type; test_commitment!("304402202ade0142008309eb376736575ad58d03e5b115499709c6db0b46e36ff394b492022037b63d78d66404d6504d4c4ac13be346f3d1802928a6d3ad95a6a944227161a2", "304402207e8d51e0c570a5868a78414f4e0cbfaed1106b171b9581542c30718ee4eb95ba02203af84194c97adf98898c9afe2f2ed4a7f8dba05a2dfab28ac9d9c604aa49a379", @@ -12521,7 +12514,7 @@ mod tests { "020000000001014bdccf28653066a2c554cafeffdfe1e678e64a69b056684deb0c4fba909423ec02000000000000000001e1120000000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e05004730440220471c9f3ad92e49b13b7b8059f43ecf8f7887b0dccbb9fdb54bfe23d62a8ae332022024bd22fae0740e86a44228c35330da9526fd7306dffb2b9dc362d5e78abef7cc0147304402207157f452f2506d73c315192311893800cfb3cc235cc1185b1cfcc136b55230db022014be242dbc6c5da141fec4034e7f387f74d6ff1899453d72ba957467540e1ecb01008576a91414011f7254d96b819c76986c277d115efce6f7b58763ac67210394854aa6eab5b2a8122cc726e9dded053a2184d88256816826d6231c068d4a5b7c820120876475527c21030d417a46946384f88d5f3337267c5e579765875dc4daca813e21734b140639e752ae67a9142002cc93ebefbb1b73f0af055dcc27a0b504ad7688ac6868fa010000" } } ); - chan.context.channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); + chan.funding.channel_transaction_parameters.channel_type_features = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies(); test_commitment_with_anchors!("3044022027b38dfb654c34032ffb70bb43022981652fce923cbbe3cbe7394e2ade8b34230220584195b78da6e25c2e8da6b4308d9db25b65b64975db9266163ef592abb7c725", "3045022100b4014970d9d7962853f3f85196144671d7d5d87426250f0a5fdaf9a55292e92502205360910c9abb397467e19dbd63d081deb4a3240903114c98cec0a23591b79b76", "02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b80074a010000000000002200202b1b5854183c12d3316565972c4668929d314d81c5dcdbb21cb45fe8a9a8114f4a01000000000000220020e9e86e4823faa62e222ebc858a226636856158f07e69898da3b0d1af0ddb3994d007000000000000220020fe0598d74fee2205cc3672e6e6647706b4f3099713b4661b62482c3addd04a5e881300000000000022002018e40f9072c44350f134bdc887bab4d9bdfc8aa468a25616c80e21757ba5dac7881300000000000022002018e40f9072c44350f134bdc887bab4d9bdfc8aa468a25616c80e21757ba5dac7c0c62d0000000000220020f3394e1e619b0eca1f91be2fb5ab4dfc59ba5b84ebe014ad1d43a564d012994aad9c6a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e0400483045022100b4014970d9d7962853f3f85196144671d7d5d87426250f0a5fdaf9a55292e92502205360910c9abb397467e19dbd63d081deb4a3240903114c98cec0a23591b79b7601473044022027b38dfb654c34032ffb70bb43022981652fce923cbbe3cbe7394e2ade8b34230220584195b78da6e25c2e8da6b4308d9db25b65b64975db9266163ef592abb7c72501475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220", { @@ -12638,7 +12631,7 @@ mod tests { &channelmanager::provided_init_features(&UserConfig::default()), 10000000, 100000, 42, &config, 0, 42, None, &logger ).unwrap(); - assert!(!channel_a.context.channel_type.supports_anchors_zero_fee_htlc_tx()); + assert!(!channel_a.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx()); let mut expected_channel_type = ChannelTypeFeatures::empty(); expected_channel_type.set_static_remote_key_required(); @@ -12657,8 +12650,8 @@ mod tests { &open_channel_msg, 7, &config, 0, &&logger, /*is_0conf=*/false ).unwrap(); - assert_eq!(channel_a.context.channel_type, expected_channel_type); - assert_eq!(channel_b.context.channel_type, expected_channel_type); + assert_eq!(channel_a.funding.get_channel_type(), &expected_channel_type); + assert_eq!(channel_b.funding.get_channel_type(), &expected_channel_type); } #[test] From 1bb3b4d62d1c87a7dd3161885578bcc800d9e957 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 31 Mar 2025 18:19:07 -0500 Subject: [PATCH 3/3] Check channel type features are consistent When reading ChannelTransactionParameters, the channel_type_features defaults to only_static_remote_key. This is similarly the case for reading a channel. Instead of overriding the latter with the former, check that they are consistent. It doesn't appear that overriding was ever necessary. --- lightning/src/ln/channel.rs | 8 ++++---- pending_changelog/3678-channel-type-check.txt | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 pending_changelog/3678-channel-type-check.txt diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6aa34965d75..551f69048c4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10747,7 +10747,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel _ => return Err(DecodeError::InvalidValue), }; - let mut channel_parameters: ChannelTransactionParameters = ReadableArgs::::read(reader, channel_value_satoshis)?; + let channel_parameters: ChannelTransactionParameters = ReadableArgs::::read(reader, channel_value_satoshis)?; let funding_transaction: Option = Readable::read(reader)?; let counterparty_cur_commitment_point = Readable::read(reader)?; @@ -10879,9 +10879,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel return Err(DecodeError::UnknownRequiredFeature); } - // ChannelTransactionParameters may have had an empty features set upon deserialization. - // To account for that, we're proactively setting/overriding the field here. - channel_parameters.channel_type_features = chan_features; + if chan_features != channel_parameters.channel_type_features { + return Err(DecodeError::InvalidValue); + } let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); diff --git a/pending_changelog/3678-channel-type-check.txt b/pending_changelog/3678-channel-type-check.txt new file mode 100644 index 00000000000..39efe7cfe71 --- /dev/null +++ b/pending_changelog/3678-channel-type-check.txt @@ -0,0 +1,5 @@ +## API Updates (0.2) + +* Upgrading to v0.2.0 from a version prior to 0.0.116 is not allowed when a channel was opened with + either `scid_privacy` or `zero_conf` included in its channel type. Upgrade to v0.0.116 first + before upgrading to v0.2.0.