Disallow opening and accepting 0-reserve legacy channels

lightning/src/ln/channel.rs

@@ -3832,6 +3832,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 				"Funding must be smaller than the total bitcoin supply. It was {channel_value_satoshis}"
 			)));
 		}
+		if !channel_type.supports_anchors_zero_fee_htlc_tx()
+			&& !channel_type.supports_anchor_zero_fee_commitments()
+			&& (msg_channel_reserve_satoshis == 0 || holder_selected_channel_reserve_satoshis == 0)
+		{
+			return Err(ChannelError::close(format!(
+				"0-reserve is not allowed on legacy channels"
+			)));
+		}
 		if msg_channel_reserve_satoshis > channel_value_satoshis {
 			return Err(ChannelError::close(format!(
 				"Bogus channel_reserve_satoshis ({msg_channel_reserve_satoshis}). Must be no greater than channel_value_satoshis: {channel_value_satoshis}"
@@ -4323,6 +4331,14 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 		}
 
 		let channel_type = get_initial_channel_type(&config, their_features);
+		if !channel_type.supports_anchors_zero_fee_htlc_tx()
+			&& !channel_type.supports_anchor_zero_fee_commitments()
+			&& holder_selected_channel_reserve_satoshis == 0
+		{
+			return Err(APIError::APIMisuseError {
+				err: format!("0-reserve is not allowed on legacy channels"),
+			});
+		}
 		debug_assert!(!channel_type.supports_any_optional_bits());
 		debug_assert!(!channel_type
 			.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config)));
@@ -4869,6 +4885,15 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 		}
 
 		let channel_type = funding.get_channel_type();
