Introduce LegacyChannelConfig to remain backwards compatible

ChannelConfig now has its static fields removed with a new TLV structure
defined. 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, ChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits, LegacyChannelConfig};
 use util::scid_utils::scid_from_parts;
 
 use io;
@@ -490,6 +490,10 @@ pub(crate) const MIN_AFFORDABLE_HTLC_COUNT: usize = 4;
 // Holder designates channel data owned for the benefice of the user client.
 // Counterparty designates channel data owned by the another channel participant entity.
 pub(super) struct Channel<Signer: Sign> {
+	// This only here for backwards-compatibility in serialization, in the future it can be removed,
+	// breaking clients running 0.0.107 and earlier.
+	_legacy_config: LegacyChannelConfig,
+
 	#[cfg(any(test, feature = "_test_utils"))]
 	pub(crate) config: ChannelConfig,
 	#[cfg(not(any(test, feature = "_test_utils")))]
@@ -929,17 +933,24 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		}
 
-		// Also update `ChannelConfig::announced_channel` to make sure we still allow users to
-		// downgrade versions after the fact.
+		// Also update `LegacyChannelConfig` to make sure we still allow users to downgrade versions
+		// after the fact.
 		//
-		// TODO: This can be removed once we prevent downgrades to v0.0.107.
-		let mut channel_options = config.channel_options.clone();
-		channel_options.announced_channel = config.own_channel_config.announced_channel;
+		// TODO: This can be removed once we break compatibility with versions 0.0.107 and earlier.
+		let legacy_config = LegacyChannelConfig {
+			config: config.channel_options.clone(),
+			announced_channel: config.own_channel_config.announced_channel,
+			commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey,
+		};
 
 		Ok(Channel {
 			user_id,
-			config: channel_options,
+
+			_legacy_config: legacy_config,
+			config: config.channel_options.clone(),
+
 			announced: config.own_channel_config.announced_channel,
+
 			inbound_handshake_limits_override: Some(config.peer_channel_config_limits.clone()),
 
 			channel_id: keys_provider.get_secure_random_bytes(),
@@ -1195,12 +1206,15 @@ impl<Signer: Sign> Channel<Signer> {
 				return Err(ChannelError::Close("Peer tried to open channel but their announcement preference is different from ours".to_owned()));
 			}
 		}
-		// Also update `ChannelConfig::announced_channel` to make sure we still allow users to
-		// downgrade versions after the fact.
+		// Also update `LegacyChannelConfig` to make sure we still allow users to downgrade versions
+		// after the fact.
 		//
-		// TODO: This can be removed once we prevent downgrades to v0.0.107.
-		let mut channel_options = config.channel_options.clone();
-		channel_options.announced_channel = announced_channel;
+		// TODO: This can be removed once we break compatibility with versions 0.0.107 and earlier.
+		let legacy_config = LegacyChannelConfig {
+			config: config.channel_options.clone(),
+			announced_channel,
+			commit_upfront_shutdown_pubkey: config.own_channel_config.commit_upfront_shutdown_pubkey,
+		};
 
 		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 {
@@ -1267,8 +1281,12 @@ impl<Signer: Sign> Channel<Signer> {
 
 		let chan = Channel {
 			user_id,
-			config: channel_options,
+
+			_legacy_config: legacy_config,
+			config: config.channel_options.clone(),
+
 			announced: announced_channel,
+
 			inbound_handshake_limits_override: None,
 
 			channel_id: msg.temporary_channel_id,
@@ -6011,7 +6029,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(2, chan_type, option),
 			(3, self.counterparty_selected_channel_reserve_satoshis, option),
 			(4, serialized_holder_selected_reserve, option),
-			(5, self.config, required),
+			(5, self._legacy_config, required),
 			(6, serialized_holder_htlc_max_in_flight, option),
 			(7, self.shutdown_scriptpubkey, option),
 			(9, self.target_closing_feerate_sats_per_kw, option),
@@ -6022,6 +6040,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(19, self.latest_inbound_scid_alias, option),
 			(21, self.outbound_scid_alias, required),
 			(23, self.announced, required),
+			(25, self.config, required),
 		});
 
 		Ok(())
@@ -6037,13 +6056,13 @@ 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 legacy_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().announced_channel = Readable::read(reader)?;
-			config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
+			legacy_config.as_mut().unwrap().config.forwarding_fee_proportional_millionths = Readable::read(reader)?;
+			legacy_config.as_mut().unwrap().config.cltv_expiry_delta = Readable::read(reader)?;
+			legacy_config.as_mut().unwrap().announced_channel = Readable::read(reader)?;
+			legacy_config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
 		} else {
 			// Read the 8 bytes of backwards-compatibility ChannelConfig data.
 			let mut _val: u64 = Readable::read(reader)?;
@@ -6280,14 +6299,15 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		let mut latest_inbound_scid_alias = None;
 		let mut outbound_scid_alias = None;
 		let mut announced = None;
+		let mut config = None;
 
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
 			(2, channel_type, option),
 			(3, counterparty_selected_channel_reserve_satoshis, option),
 			(4, holder_selected_channel_reserve_satoshis, option),
-			(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
+			(5, legacy_config, option), // Note that if none is provided we will *not* overwrite the existing one.
 			(6, holder_max_htlc_value_in_flight_msat, option),
 			(7, shutdown_scriptpubkey, option),
 			(9, target_closing_feerate_sats_per_kw, option),
@@ -6298,6 +6318,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 			(19, latest_inbound_scid_alias, option),
 			(21, outbound_scid_alias, option),
 			(23, announced, option),
+			(25, config, option),
 		});
 
 		if let Some(preimages) = preimages_opt {
@@ -6337,8 +6358,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 		Ok(Channel {
 			user_id,
 
-			config: config.unwrap(),
-			announced: announced.unwrap_or(config.unwrap().announced_channel),
+			_legacy_config: legacy_config.unwrap(),
+			config: config.unwrap_or(legacy_config.unwrap().config),
+
+			announced: announced.unwrap_or(legacy_config.unwrap().announced_channel),
 
 			// Note that we don't care about serializing handshake limits as we only ever serialize
 			// channel data after the handshake has completed.

lightning/src/util/config.rs

@@ -291,10 +291,6 @@ pub struct ChannelConfig {
 	///
 	/// [`MIN_CLTV_EXPIRY_DELTA`]: crate::ln::channelmanager::MIN_CLTV_EXPIRY_DELTA
 	pub cltv_expiry_delta: u16,
-	/// Deprecated, use [`ChannelHandshakeConfig::announced_channel`] instead.
-	pub(crate) announced_channel: bool,
-	/// Deprecated, use [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] instead.
-	pub(crate) 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.
 	///
@@ -343,8 +339,6 @@ 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,
 		}
@@ -353,14 +347,79 @@ impl Default for ChannelConfig {
 
 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),
+	(2, forwarding_fee_base_msat, required),
+	(4, cltv_expiry_delta, required),
+	(6, max_dust_htlc_exposure_msat, required),
+	(8, force_close_avoidance_max_fee_satoshis, required),
 });
 
+/// Legacy version of [`ChannelConfig`] that stored the static [`announced_channel`] and
+/// [`commit_upfront_shutdown_pubkey`] fields.
+#[derive(Copy, Clone, Debug)]
+pub(crate) struct LegacyChannelConfig {
+	pub(crate) config: ChannelConfig,
+	pub(crate) announced_channel: bool,
+	pub(crate) commit_upfront_shutdown_pubkey: bool,
+}
+
+impl Default for LegacyChannelConfig {
+	fn default() -> Self {
+		Self {
+			config: ChannelConfig::default(),
+			announced_channel: false,
+			commit_upfront_shutdown_pubkey: true,
+		}
+	}
+}
+
+// ChannelConfig redefined its TLV fields, so LegacyChannelConfig should remain consistent with what
+// ChannelConfig was previously.
+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.config.forwarding_fee_proportional_millionths, required),
+			(1, self.config.max_dust_htlc_exposure_msat, (default_value, 5_000_000)),
+			(2, self.config.cltv_expiry_delta, required),
+			(3, self.config.force_close_avoidance_max_fee_satoshis, (default_value, 1000)),
+			(4, self.announced_channel, required),
+			(6, self.commit_upfront_shutdown_pubkey, required),
+			(8, self.config.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 {
+			config: 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.
 ///
 /// Default::default() provides sane defaults for most configurations