From ec7ccf0415d665441d74edbc479fb9ad357c2751 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 9 Jun 2022 14:01:56 -0700 Subject: [PATCH] Introduce LegacyChannelConfig to remain backwards compatible ChannelConfig now has its static fields removed. We introduce a new LegacyChannelConfig struct that maintains the serialization as previously defined by ChannelConfig to remain backwards compatible with clients running 0.0.107 and earlier. --- lightning/src/ln/channel.rs | 41 +++++---- lightning/src/ln/functional_test_utils.rs | 4 +- lightning/src/ln/payment_tests.rs | 3 +- lightning/src/util/config.rs | 106 +++++++++++++++------- 4 files changed, 101 insertions(+), 53 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c9e50000..a168177f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -39,7 +39,7 @@ use util::events::ClosureReason; use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use util::logger::Logger; use util::errors::APIError; -use util::config::{UserConfig, ChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits}; +use util::config::{UserConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits}; use util::scid_utils::scid_from_parts; use io; @@ -491,9 +491,9 @@ pub(crate) const MIN_AFFORDABLE_HTLC_COUNT: usize = 4; // Counterparty designates channel data owned by the another channel participant entity. pub(super) struct Channel { #[cfg(any(test, feature = "_test_utils"))] - pub(crate) config: ChannelConfig, + pub(crate) config: LegacyChannelConfig, #[cfg(not(any(test, feature = "_test_utils")))] - config: ChannelConfig, + config: LegacyChannelConfig, inbound_handshake_limits_override: Option, @@ -930,7 +930,13 @@ impl Channel { Ok(Channel { user_id, - config: config.channel_options.clone(), + + config: LegacyChannelConfig { + mutable: config.channel_options.clone(), + announced_channel: config.own_channel_config.announced_channel, + commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey, + }, + inbound_handshake_limits_override: Some(config.peer_channel_config_limits.clone()), channel_id: keys_provider.get_secure_random_bytes(), @@ -1117,7 +1123,6 @@ impl Channel { delayed_payment_basepoint: msg.delayed_payment_basepoint, htlc_basepoint: msg.htlc_basepoint }; - let mut local_config = (*config).channel_options.clone(); if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT { return Err(ChannelError::Close(format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks. It must be greater than {}", config.own_channel_config.our_to_self_delay, BREAKDOWN_TIMEOUT))); @@ -1186,8 +1191,6 @@ impl Channel { return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours".to_owned())); } } - // we either accept their preference or the preferences match - local_config.announced_channel = announced_channel; let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis); if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { @@ -1254,7 +1257,13 @@ impl Channel { let chan = Channel { user_id, - config: local_config, + + config: LegacyChannelConfig { + mutable: config.channel_options.clone(), + announced_channel, + commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey, + }, + inbound_handshake_limits_override: None, channel_id: msg.temporary_channel_id, @@ -4011,7 +4020,7 @@ impl Channel { // We always add force_close_avoidance_max_fee_satoshis to our normal // feerate-calculated fee, but allow the max to be overridden if we're using a // target feerate-calculated fee. - cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.force_close_avoidance_max_fee_satoshis, + cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.mutable.force_close_avoidance_max_fee_satoshis, proposed_max_feerate as u64 * tx_weight / 1000) } else { self.channel_value_satoshis - (self.value_to_self_msat + 999) / 1000 @@ -4471,15 +4480,15 @@ impl Channel { } pub fn get_fee_proportional_millionths(&self) -> u32 { - self.config.forwarding_fee_proportional_millionths + self.config.mutable.forwarding_fee_proportional_millionths } pub fn get_cltv_expiry_delta(&self) -> u16 { - cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) + cmp::max(self.config.mutable.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) } pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 { - self.config.max_dust_htlc_exposure_msat + self.config.mutable.max_dust_htlc_exposure_msat } pub fn get_feerate(&self) -> u32 { @@ -4566,7 +4575,7 @@ impl Channel { /// Gets the fee we'd want to charge for adding an HTLC output to this Channel /// Allowed in any state (including after shutdown) pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 { - self.config.forwarding_fee_base_msat + self.config.mutable.forwarding_fee_base_msat } /// Returns true if we've ever received a message from the remote end for this Channel @@ -6022,11 +6031,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel let user_id = Readable::read(reader)?; - let mut config = Some(ChannelConfig::default()); + let mut config = Some(LegacyChannelConfig::default()); if ver == 1 { // Read the old serialization of the ChannelConfig from version 0.0.98. - config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?; - config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?; + config.as_mut().unwrap().mutable.forwarding_fee_proportional_millionths = Readable::read(reader)?; + config.as_mut().unwrap().mutable.cltv_expiry_delta = Readable::read(reader)?; config.as_mut().unwrap().announced_channel = Readable::read(reader)?; config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?; } else { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 300efb12..9456d43b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1679,7 +1679,9 @@ pub fn do_claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, ($node: expr, $prev_node: expr, $next_node: expr, $new_msgs: expr) => { { $node.node.handle_update_fulfill_htlc(&$prev_node.node.get_our_node_id(), &next_msgs.as_ref().unwrap().0); - let fee = $node.node.channel_state.lock().unwrap().by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap().config.forwarding_fee_base_msat; + let fee = $node.node.channel_state.lock().unwrap() + .by_id.get(&next_msgs.as_ref().unwrap().0.channel_id).unwrap() + .config.mutable.forwarding_fee_base_msat; expect_payment_forwarded!($node, $next_node, $prev_node, Some(fee as u64), false, false); expected_total_fee_msat += fee as u64; check_added_monitors!($node, 1); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 07e531c5..6fcb18cf 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -531,7 +531,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { // Update the fee on the middle hop to ensure PaymentSent events have the correct (retried) fee // and not the original fee. We also update node[1]'s relevant config as // do_claim_payment_along_route expects us to never overpay. - nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap().config.forwarding_fee_base_msat += 100_000; + nodes[1].node.channel_state.lock().unwrap().by_id.get_mut(&chan_id_2).unwrap() + .config.mutable.forwarding_fee_base_msat += 100_000; new_route.paths[0][0].fee_msat += 100_000; assert!(nodes[0].node.retry_payment(&new_route, payment_id_1).is_err()); // Shouldn't be allowed to retry a fulfilled payment diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index c2c73d8c..570e570a 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -291,30 +291,6 @@ pub struct ChannelConfig { /// /// [`MIN_CLTV_EXPIRY_DELTA`]: crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA pub cltv_expiry_delta: u16, - /// Set to announce the channel publicly and notify all nodes that they can route via this - /// channel. - /// - /// This should only be set to true for nodes which expect to be online reliably. - /// - /// As the node which funds a channel picks this value this will only apply for new outbound - /// channels unless [`ChannelHandshakeLimits::force_announced_channel_preference`] is set. - /// - /// This cannot be changed after the initial channel handshake. - /// - /// Default value: false. - pub announced_channel: bool, - /// When set, we commit to an upfront shutdown_pubkey at channel open. If our counterparty - /// supports it, they will then enforce the mutual-close output to us matches what we provided - /// at intialization, preventing us from closing to an alternate pubkey. - /// - /// This is set to true by default to provide a slight increase in security, though ultimately - /// any attacker who is able to take control of a channel can just as easily send the funds via - /// lightning payments, so we never require that our counterparties support this option. - /// - /// This cannot be changed after a channel has been initialized. - /// - /// Default value: true. - pub commit_upfront_shutdown_pubkey: bool, /// Limit our total exposure to in-flight HTLCs which are burned to fees as they are too /// small to claim on-chain. /// @@ -363,23 +339,83 @@ impl Default for ChannelConfig { forwarding_fee_proportional_millionths: 0, forwarding_fee_base_msat: 1000, cltv_expiry_delta: 6 * 12, // 6 blocks/hour * 12 hours - announced_channel: false, - commit_upfront_shutdown_pubkey: true, max_dust_htlc_exposure_msat: 5_000_000, force_close_avoidance_max_fee_satoshis: 1000, } } } -impl_writeable_tlv_based!(ChannelConfig, { - (0, forwarding_fee_proportional_millionths, required), - (1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)), - (2, cltv_expiry_delta, required), - (3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)), - (4, announced_channel, required), - (6, commit_upfront_shutdown_pubkey, required), - (8, forwarding_fee_base_msat, required), -}); +/// Legacy version of [`ChannelConfig`] that stored the static +/// [`ChannelHandshakeConfig::announced_channel`] and +/// [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] fields. +#[derive(Copy, Clone, Debug)] +pub(crate) struct LegacyChannelConfig { + pub(crate) mutable: ChannelConfig, + /// Deprecated but may still be read from. See [`ChannelHandshakeConfig::announced_channel`] to + /// set this when opening/accepting a channel. + pub(crate) announced_channel: bool, + /// Deprecated but may still be read from. See + /// [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] to set this when + /// opening/accepting a channel. + pub(crate) commit_upfront_shutdown_pubkey: bool, +} + +impl Default for LegacyChannelConfig { + fn default() -> Self { + Self { + mutable: ChannelConfig::default(), + announced_channel: false, + commit_upfront_shutdown_pubkey: true, + } + } +} + +impl ::util::ser::Writeable for LegacyChannelConfig { + fn write(&self, writer: &mut W) -> Result<(), ::io::Error> { + write_tlv_fields!(writer, { + (0, self.mutable.forwarding_fee_proportional_millionths, required), + (1, self.mutable.max_dust_htlc_exposure_msat, (default_value, 5_000_000)), + (2, self.mutable.cltv_expiry_delta, required), + (3, self.mutable.force_close_avoidance_max_fee_satoshis, (default_value, 1000)), + (4, self.announced_channel, required), + (6, self.commit_upfront_shutdown_pubkey, required), + (8, self.mutable.forwarding_fee_base_msat, required), + }); + Ok(()) + } +} + +impl ::util::ser::Readable for LegacyChannelConfig { + fn read(reader: &mut R) -> Result { + let mut forwarding_fee_proportional_millionths = 0; + let mut max_dust_htlc_exposure_msat = 5_000_000; + let mut cltv_expiry_delta = 0; + let mut force_close_avoidance_max_fee_satoshis = 1000; + let mut announced_channel = false; + let mut commit_upfront_shutdown_pubkey = false; + let mut forwarding_fee_base_msat = 0; + read_tlv_fields!(reader, { + (0, forwarding_fee_proportional_millionths, required), + (1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)), + (2, cltv_expiry_delta, required), + (3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)), + (4, announced_channel, required), + (6, commit_upfront_shutdown_pubkey, required), + (8, forwarding_fee_base_msat, required), + }); + Ok(Self { + mutable: ChannelConfig { + forwarding_fee_proportional_millionths, + max_dust_htlc_exposure_msat, + cltv_expiry_delta, + force_close_avoidance_max_fee_satoshis, + forwarding_fee_base_msat, + }, + announced_channel, + commit_upfront_shutdown_pubkey, + }) + } +} /// Top-level config which holds ChannelHandshakeLimits and ChannelConfig. /// -- 2.30.2