diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index f6fa07199fa..65d630c9a90 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1066,8 +1066,8 @@ fn two_peer_forwarding_seed() -> Vec { 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 - ext_from_hex("0010 00021aaa 0008aaa208aa2a0a9aaa 03000000000000000000000000000000", &mut test); + // init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac + ext_from_hex("0010 00021aaa 0008aaa210aa2a0a9aaa 03000000000000000000000000000000", &mut test); // inbound read from peer id 0 of len 18 ext_from_hex("030012", &mut test); @@ -1167,8 +1167,8 @@ fn two_peer_forwarding_seed() -> Vec { 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 - ext_from_hex("0010 00021aaa 0008aaa208aa2a0a9aaa 01000000000000000000000000000000", &mut test); + // init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac + ext_from_hex("0010 00021aaa 0008aaa210aa2a0a9aaa 01000000000000000000000000000000", &mut test); // create outbound channel to peer 1 for 50k sat ext_from_hex( @@ -1180,17 +1180,17 @@ fn two_peer_forwarding_seed() -> Vec { // 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); + // message header indicating message length 278 + 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); + // inbound read from peer id 1 of len 39 + ext_from_hex("030127", &mut test); // rest of accept_channel and mac ext_from_hex( - "0000000000000000000000000000000000 0000 01000000000000000000000000000000", + "0000000000000000000000000000000000 0000 01021000 01000000000000000000000000000000", &mut test, ); @@ -1582,8 +1582,8 @@ fn gossip_exchange_seed() -> Vec { 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 - ext_from_hex("0010 00021aaa 0008aaa20aaa2a0a9aaa 03000000000000000000000000000000", &mut test); + // init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac + ext_from_hex("0010 00021aaa 0008aaa210aa2a0a9aaa 03000000000000000000000000000000", &mut test); // new inbound connection with id 1 ext_from_hex("01", &mut test); @@ -1602,8 +1602,8 @@ fn gossip_exchange_seed() -> Vec { 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 - ext_from_hex("0010 00021aaa 0008aaa20aaa2a0a9aaa 01000000000000000000000000000000", &mut test); + // init message (type 16) with static_remotekey required, no anchors/taproot, and other bits optional and mac + ext_from_hex("0010 00021aaa 0008aaa210aa2a0a9aaa 01000000000000000000000000000000", &mut test); // inbound read from peer id 0 of len 18 ext_from_hex("030012", &mut test); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 09c9f5804fc..9b2bd7f66e3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3845,18 +3845,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 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; + 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 { @@ -11541,37 +11533,31 @@ 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 { - 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 { - 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) + 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 InboundV1Channel @@ -11595,7 +11581,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 { @@ -11992,13 +11978,7 @@ where let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis( channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS); - // First check the channel type is known, failing before we do anything else if we don't - // support this channel type. - if msg.common_fields.channel_type.is_none() { - 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, diff --git a/lightning/src/ln/channel_type_tests.rs b/lightning/src/ln/channel_type_tests.rs index 97eb7e06b55..13470d50614 100644 --- a/lightning/src/ln/channel_type_tests.rs +++ b/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,104 @@ 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] +fn test_rejects_if_channel_type_differ() { + // Tests that if the `channel_type` in `accept_channel` does not match the one set in + // `open_channel` it rejects the channel. + let secp_ctx = Secp256k1::new(); + let test_est = TestFeeEstimator::new(15000); + let fee_estimator = LowerBoundedFeeEstimator::new(&test_est); + let network = Network::Testnet; + let keys_provider = TestKeysInterface::new(&[42; 32], network); + let logger = TestLogger::new(); + + let node_id_a = + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[1; 32]).unwrap()); + let node_id_b = + PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[2; 32]).unwrap()); + + let config = UserConfig::default(); + + let mut channel_a = OutboundV1Channel::<&TestKeysInterface>::new( + &fee_estimator, + &&keys_provider, + &&keys_provider, + node_id_b, + &channelmanager::provided_init_features(&config), + 10000000, + 100000, + 42, + &config, + 0, + 42, + None, + &logger, + ) + .unwrap(); + + let open_channel_msg = + channel_a.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap(); + + 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(); + + // Change the `channel_type` in `accept_channel` msg to make it different from the one set in + // `open_channel` to cause failure. + let mut accept_channel_msg = channel_b.get_accept_channel_message(&&logger).unwrap(); + let mut channel_type = channelmanager::provided_channel_type_features(&config); + channel_type.set_zero_conf_required(); + accept_channel_msg.common_fields.channel_type = Some(channel_type.clone()); + + let res = channel_a.accept_channel( + &accept_channel_msg, + &config.channel_handshake_limits, + &channelmanager::provided_init_features(&config), + ); + assert!(res.is_err()); } #[test] diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9fe31c300d8..cd03cda9903 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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. @@ -13695,7 +13695,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(); diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 2cf7e109eb1..9c02c9825b4 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -242,10 +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, - /// The channel type that this channel will represent - /// - /// If this is `None`, we derive the channel type from the intersection of our - /// feature bits with our counterparty's feature bits from the [`Init`] message. + /// 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, } @@ -356,9 +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, - /// The channel type that this channel will represent. If none is set, we derive the channel - /// type from the intersection of our feature bits with our counterparty's feature bits from - /// the Init message. + /// 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`].