f - update option_channel_type error msg

Author: elnosh

fuzz/src/full_stack.rs

@@ -1066,7 +1066,7 @@ fn two_peer_forwarding_seed() -> Vec<u8> {
 	ext_from_hex("0010 03000000000000000000000000000000", &mut test);
 	// inbound read from peer id 0 of len 32
 	ext_from_hex("030020", &mut test);
-	// init message (type 16) with static_remotekey required, no channel_type/anchors/taproot, and other bits optional and mac
+	// init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac
 	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 03000000000000000000000000000000", &mut test);
 
 	// inbound read from peer id 0 of len 18
@@ -1167,7 +1167,7 @@ fn two_peer_forwarding_seed() -> Vec<u8> {
 	ext_from_hex("0010 01000000000000000000000000000000", &mut test);
 	// inbound read from peer id 1 of len 32
 	ext_from_hex("030120", &mut test);
-	// init message (type 16) with static_remotekey required, no channel_type/anchors/taproot, and other bits optional and mac
+	// init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac
 	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 01000000000000000000000000000000", &mut test);
 
 	// create outbound channel to peer 1 for 50k sat

lightning/src/ln/channel.rs

@@ -3838,12 +3838,10 @@ where
 			return Err(ChannelError::close("Got an accept_channel message at a strange time".to_owned()));
 		}
 
-		if let Some(ty) = &common_fields.channel_type {
-			if ty != funding.get_channel_type() {
-				return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned()));
-			}
-		} else {
-			return Err(ChannelError::close("channel_type assumed to be supported".to_owned()));
+		let channel_type = common_fields.channel_type.as_ref()
+			.ok_or_else(|| ChannelError::close("option_channel_type assumed to be supported".to_owned()))?;
+		if channel_type != funding.get_channel_type() {
+			return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned()));
 		}
 
 		if common_fields.dust_limit_satoshis > 21000000 * 100000000 {
@@ -11538,30 +11536,29 @@ where
 pub(super) fn channel_type_from_open_channel(
 	common_fields: &msgs::CommonOpenChannelFields, our_supported_features: &ChannelTypeFeatures
 ) -> Result<ChannelTypeFeatures, ChannelError> {
-	if let Some(channel_type) = &common_fields.channel_type {
-		if channel_type.supports_any_optional_bits() {
-			return Err(ChannelError::close("Channel Type field contained optional bits - this is not allowed".to_owned()));
-		}
+	let channel_type = common_fields.channel_type.as_ref()
+		.ok_or_else(|| ChannelError::close("option_channel_type assumed to be supported".to_owned()))?;
 
-		// We only support the channel types defined by the `ChannelManager` in
-		// `provided_channel_type_features`. The channel type must always support
-		// `static_remote_key`, either implicitly with `option_zero_fee_commitments`
-		// or explicitly.
-		if !channel_type.requires_static_remote_key() && !channel_type.requires_anchor_zero_fee_commitments() {
-			return Err(ChannelError::close("Channel Type was not understood - we require static remote key".to_owned()));
-		}
-		// Make sure we support all of the features behind the channel type.
-		if channel_type.requires_unknown_bits_from(&our_supported_features) {
-			return Err(ChannelError::close("Channel Type contains unsupported features".to_owned()));
-		}
-		let announce_for_forwarding = if (common_fields.channel_flags & 1) == 1 { true } else { false };
-		if channel_type.requires_scid_privacy() && announce_for_forwarding {
-			return Err(ChannelError::close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
-		}
-		Ok(channel_type.clone())
-	} else {
-		return Err(ChannelError::close("channel_type assumed to be supported".to_owned()));
+	if channel_type.supports_any_optional_bits() {
+		return Err(ChannelError::close("Channel Type field contained optional bits - this is not allowed".to_owned()));
+	}
+
+	// We only support the channel types defined by the `ChannelManager` in
+	// `provided_channel_type_features`. The channel type must always support
+	// `static_remote_key`, either implicitly with `option_zero_fee_commitments`
+	// or explicitly.
+	if !channel_type.requires_static_remote_key() && !channel_type.requires_anchor_zero_fee_commitments() {
+		return Err(ChannelError::close("Channel Type was not understood - we require static remote key".to_owned()));
+	}
+	// Make sure we support all of the features behind the channel type.
+	if channel_type.requires_unknown_bits_from(&our_supported_features) {
+		return Err(ChannelError::close("Channel Type contains unsupported features".to_owned()));
+	}
+	let announce_for_forwarding = if (common_fields.channel_flags & 1) == 1 { true } else { false };
+	if channel_type.requires_scid_privacy() && announce_for_forwarding {
+		return Err(ChannelError::close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
 	}
+	Ok(channel_type.clone())
 }
 
 impl<SP: Deref> InboundV1Channel<SP>

lightning/src/ln/msgs.rs

@@ -242,7 +242,8 @@ pub struct CommonOpenChannelFields {
 	/// Optionally, a request to pre-set the to-channel-initiator output's scriptPubkey for when we
 	/// collaboratively close
 	pub shutdown_scriptpubkey: Option<ScriptBuf>,
-	/// The channel type that this channel will represent
+	/// The channel type that this channel will represent. As defined in the latest
+	/// specification, this field is required. However, it is an `Option` for legacy reasons.
 	pub channel_type: Option<ChannelTypeFeatures>,
 }
 
@@ -353,7 +354,8 @@ pub struct CommonAcceptChannelFields {
 	/// Optionally, a request to pre-set the to-channel-acceptor output's scriptPubkey for when we
 	/// collaboratively close
 	pub shutdown_scriptpubkey: Option<ScriptBuf>,
-	/// The channel type that this channel will represent
+	/// The channel type that this channel will represent. As defined in the latest
+	/// specification, this field is required. However, it is an `Option` for legacy reasons.
 	///
 	/// This is required to match the equivalent field in [`OpenChannel`] or [`OpenChannelV2`]'s
 	/// [`CommonOpenChannelFields::channel_type`].