From: Duncan Dean Date: Wed, 7 Jun 2023 10:33:41 +0000 (+0200) Subject: Move `Channel::get_feerate_sat_per_1000_weight` and other methods X-Git-Tag: v0.0.116-alpha1~10^2~18 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=3ff94fae55a155603ddcb7c2a96d3b36a0eefa63;p=rust-lightning Move `Channel::get_feerate_sat_per_1000_weight` and other methods This is one of a series of commits to make sure methods are moved by chunks so they are easily reviewable in diffs. Unfortunately they are not purely move-only as fields to be updated for things to compile, but these should be quite clear. This commit also uses the `context` field where needed for compilation and tests to pass due to the above change. f s/tarcontext.get_/target_/ --- diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a57893148..e5452d54f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1341,6 +1341,33 @@ impl ChannelContext { fn counterparty_funding_pubkey(&self) -> &PublicKey { &self.get_counterparty_pubkeys().funding_pubkey } + + pub fn get_feerate_sat_per_1000_weight(&self) -> u32 { + self.feerate_per_kw + } + + pub fn get_dust_buffer_feerate(&self, outbound_feerate_update: Option) -> u32 { + // When calculating our exposure to dust HTLCs, we assume that the channel feerate + // may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, + // whichever is higher. This ensures that we aren't suddenly exposed to significantly + // more dust balance if the feerate increases when we have several HTLCs pending + // which are near the dust limit. + let mut feerate_per_kw = self.feerate_per_kw; + // If there's a pending update fee, use it to ensure we aren't under-estimating + // potential feerate updates coming soon. + if let Some((feerate, _)) = self.pending_update_fee { + feerate_per_kw = cmp::max(feerate_per_kw, feerate); + } + if let Some(feerate) = outbound_feerate_update { + feerate_per_kw = cmp::max(feerate_per_kw, feerate); + } + cmp::max(2530, feerate_per_kw * 1250 / 1000) + } + + /// Get forwarding information for the counterparty. + pub fn counterparty_forwarding_info(&self) -> Option { + self.counterparty_forwarding_info.clone() + } } // Internal utility functions for channels @@ -2920,7 +2947,7 @@ impl Channel { let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if self.context.opt_anchors() { (0, 0) } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(outbound_feerate_update) as u64; + let dust_buffer_feerate = self.context.get_dust_buffer_feerate(outbound_feerate_update) as u64; (dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000, dust_buffer_feerate * htlc_success_tx_weight(false) / 1000) }; @@ -2952,7 +2979,7 @@ impl Channel { let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if self.context.opt_anchors() { (0, 0) } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(outbound_feerate_update) as u64; + let dust_buffer_feerate = self.context.get_dust_buffer_feerate(outbound_feerate_update) as u64; (dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000, dust_buffer_feerate * htlc_success_tx_weight(false) / 1000) }; @@ -3075,7 +3102,7 @@ impl Channel { let (htlc_success_dust_limit, htlc_timeout_dust_limit) = if self.context.opt_anchors() { (self.context.counterparty_dust_limit_satoshis, self.context.holder_dust_limit_satoshis) } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; + let dust_buffer_feerate = self.context.get_dust_buffer_feerate(None) as u64; (self.context.counterparty_dust_limit_satoshis + dust_buffer_feerate * htlc_success_tx_weight(false) / 1000, self.context.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000) }; @@ -3383,7 +3410,7 @@ impl Channel { let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if self.context.opt_anchors() { (0, 0) } else { - let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; + let dust_buffer_feerate = self.context.get_dust_buffer_feerate(None) as u64; (dust_buffer_feerate * htlc_timeout_tx_weight(false) / 1000, dust_buffer_feerate * htlc_success_tx_weight(false) / 1000) }; @@ -4394,7 +4421,7 @@ impl Channel { return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned())); } Channel::::check_remote_fee(fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?; - let feerate_over_dust_buffer = msg.feerate_per_kw > self.get_dust_buffer_feerate(None); + let feerate_over_dust_buffer = msg.feerate_per_kw > self.context.get_dust_buffer_feerate(None); self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced)); self.context.update_time_counter += 1; @@ -5126,28 +5153,6 @@ impl Channel { }) } - pub fn get_feerate_sat_per_1000_weight(&self) -> u32 { - self.context.feerate_per_kw - } - - pub fn get_dust_buffer_feerate(&self, outbound_feerate_update: Option) -> u32 { - // When calculating our exposure to dust HTLCs, we assume that the channel feerate - // may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, - // whichever is higher. This ensures that we aren't suddenly exposed to significantly - // more dust balance if the feerate increases when we have several HTLCs pending - // which are near the dust limit. - let mut feerate_per_kw = self.context.feerate_per_kw; - // If there's a pending update fee, use it to ensure we aren't under-estimating - // potential feerate updates coming soon. - if let Some((feerate, _)) = self.context.pending_update_fee { - feerate_per_kw = cmp::max(feerate_per_kw, feerate); - } - if let Some(feerate) = outbound_feerate_update { - feerate_per_kw = cmp::max(feerate_per_kw, feerate); - } - cmp::max(2530, feerate_per_kw * 1250 / 1000) - } - pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 { self.context.cur_holder_commitment_transaction_number + 1 } @@ -6247,11 +6252,6 @@ impl Channel { } } - /// Get forwarding information for the counterparty. - pub fn counterparty_forwarding_info(&self) -> Option { - self.context.counterparty_forwarding_info.clone() - } - pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> { if msg.contents.htlc_minimum_msat >= self.context.channel_value_satoshis * 1000 { return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string())); @@ -7735,7 +7735,7 @@ mod tests { let mut node_a_chan = Channel::::new_outbound(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42).unwrap(); assert!(node_a_chan.context.counterparty_forwarding_info.is_none()); assert_eq!(node_a_chan.context.holder_htlc_minimum_msat, 1); // the default - assert!(node_a_chan.counterparty_forwarding_info().is_none()); + assert!(node_a_chan.context.counterparty_forwarding_info().is_none()); // Make sure that receiving a channel update will update the Channel as expected. let update = ChannelUpdate { @@ -7758,7 +7758,7 @@ mod tests { // The counterparty can send an update with a higher minimum HTLC, but that shouldn't // change our official htlc_minimum_msat. assert_eq!(node_a_chan.context.holder_htlc_minimum_msat, 1); - match node_a_chan.counterparty_forwarding_info() { + match node_a_chan.context.counterparty_forwarding_info() { Some(info) => { assert_eq!(info.cltv_expiry_delta, 100); assert_eq!(info.fee_base_msat, 110); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index fd536a136..af5159565 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1478,7 +1478,7 @@ impl ChannelDetails { node_id: channel.context.get_counterparty_node_id(), features: latest_features, unspendable_punishment_reserve: to_remote_reserve_satoshis, - forwarding_info: channel.counterparty_forwarding_info(), + forwarding_info: channel.context.counterparty_forwarding_info(), // Ensures that we have actually received the `htlc_minimum_msat` value // from the counterparty through the `OpenChannel` or `AcceptChannel` // message (as they are always the first message from the counterparty). @@ -1496,7 +1496,7 @@ impl ChannelDetails { outbound_scid_alias: if channel.context.is_usable() { Some(channel.context.outbound_scid_alias()) } else { None }, inbound_scid_alias: channel.context.latest_inbound_scid_alias(), channel_value_satoshis: channel.context.get_value_satoshis(), - feerate_sat_per_1000_weight: Some(channel.get_feerate_sat_per_1000_weight()), + feerate_sat_per_1000_weight: Some(channel.context.get_feerate_sat_per_1000_weight()), unspendable_punishment_reserve: to_self_reserve_satoshis, balance_msat: balance.balance_msat, inbound_capacity_msat: balance.inbound_capacity_msat, @@ -3945,18 +3945,18 @@ where fn update_channel_fee(&self, chan_id: &[u8; 32], chan: &mut Channel<::Signer>, new_feerate: u32) -> NotifyOption { if !chan.context.is_outbound() { return NotifyOption::SkipPersist; } // If the feerate has decreased by less than half, don't bother - if new_feerate <= chan.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.get_feerate_sat_per_1000_weight() { + if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() { log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.", - log_bytes!(chan_id[..]), chan.get_feerate_sat_per_1000_weight(), new_feerate); + log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate); return NotifyOption::SkipPersist; } if !chan.context.is_live() { log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {} as it cannot currently be updated (probably the peer is disconnected).", - log_bytes!(chan_id[..]), chan.get_feerate_sat_per_1000_weight(), new_feerate); + log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate); return NotifyOption::SkipPersist; } log_trace!(self.logger, "Channel {} qualifies for a feerate change from {} to {}.", - log_bytes!(chan_id[..]), chan.get_feerate_sat_per_1000_weight(), new_feerate); + log_bytes!(chan_id[..]), chan.context.get_feerate_sat_per_1000_weight(), new_feerate); chan.queue_update_fee(new_feerate, &self.logger); NotifyOption::DoPersist diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 79210068f..a582836a7 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -790,7 +790,7 @@ macro_rules! get_feerate { 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.get_feerate_sat_per_1000_weight() + chan.context.get_feerate_sat_per_1000_weight() } } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 58977e789..d64060be7 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9628,7 +9628,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); let chan = chan_lock.channel_by_id.get(&channel_id).unwrap(); - chan.get_dust_buffer_feerate(None) as u64 + chan.context.get_dust_buffer_feerate(None) as u64 }; let dust_outbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_timeout_tx_weight(opt_anchors) / 1000 + open_channel.dust_limit_satoshis - 1) * 1000; let dust_outbound_htlc_on_holder_tx: u64 = config.channel_config.max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat;