Make channel_type required

Set the option_channel_type feature as required and
fail the channel if channel_type is not included during
channel negotiation in open_channel or accept_channel.

Author: elnosh

fuzz/src/full_stack.rs

@@ -1067,7 +1067,7 @@ fn two_peer_forwarding_seed() -> Vec<u8> {
 	// 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
-	ext_from_hex("0010 00021aaa 0008aaa208aa2a0a9aaa 03000000000000000000000000000000", &mut test);
+	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 03000000000000000000000000000000", &mut test);
 
 	// inbound read from peer id 0 of len 18
 	ext_from_hex("030012", &mut test);
@@ -1168,7 +1168,7 @@ fn two_peer_forwarding_seed() -> Vec<u8> {
 	// 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
-	ext_from_hex("0010 00021aaa 0008aaa208aa2a0a9aaa 01000000000000000000000000000000", &mut test);
+	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 01000000000000000000000000000000", &mut test);
 
 	// create outbound channel to peer 1 for 50k sat
 	ext_from_hex(
@@ -1181,16 +1181,16 @@ fn two_peer_forwarding_seed() -> Vec<u8> {
 	// inbound read from peer id 1 of len 18
 	ext_from_hex("030112", &mut test);
 	// message header indicating message length 274
-	ext_from_hex("0112 01000000000000000000000000000000", &mut test);
+	ext_from_hex("0116 01000000000000000000000000000000", &mut test);
 	// inbound read from peer id 1 of len 255
 	ext_from_hex("0301ff", &mut test);
 	// beginning of accept_channel
 	ext_from_hex("0021 0000000000000000000000000000000000000000000000000000000000000e12 0000000000000162 00000000004c4b40 00000000000003e8 00000000000003e8 00000002 03f0 0005 030000000000000000000000000000000000000000000000000000000000000100 030000000000000000000000000000000000000000000000000000000000000200 030000000000000000000000000000000000000000000000000000000000000300 030000000000000000000000000000000000000000000000000000000000000400 030000000000000000000000000000000000000000000000000000000000000500 02660000000000000000000000000000", &mut test);
 	// inbound read from peer id 1 of len 35
-	ext_from_hex("030123", &mut test);
+	ext_from_hex("030127", &mut test);
 	// rest of accept_channel and mac
 	ext_from_hex(
-		"0000000000000000000000000000000000 0000 01000000000000000000000000000000",
+		"0000000000000000000000000000000000 0000 01021000 01000000000000000000000000000000",
 		&mut test,
 	);
 
@@ -1583,7 +1583,7 @@ fn gossip_exchange_seed() -> Vec<u8> {
 	// 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
-	ext_from_hex("0010 00021aaa 0008aaa20aaa2a0a9aaa 03000000000000000000000000000000", &mut test);
+	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 03000000000000000000000000000000", &mut test);
 
 	// new inbound connection with id 1
 	ext_from_hex("01", &mut test);
@@ -1603,7 +1603,7 @@ fn gossip_exchange_seed() -> Vec<u8> {
 	// 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
-	ext_from_hex("0010 00021aaa 0008aaa20aaa2a0a9aaa 01000000000000000000000000000000", &mut test);
+	ext_from_hex("0010 00021aaa 0008aaa218aa2a0a9aaa 01000000000000000000000000000000", &mut test);
 
 	// inbound read from peer id 0 of len 18
 	ext_from_hex("030012", &mut test);

lightning/src/ln/channel.rs

@@ -3842,14 +3842,8 @@ where
 			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 if their_features.supports_channel_type() {
-			// Assume they've accepted the channel type as they said they understand it.
 		} else {
-			let channel_type = ChannelTypeFeatures::from_init(&their_features);
-			if channel_type != ChannelTypeFeatures::only_static_remote_key() {
-				return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
-			}
-			funding.channel_transaction_parameters.channel_type_features = channel_type;
+			return Err(ChannelError::close("channel_type assumed to be supported".to_owned()));
 		}
 
 		if common_fields.dust_limit_satoshis > 21000000 * 100000000 {
@@ -11542,8 +11536,7 @@ where
 /// [`msgs::CommonOpenChannelFields`].
 #[rustfmt::skip]
 pub(super) fn channel_type_from_open_channel(
-	common_fields: &msgs::CommonOpenChannelFields, their_features: &InitFeatures,
-	our_supported_features: &ChannelTypeFeatures
+	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() {
@@ -11567,11 +11560,7 @@ pub(super) fn channel_type_from_open_channel(
 		}
 		Ok(channel_type.clone())
 	} else {
-		let channel_type = ChannelTypeFeatures::from_init(&their_features);
-		if channel_type != ChannelTypeFeatures::only_static_remote_key() {
-			return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
-		}
-		Ok(channel_type)
+		return Err(ChannelError::close("channel_type assumed to be supported".to_owned()));
 	}
 }
 
@@ -11596,7 +11585,7 @@ where
 
 		// First check the channel type is known, failing before we do anything else if we don't
 		// support this channel type.
-		let channel_type = channel_type_from_open_channel(&msg.common_fields, their_features, our_supported_features)?;
+		let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;
 
 		let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.common_fields.funding_satoshis, config);
 		let counterparty_pubkeys = ChannelPublicKeys {
@@ -11999,7 +11988,7 @@ where
 			return Err(ChannelError::close(format!("Rejecting V2 channel {} missing channel_type",
 				msg.common_fields.temporary_channel_id)))
 		}
-		let channel_type = channel_type_from_open_channel(&msg.common_fields, their_features, our_supported_features)?;
+		let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;
 
 		let counterparty_pubkeys = ChannelPublicKeys {
 			funding_pubkey: msg.common_fields.funding_pubkey,

lightning/src/ln/channel_type_tests.rs

@@ -295,9 +295,9 @@ fn do_test_supports_channel_type(config: UserConfig, expected_channel_type: Chan
 }
 
 #[test]
-fn test_rejects_implicit_simple_anchors() {
-	// Tests that if `option_anchors` is being negotiated implicitly through the intersection of
-	// each side's `InitFeatures`, it is rejected.
+fn test_rejects_if_channel_type_not_set() {
+	// Tests that if `channel_type` is not set in `open_channel` and `accept_channel`, it is
+	// rejected.
 	let secp_ctx = Secp256k1::new();
 	let test_est = TestFeeEstimator::new(15000);
 	let fee_estimator = LowerBoundedFeeEstimator::new(&test_est);
@@ -312,13 +312,6 @@ fn test_rejects_implicit_simple_anchors() {
 
 	let config = UserConfig::default();
 
-	// See feature bit assignments: https://github.com/lightning/bolts/blob/master/09-features.md
-	let static_remote_key_required: u64 = 1 << 12;
-	let simple_anchors_required: u64 = 1 << 20;
-	let raw_init_features = static_remote_key_required | simple_anchors_required;
-	let init_features_with_simple_anchors =
-		InitFeatures::from_le_bytes(raw_init_features.to_le_bytes().to_vec());
-
 	let mut channel_a = OutboundV1Channel::<&TestKeysInterface>::new(
 		&fee_estimator,
 		&&keys_provider,
@@ -336,20 +329,18 @@ fn test_rejects_implicit_simple_anchors() {
 	)
 	.unwrap();
 
-	// Set `channel_type` to `None` to force the implicit feature negotiation.
+	// Set `channel_type` to `None` to cause failure.
 	let mut open_channel_msg =
 		channel_a.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
 	open_channel_msg.common_fields.channel_type = None;
 
-	// Since A supports both `static_remote_key` and `option_anchors`, but B only accepts
-	// `static_remote_key`, it will fail the channel.
 	let channel_b = InboundV1Channel::<&TestKeysInterface>::new(
 		&fee_estimator,
 		&&keys_provider,
 		&&keys_provider,
 		node_id_a,
 		&channelmanager::provided_channel_type_features(&config),
-		&init_features_with_simple_anchors,
+		&channelmanager::provided_init_features(&config),
 		&open_channel_msg,
 		7,
 		&config,
@@ -358,6 +349,35 @@ fn test_rejects_implicit_simple_anchors() {
 		/*is_0conf=*/ false,
 	);
 	assert!(channel_b.is_err());
+
+	open_channel_msg.common_fields.channel_type =
+		Some(channel_a.funding.get_channel_type().clone());
+	let mut channel_b = InboundV1Channel::<&TestKeysInterface>::new(
+		&fee_estimator,
+		&&keys_provider,
+		&&keys_provider,
+		node_id_a,
+		&channelmanager::provided_channel_type_features(&config),
+		&channelmanager::provided_init_features(&config),
+		&open_channel_msg,
+		7,
+		&config,
+		0,
+		&&logger,
+		/*is_0conf=*/ false,
+	)
+	.unwrap();
+
+	// Set `channel_type` to `None` in `accept_channel` to cause failure.
+	let mut accept_channel_msg = channel_b.get_accept_channel_message(&&logger).unwrap();
+	accept_channel_msg.common_fields.channel_type = None;
+
+	let res = channel_a.accept_channel(
+		&accept_channel_msg,
+		&config.channel_handshake_limits,
+		&channelmanager::provided_init_features(&config),
+	);
+	assert!(res.is_err());
 }
 
 #[test]

lightning/src/ln/channelmanager.rs

@@ -8593,7 +8593,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		// We can get the channel type at this point already as we'll need it immediately in both the
 		// manual and the automatic acceptance cases.
 		let channel_type = channel::channel_type_from_open_channel(
-			common_fields, &peer_state.latest_features, &self.channel_type_features()
+			common_fields, &self.channel_type_features()
 		).map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, common_fields.temporary_channel_id))?;
 
 		// If we're doing manual acceptance checks on the channel, then defer creation until we're sure we want to accept.
@@ -13694,7 +13694,7 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures {
 	features.set_basic_mpp_optional();
 	features.set_wumbo_optional();
 	features.set_shutdown_any_segwit_optional();
-	features.set_channel_type_optional();
+	features.set_channel_type_required();
 	features.set_scid_privacy_optional();
 	features.set_zero_conf_optional();
 	features.set_route_blinding_optional();