f! remove new ser version + allow downgrade in channel

Author: wpaulino

lightning/src/ln/channel.rs

@@ -36,7 +36,7 @@ use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Channel
 use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::chain::keysinterface::{Sign, KeysInterface, BaseSign};
 use crate::util::events::ClosureReason;
-use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
+use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use crate::util::logger::Logger;
 use crate::util::errors::APIError;
 use crate::util::config::{UserConfig, ChannelConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits};
@@ -5971,7 +5971,7 @@ impl<Signer: Sign> Channel<Signer> {
 	}
 }
 
-const SERIALIZATION_VERSION: u8 = 3;
+const SERIALIZATION_VERSION: u8 = 2;
 const MIN_SERIALIZATION_VERSION: u8 = 2;
 
 impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,;
@@ -6052,6 +6052,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		self.latest_monitor_update_id.write(writer)?;
 
+		let mut key_data = VecWriter(Vec::new());
+		self.holder_signer.write(&mut key_data)?;
+		assert!(key_data.0.len() < core::usize::MAX);
+		assert!(key_data.0.len() < core::u32::MAX as usize);
+		(key_data.0.len() as u32).write(writer)?;
+		writer.write_all(&key_data.0[..])?;
+
 		// Write out the old serialization for shutdown_pubkey for backwards compatibility, if
 		// deserialized from that format.
 		match self.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) {
@@ -6285,7 +6292,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 		let user_id_high_opt = Some((self.user_id >> 64) as u64);
 
 		// `channel_keys_id` is serialized as an option to remain backwards compatible until we bump
-		// `MIN_SERIALIZATION_VERSION` to 3.
+		// `SERIALIZATION_VERSION` to 3.
 		let channel_keys_id = Some(self._channel_keys_id);
 
 		write_tlv_fields!(writer, {
@@ -6303,7 +6310,6 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(5, self.config, required),
 			(6, serialized_holder_htlc_max_in_flight, option),
 			(7, self.shutdown_scriptpubkey, option),
-			(8, channel_keys_id, option),
 			(9, self.target_closing_feerate_sats_per_kw, option),
 			(11, self.monitor_pending_finalized_fulfills, vec_type),
 			(13, self.channel_creation_height, required),
@@ -6313,6 +6319,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			(21, self.outbound_scid_alias, required),
 			(23, channel_ready_event_emitted, option),
 			(25, user_id_high_opt, option),
+			(27, channel_keys_id, option),
 		});
 
 		Ok(())
@@ -6349,18 +6356,16 @@ impl<'a, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<<K::Target as KeysInte
 
 		let latest_monitor_update_id = Readable::read(reader)?;
 
-		let mut holder_signer = None;
-		if ver <= 2 {
-			let keys_len: u32 = Readable::read(reader)?;
-			let mut keys_data = Vec::with_capacity(cmp::min(keys_len as usize, MAX_ALLOC_SIZE));
-			while keys_data.len() != keys_len as usize {
-				// Read 1KB at a time to avoid accidentally allocating 4GB on corrupted channel keys
-				let mut data = [0; 1024];
-				let read_slice = &mut data[0..cmp::min(1024, keys_len as usize - keys_data.len())];
-				reader.read_exact(read_slice)?;
-				keys_data.extend_from_slice(read_slice);
-			}
-			holder_signer = Some(keys_source.read_chan_signer(&keys_data)?);
+		// Read the serialize signer bytes. We'll choose to deserialize them or not based on whether
+		// the `channel_keys_id` TLV is present below.
+		let keys_len: u32 = Readable::read(reader)?;
+		let mut keys_data = Vec::with_capacity(cmp::min(keys_len as usize, MAX_ALLOC_SIZE));
+		while keys_data.len() != keys_len as usize {
+			// Read 1KB at a time to avoid accidentally allocating 4GB on corrupted channel keys
+			let mut data = [0; 1024];
+			let read_slice = &mut data[0..cmp::min(1024, keys_len as usize - keys_data.len())];
+			reader.read_exact(read_slice)?;
+			keys_data.extend_from_slice(read_slice);
 		}
 
 		// Read the old serialization for shutdown_pubkey, preferring the TLV field later if set.
@@ -6590,7 +6595,6 @@ impl<'a, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<<K::Target as KeysInte
 			(5, 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),
-			(8, channel_keys_id, option),
 			(9, target_closing_feerate_sats_per_kw, option),
 			(11, monitor_pending_finalized_fulfills, vec_type),
 			(13, channel_creation_height, option),
@@ -6600,23 +6604,22 @@ impl<'a, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<<K::Target as KeysInte
 			(21, outbound_scid_alias, option),
 			(23, channel_ready_event_emitted, option),
 			(25, user_id_high_opt, option),
+			(27, channel_keys_id, option),
 		});
 
-		let (channel_keys_id, mut holder_signer) = if ver <= 2 {
-			assert!(holder_signer.is_some() && channel_keys_id.is_none());
-			let holder_signer = holder_signer.unwrap();
-			(holder_signer.channel_keys_id(), holder_signer)
+		let (channel_keys_id, holder_signer) = if channel_keys_id.is_some() {
+			let mut holder_signer = keys_source.derive_channel_signer(channel_value_satoshis, channel_keys_id);
+			// If we've gotten to the funding stage of the channel, populate the signer with its
+			// required channel parameters.
+			let non_shutdown_state = channel_state & (!MULTI_STATE_FLAGS);
+			if non_shutdown_state >= (ChannelState::FundingCreated as u32) {
+				holder_signer.ready_channel(&channel_parameters);
+			}
+			(channel_keys_id.unwrap(), holder_signer)
 		} else {
-			assert!(holder_signer.is_none() && channel_keys_id.is_some());
-			let channel_keys_id = channel_keys_id.unwrap();
-			(channel_keys_id, keys_source.derive_channel_signer(channel_value_satoshis, channel_keys_id))
+			let holder_signer = keys_source.read_chan_signer(&keys_data)?;
+			(holder_signer.channel_keys_id(), holder_signer)
 		};
-		// If we've gotten to the funding stage of the channel, populate the signer with its
-		// required channel parameters.
-		let non_shutdown_state = channel_state & (!MULTI_STATE_FLAGS);
-		if non_shutdown_state >= (ChannelState::FundingCreated as u32) {
-			holder_signer.ready_channel(&channel_parameters);
-		}
 
 		if let Some(preimages) = preimages_opt {
 			let mut iter = preimages.into_iter();