Merge pull request #3632 from jkczyz/2025-02-reject-old-channel-versions

Reject channels serialized with version <= 2

Author: wpaulino

lightning/src/ln/channel.rs

@@ -10296,28 +10296,25 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 	}
 }
 
-impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)> for FundedChannel<SP>
+impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c ChannelTypeFeatures)> for FundedChannel<SP>
 		where
 			ES::Target: EntropySource,
 			SP::Target: SignerProvider
 {
-	fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)) -> Result<Self, DecodeError> {
-		let (entropy_source, signer_provider, serialized_height, our_supported_features) = args;
+	fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, &'c ChannelTypeFeatures)) -> Result<Self, DecodeError> {
+		let (entropy_source, signer_provider, our_supported_features) = args;
 		let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
+		if ver <= 2 {
+			return Err(DecodeError::UnknownVersion);
+		}
 
 		// `user_id` used to be a single u64 value. In order to remain backwards compatible with
 		// versions prior to 0.0.113, the u128 is serialized as two separate u64 values. We read
 		// the low bytes now and the high bytes later.
 		let user_id_low: u64 = Readable::read(reader)?;
 
-		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().options.forwarding_fee_proportional_millionths = Readable::read(reader)?;
-			config.as_mut().unwrap().options.cltv_expiry_delta = Readable::read(reader)?;
-			config.as_mut().unwrap().announce_for_forwarding = Readable::read(reader)?;
-			config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
-		} else {
+		let mut config = LegacyChannelConfig::default();
+		{
 			// Read the 8 bytes of backwards-compatibility ChannelConfig data.
 			let mut _val: u64 = Readable::read(reader)?;
 		}
@@ -10481,23 +10478,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 		let holder_dust_limit_satoshis = Readable::read(reader)?;
 		let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?;
 		let mut counterparty_selected_channel_reserve_satoshis = None;
-		if ver == 1 {
-			// Read the old serialization from version 0.0.98.
-			counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?);
-		} else {
-			// Read the 8 bytes of backwards-compatibility data.
+		{
+			// Read the 8 bytes of backwards-compatibility counterparty_selected_channel_reserve_satoshis data.
 			let _dummy: u64 = Readable::read(reader)?;
 		}
 		let counterparty_htlc_minimum_msat = Readable::read(reader)?;
 		let holder_htlc_minimum_msat = Readable::read(reader)?;
 		let counterparty_max_accepted_htlcs = Readable::read(reader)?;
 
 		let mut minimum_depth = None;
-		if ver == 1 {
-			// Read the old serialization from version 0.0.98.
-			minimum_depth = Some(Readable::read(reader)?);
-		} else {
-			// Read the 4 bytes of backwards-compatibility data.
+		{
+			// Read the 4 bytes of backwards-compatibility minimum_depth data.
 			let _dummy: u32 = Readable::read(reader)?;
 		}
 
@@ -10542,20 +10533,20 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 		// Prior to supporting channel type negotiation, all of our channels were static_remotekey
 		// only, so we default to that if none was written.
 		let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
-		let mut channel_creation_height = Some(serialized_height);
+		let mut channel_creation_height = 0u32;
 		let mut preimages_opt: Option<Vec<Option<PaymentPreimage>>> = None;
 
 		// If we read an old Channel, for simplicity we just treat it as "we never sent an
 		// AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
-		let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
+		let mut announcement_sigs_state = AnnouncementSigsState::NotSent;
 		let mut latest_inbound_scid_alias = None;
-		let mut outbound_scid_alias = None;
+		let mut outbound_scid_alias = 0u64;
 		let mut channel_pending_event_emitted = None;
 		let mut channel_ready_event_emitted = None;
 		let mut funding_tx_broadcast_safe_event_emitted = None;
 
 		let mut user_id_high_opt: Option<u64> = None;
-		let mut channel_keys_id: Option<[u8; 32]> = None;
+		let mut channel_keys_id = [0u8; 32];
 		let mut temporary_channel_id: Option<ChannelId> = None;
 		let mut holder_max_accepted_htlcs: Option<u16> = None;
 
@@ -10584,21 +10575,21 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 			(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, config, required),
 			(6, holder_max_htlc_value_in_flight_msat, option),
 			(7, shutdown_scriptpubkey, option),
 			(8, blocked_monitor_updates, optional_vec),
 			(9, target_closing_feerate_sats_per_kw, option),
 			(10, monitor_pending_update_adds, option), // Added in 0.0.122
 			(11, monitor_pending_finalized_fulfills, optional_vec),
-			(13, channel_creation_height, option),
+			(13, channel_creation_height, required),
 			(15, preimages_opt, optional_vec),
-			(17, announcement_sigs_state, option),
+			(17, announcement_sigs_state, required),
 			(19, latest_inbound_scid_alias, option),
-			(21, outbound_scid_alias, option),
+			(21, outbound_scid_alias, required),
 			(23, channel_ready_event_emitted, option),
 			(25, user_id_high_opt, option),
-			(27, channel_keys_id, option),
+			(27, channel_keys_id, required),
 			(28, holder_max_accepted_htlcs, option),
 			(29, temporary_channel_id, option),
 			(31, channel_pending_event_emitted, option),
@@ -10615,12 +10606,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 			(53, funding_tx_broadcast_safe_event_emitted, option),
 		});
 