+		if !channel_type.supports_anchors_zero_fee_htlc_tx()
+			&& !channel_type.supports_anchor_zero_fee_commitments()
+			&& (channel_reserve_satoshis == 0
+				|| funding.holder_selected_channel_reserve_satoshis == 0)
+		{
+			return Err(ChannelError::close(
+				"0-reserve is not allowed on legacy channels".to_owned(),
+			));
+		}
 		if common_fields.max_accepted_htlcs > max_htlcs(channel_type) {
 			return Err(ChannelError::close(format!(
 				"max_accepted_htlcs was {}. It must not be larger than {}",
@@ -6537,17 +6562,15 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 	/// If we receive an error message when attempting to open a channel, it may only be a rejection
 	/// of the channel type we tried, not of our ability to open any channel at all. We can see if a
 	/// downgrade of channel features would be possible so that we can still open the channel.
-	#[rustfmt::skip]
 	pub(crate) fn maybe_downgrade_channel_features<F: FeeEstimator>(
 		&mut self, funding: &mut FundingScope, fee_estimator: &LowerBoundedFeeEstimator<F>,
 		user_config: &UserConfig, their_features: &InitFeatures,
 	) -> Result<(), ()> {
-		if !funding.is_outbound() ||
-			!matches!(
+		if !funding.is_outbound()
+			|| !matches!(
 				self.channel_state, ChannelState::NegotiatingFunding(flags)
 				if flags == NegotiatingFundingFlags::OUR_INIT_SENT
-			)
-		{
+			) {
 			return Err(());
 		}
 		if funding.get_channel_type() == &ChannelTypeFeatures::only_static_remote_key() {
@@ -6578,11 +6601,17 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 		}
 
 		let next_channel_type = get_initial_channel_type(user_config, &eligible_features);
+		if !next_channel_type.supports_anchors_zero_fee_htlc_tx()
+			&& !next_channel_type.supports_anchor_zero_fee_commitments()
+			&& funding.holder_selected_channel_reserve_satoshis == 0
+		{
+			// 0-reserve is not allowed on legacy channels
+			return Err(());
+		}
 
-		self.feerate_per_kw = selected_commitment_sat_per_1000_weight(
-			&fee_estimator, &next_channel_type,
-		);
-	 	funding.channel_transaction_parameters.channel_type_features = next_channel_type;
+		self.feerate_per_kw =
+			selected_commitment_sat_per_1000_weight(&fee_estimator, &next_channel_type);
+		funding.channel_transaction_parameters.channel_type_features = next_channel_type;
 
 		Ok(())
 	}

lightning/src/ln/channel_open_tests.rs

@@ -24,7 +24,8 @@ use crate::ln::channelmanager::{
 	MAX_UNFUNDED_CHANS_PER_PEER,
 };
 use crate::ln::msgs::{
-	AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent,
+	AcceptChannel, BaseMessageHandler, ChannelMessageHandler, ErrorAction, ErrorMessage,
+	MessageSendEvent,
 };
 use crate::ln::types::ChannelId;
 use crate::ln::{functional_test_utils::*, msgs};
@@ -48,6 +49,7 @@ use bitcoin::{Amount, Sequence, Transaction, TxIn, TxOut, Witness};
 use lightning_macros::xtest;
 
 use lightning_types::features::ChannelTypeFeatures;
+use types::string::UntrustedString;
 
 #[test]
 fn test_outbound_chans_unlimited() {
@@ -2496,3 +2498,219 @@ fn test_fund_pending_channel() {
 	};
 	check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100_000);
 }
+
+#[xtest(feature = "_externalize_tests")]
+fn test_legacy_0reserve_is_not_allowed() {
+	{
+		let chanmon_cfgs = create_chanmon_cfgs(2);
+		let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+		let mut channel_config = test_default_channel_config();
+		assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx);
+		let node_chanmgrs = create_node_chanmgrs(
+			2,
+			&node_cfgs,
+			&[Some(channel_config.clone()), Some(channel_config.clone())],
+		);
+		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+		let _node_a_id = nodes[0].node.get_our_node_id();
+		let node_b_id = nodes[1].node.get_our_node_id();
+
+		let mut legacy_channel_config = test_default_channel_config();
+		legacy_channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
+		legacy_channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments =
+			false;
+
+		assert_eq!(
+			nodes[0]
+				.node
+				.create_channel_to_trusted_peer_0reserve(
+					node_b_id,
+					100_000,
+					0,
+					42,
+					None,
+					Some(legacy_channel_config)
+				)
+				.unwrap_err(),
+			APIError::APIMisuseError {
+				err: "0-reserve is not allowed on legacy channels".to_owned()
+			}
+		);
+	}
+	{
+		let chanmon_cfgs = create_chanmon_cfgs(2);
+		let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+		let mut channel_config = test_default_channel_config();
+		channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
+		channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
+		let node_chanmgrs = create_node_chanmgrs(
+			2,
+			&node_cfgs,
+			&[Some(channel_config.clone()), Some(channel_config.clone())],
+		);
+		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+		let _node_a_id = nodes[0].node.get_our_node_id();
+		let node_b_id = nodes[1].node.get_our_node_id();
+
+		assert_eq!(
+			nodes[0]
+				.node
+				.create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None)
+				.unwrap_err(),
+			APIError::APIMisuseError {
+				err: "0-reserve is not allowed on legacy channels".to_owned()
+			}
+		);
+	}
+
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let mut channel_config = test_default_channel_config();
+	channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = false;
+	channel_config.channel_handshake_config.negotiate_anchor_zero_fee_commitments = false;
+	let node_chanmgrs = create_node_chanmgrs(
+		2,
+		&node_cfgs,
+		&[Some(channel_config.clone()), Some(channel_config.clone())],
+	);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_a_id = nodes[0].node.get_our_node_id();
+	let node_b_id = nodes[1].node.get_our_node_id();
+
+	nodes[0].node.create_channel(node_b_id, 100_000, 0, 42, None, None).unwrap();
+	let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);
+	let mut err_open_channel_msg = open_channel_msg.clone();
+	err_open_channel_msg.channel_reserve_satoshis = 0;
+	assert_eq!(
+		err_open_channel_msg.common_fields.channel_type,
+		Some(ChannelTypeFeatures::only_static_remote_key())
+	);
+
+	nodes[1].node.handle_open_channel(node_a_id, &err_open_channel_msg);
+	let events = nodes[1].node.get_and_clear_pending_events();
+	match events[0] {
+		Event::OpenChannelRequest { temporary_channel_id, .. } => {
+			let error = nodes[1]
+				.node
+				.accept_inbound_channel_from_trusted_peer(
+					&temporary_channel_id,
+					&node_a_id,
+					42,
+					TrustedChannelFeatures::ZeroReserve,
+					None,
+				)
+				.unwrap_err();
+			assert_eq!(
+				error,
+				APIError::ChannelUnavailable {
+					err: "0-reserve is not allowed on legacy channels".to_owned()
+				}
+			);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	let err_msg = get_err_msg(&nodes[1], &node_a_id);
+	assert_eq!(
+		err_msg,
+		ErrorMessage {
+			channel_id: err_open_channel_msg.common_fields.temporary_channel_id,
+			data: "0-reserve is not allowed on legacy channels".to_string()
+		}
+	);
+
+	nodes[1].node.handle_open_channel(node_a_id, &err_open_channel_msg);
+	let events = nodes[1].node.get_and_clear_pending_events();
+	match events[0] {
+		Event::OpenChannelRequest { temporary_channel_id, .. } => {
+			let error = nodes[1]
+				.node
+				.accept_inbound_channel(&temporary_channel_id, &node_a_id, 42, None)
+				.unwrap_err();
+			assert_eq!(
+				error,
+				APIError::ChannelUnavailable {
+					err: "0-reserve is not allowed on legacy channels".to_owned()
+				}
+			);
+		},
+		_ => panic!("Unexpected event"),
+	}
+	let err_msg = get_err_msg(&nodes[1], &node_a_id);
+	assert_eq!(
+		err_msg,
+		ErrorMessage {
+			channel_id: err_open_channel_msg.common_fields.temporary_channel_id,
+			data: "0-reserve is not allowed on legacy channels".to_string()
+		}
+	);
+
+	handle_and_accept_open_channel(&nodes[1], node_a_id, &open_channel_msg);
+	let mut accept_channel_msg =
+		get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, node_a_id);
+	accept_channel_msg.channel_reserve_satoshis = 0;
+
+	nodes[0].node.handle_accept_channel(node_b_id, &accept_channel_msg);
+	let err_msg = get_err_msg(&nodes[0], &node_b_id);
+	assert_eq!(
+		err_msg,
+		ErrorMessage {
+			channel_id: open_channel_msg.common_fields.temporary_channel_id,
+			data: "0-reserve is not allowed on legacy channels".to_string()
+		}
+	);
+	let err = "0-reserve is not allowed on legacy channels".to_owned();
+	let reason = ClosureReason::ProcessingError { err };
+	let expected_closing = ExpectedCloseEvent::from_id_reason(
+		open_channel_msg.common_fields.temporary_channel_id,
+		false,
+		reason,
+	);
+	check_closed_events(&nodes[0], &[expected_closing]);
+}
+
+#[xtest(feature = "_externalize_tests")]
+fn test_error_if_0reserve_negotiates_down_to_legacy() {
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let channel_config = test_default_channel_config();
+	assert!(channel_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx);
+	let node_chanmgrs = create_node_chanmgrs(
+		2,
+		&node_cfgs,
+		&[Some(channel_config.clone()), Some(channel_config.clone())],
+	);
+	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_b_id = nodes[1].node.get_our_node_id();
+
+	nodes[0]
+		.node
+		.create_channel_to_trusted_peer_0reserve(node_b_id, 100_000, 0, 42, None, None)
+		.unwrap();
+	let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);
+	assert_eq!(
+		open_channel_msg.common_fields.channel_type,
+		Some(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies())
+	);
+	assert_eq!(open_channel_msg.channel_reserve_satoshis, 0);
+
+	let reason = "Don't like your channel".to_owned();
+	nodes[0].node.handle_error(
+		node_b_id,
+		&ErrorMessage {
+			channel_id: open_channel_msg.common_fields.temporary_channel_id,
+			data: reason.clone(),
+		},
+	);
+
+	let reason = ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(reason) };
+	let expected_closing = ExpectedCloseEvent::from_id_reason(
+		open_channel_msg.common_fields.temporary_channel_id,
+		false,
+		reason,
+	);
+	check_closed_events(&nodes[0], &[expected_closing]);
+}

lightning/src/ln/channelmanager.rs

@@ -3617,6 +3617,8 @@ pub enum TrustedChannelFeatures {
 	/// with a revoked commitment transaction *for free*.
 	///
 	/// Note that there is no guarantee that the counterparty accepts such a channel themselves.
+	///
+	/// The zero-reserve feature is not allowed on legacy / anchorless channels.
 	ZeroReserve,
 	/// Sets the combination of [`TrustedChannelFeatures::ZeroConf`] and [`TrustedChannelFeatures::ZeroReserve`]
 	ZeroConfZeroReserve,
@@ -3873,6 +3875,8 @@ impl<
 	/// transaction *for free*.
 	///
 	/// Note that there is no guarantee that the counterparty accepts such a channel.
+	///
+	/// The zero-reserve feature is not allowed on legacy / anchorless channels.
 	pub fn create_channel_to_trusted_peer_0reserve(
 		&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64,
 		user_channel_id: u128, temporary_channel_id: Option<ChannelId>,