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.

Author: wpaulino

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<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 {

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);

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

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<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.
 ///