-		let (channel_keys_id, holder_signer) = if let Some(channel_keys_id) = channel_keys_id {
-			let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
-			(channel_keys_id, holder_signer)
-		} else {
-			return Err(DecodeError::InvalidValue);
-		};
+		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
 
 		if let Some(preimages) = preimages_opt {
 			let mut iter = preimages.into_iter();
@@ -10651,8 +10637,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 		// ChannelTransactionParameters may have had an empty features set upon deserialization.
 		// To account for that, we're proactively setting/overriding the field here.
 		channel_parameters.channel_type_features = chan_features.clone();
-		// ChannelTransactionParameters::channel_value_satoshis defaults to 0 prior to version 0.2.
-		channel_parameters.channel_value_satoshis = channel_value_satoshis;
 
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
@@ -10764,7 +10748,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 			context: ChannelContext {
 				user_id,
 
-				config: config.unwrap(),
+				config,
 
 				prev_config: None,
 
@@ -10775,7 +10759,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				channel_id,
 				temporary_channel_id,
 				channel_state,
-				announcement_sigs_state: announcement_sigs_state.unwrap(),
+				announcement_sigs_state,
 				secp_ctx,
 
 				latest_monitor_update_id,
@@ -10825,7 +10809,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				funding_tx_confirmed_in,
 				funding_tx_confirmation_height,
 				short_channel_id,
-				channel_creation_height: channel_creation_height.unwrap(),
+				channel_creation_height,
 
 				counterparty_dust_limit_satoshis,
 				holder_dust_limit_satoshis,
@@ -10858,7 +10842,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 
 				latest_inbound_scid_alias,
 				// Later in the ChannelManager deserialization phase we scan for channels and assign scid aliases if its missing
-				outbound_scid_alias: outbound_scid_alias.unwrap_or(0),
+				outbound_scid_alias,
 
 				funding_tx_broadcast_safe_event_emitted: funding_tx_broadcast_safe_event_emitted.unwrap_or(false),
 				channel_pending_event_emitted: channel_pending_event_emitted.unwrap_or(true),
@@ -11567,7 +11551,7 @@ mod tests {
 		let mut s = crate::io::Cursor::new(&encoded_chan);
 		let mut reader = crate::util::ser::FixedLengthReader::new(&mut s, encoded_chan.len() as u64);
 		let features = channelmanager::provided_channel_type_features(&config);
-		let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, 0, &features)).unwrap();
+		let decoded_chan = FundedChannel::read(&mut reader, (&&keys_provider, &&keys_provider, &features)).unwrap();
 		assert_eq!(decoded_chan.context.pending_outbound_htlcs, pending_outbound_htlcs);
 		assert_eq!(decoded_chan.context.holding_cell_htlc_updates, holding_cell_htlc_updates);
 	}

lightning/src/ln/channelmanager.rs

@@ -13634,7 +13634,7 @@ where
 		let mut close_background_events = Vec::new();
 		for _ in 0..channel_count {
 			let mut channel: FundedChannel<SP> = FundedChannel::read(reader, (
-				&args.entropy_source, &args.signer_provider, best_block_height, &provided_channel_type_features(&args.default_config)
+				&args.entropy_source, &args.signer_provider, &provided_channel_type_features(&args.default_config)
 			))?;
 			let logger = WithChannelContext::from(&args.logger, &channel.context, None);
 			let channel_id = channel.context.channel_id();