Introduce LegacyChannelConfig to remain backwards compatible
authorWilmer Paulino <wilmer.paulino@gmail.com>
Thu, 9 Jun 2022 21:01:56 +0000 (14:01 -0700)
committerWilmer Paulino <wilmer.paulino@gmail.com>
Thu, 9 Jun 2022 23:18:15 +0000 (16:18 -0700)
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
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/payment_tests.rs
lightning/src/util/config.rs

index c9e5000073249deea92b746131270b9ac9a9369e..a168177fd0553d534cb7a894a6cbcc8c5a672a6e 100644 (file)
@@ -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<Signer: Sign> {
        #[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<ChannelHandshakeLimits>,
 
@@ -930,7 +930,13 @@ impl<Signer: Sign> Channel<Signer> {
 
                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<Signer: Sign> Channel<Signer> {
                        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<Signer: Sign> Channel<Signer> {
                                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::<Signer>::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<Signer: Sign> Channel<Signer> {
 
                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<Signer: Sign> Channel<Signer> {
                                // 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<Signer: Sign> Channel<Signer> {
        }
 
        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<Signer: Sign> Channel<Signer> {
        /// 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<Signer>
 
                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 {
index 300efb12bce74eaa11bebd07d5eda591201e87d6..9456d43b4bc0ba3faa02519f5572c69bbc37eb0a 100644 (file)
@@ -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);
index 07e531c5b4b7e330b252414a89207ea70e698bc2..6fcb18cf00bc925a5cb4318c838841ccf339baee 100644 (file)
@@ -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
index c2c73d8cb6c8bffb589a23e7c8e55d8725696922..570e570a893e9583e4bb57ca18d4cb7c515a581c 100644 (file)
@@ -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<W: ::util::ser::Writer>(&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<R: ::io::Read>(reader: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
+               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.
 ///