Merge pull request lightningdevkit#4580 from tankyleo/2026-04-reserve-greater-than-channel-value

Error if the calculated reserve would be greater than the channel value

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -2753,16 +2753,14 @@ impl FundingScope {
 	) -> Result<Self, String> {
 		if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
 			return Err(format!(
-				"Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply",
-				context.channel_id(),
+				"Our {} contribution exceeds the total bitcoin supply",
 				our_funding_contribution,
 			));
 		}
 
 		if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
 			return Err(format!(
-				"Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply",
-				context.channel_id(),
+				"Their {} contribution exceeds the total bitcoin supply",
 				their_funding_contribution,
 			));
 		}
@@ -2822,17 +2820,31 @@ impl FundingScope {
 		// New reserve values are based on the new channel value and are v2-specific
 		let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
 			post_channel_value_sat,
-			MIN_CHAN_DUST_LIMIT_SATOSHIS,
+			context.holder_dust_limit_satoshis,
 			prev_funding
 				.counterparty_selected_channel_reserve_satoshis
 				.expect("counterparty reserve is set")
 				== 0,
-		);
+		)
+		.map_err(|()| {
+			format!(
+				"The post-splice channel value {post_channel_value_sat} is smaller \
+				than our dust limit {}",
+				context.holder_dust_limit_satoshis
+			)
+		})?;
 		let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
 			post_channel_value_sat,
 			context.counterparty_dust_limit_satoshis,
 			prev_funding.holder_selected_channel_reserve_satoshis == 0,
-		);
+		)
+		.map_err(|()| {
+			format!(
+				"The post-splice channel value {post_channel_value_sat} is smaller \
+				than their dust limit {}",
+				context.counterparty_dust_limit_satoshis,
+			)
+		})?;
 
 		Ok(Self {
 			channel_transaction_parameters: post_channel_transaction_parameters,
@@ -3395,6 +3407,9 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
 	/// We use this to close if funding is never broadcasted.
 	pub(super) channel_creation_height: u32,
 
+	#[cfg(any(test, feature = "_test_utils"))]
+	pub(crate) counterparty_dust_limit_satoshis: u64,
+	#[cfg(not(any(test, feature = "_test_utils")))]
 	counterparty_dust_limit_satoshis: u64,
 
 	#[cfg(any(test, feature = "_test_utils"))]
@@ -6757,20 +6772,32 @@ fn get_legacy_default_holder_max_htlc_value_in_flight_msat(channel_value_satoshi
 /// This is used both for outbound and inbound channels and has lower bound
 /// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`, and the `dust_limit_satoshis` of
 /// the counterparty.
+///
+/// Returns `Err` if `channel_value_satoshis` is smaller than
+/// `MIN_THEIR_CHAN_RESERVE_SATOSHIS` or the `dust_limit_satoshis` of the
+/// counterparty.
 pub(crate) fn get_holder_selected_channel_reserve_satoshis(
 	channel_value_satoshis: u64, their_dust_limit_satoshis: u64, config: &UserConfig,
 	is_0reserve: bool,
-) -> u64 {
+) -> Result<u64, ()> {
+	if channel_value_satoshis < MIN_THEIR_CHAN_RESERVE_SATOSHIS
+		|| channel_value_satoshis < their_dust_limit_satoshis
+	{
+		return Err(());
+	}
 	if is_0reserve {
-		return 0;
+		return Ok(0);
 	}
-	let counterparty_chan_reserve_prop_mil =
-		config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64;
+	// As described in the `ChannelHandshakeConfig` docs, we cap this value at 1_000_000.
+	let counterparty_chan_reserve_prop_mil = cmp::min(
+		config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64,
+		1_000_000,
+	);
 	let calculated_reserve =
 		channel_value_satoshis.saturating_mul(counterparty_chan_reserve_prop_mil) / 1_000_000;
 	let channel_reserve_satoshis = cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS);
 	let channel_reserve_satoshis = cmp::max(channel_reserve_satoshis, their_dust_limit_satoshis);
-	cmp::min(channel_value_satoshis, channel_reserve_satoshis)
+	Ok(channel_reserve_satoshis)
 }
 
 /// This is for legacy reasons, present for forward-compatibility.
@@ -6787,19 +6814,24 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis(
 /// Returns a minimum channel reserve value each party needs to maintain, fixed in the spec to a
 /// default of 1% of the total channel value.
 ///
-/// Guaranteed to return a value no larger than channel_value_satoshis
+/// Guaranteed to return a value no larger than `channel_value_satoshis`
 ///
 /// This is used both for outbound and inbound channels and has lower bound
 /// of `dust_limit_satoshis`.
+///
+/// Returns `Err` if `channel_value_satoshis` is smaller than `dust_limit_satoshis`.
 pub(crate) fn get_v2_channel_reserve_satoshis(
 	channel_value_satoshis: u64, dust_limit_satoshis: u64, is_0reserve: bool,
-) -> u64 {
+) -> Result<u64, ()> {
+	if channel_value_satoshis < dust_limit_satoshis {
+		return Err(());
+	}
 	if is_0reserve {
-		return 0;
+		return Ok(0);
 	}
 	// Fixed at 1% of channel value by spec.
 	let (q, _) = channel_value_satoshis.overflowing_div(100);
-	cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis))
+	Ok(cmp::max(q, dust_limit_satoshis))
 }
 
 /// Returns the minimum feerate for RBF attempts given a previous feerate.
@@ -12922,7 +12954,8 @@ where
 			their_funding_contribution,
 			counterparty_funding_pubkey,
 			our_new_holder_keys,
-		)?;
+		)
+		.map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?;
 
 		let (post_splice_holder_balance, post_splice_counterparty_balance) =
 			self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err(
@@ -14578,12 +14611,19 @@ impl<SP: SignerProvider> OutboundV1Channel<SP> {
 		// a dust limit higher than our selected reserve.
 		let their_dust_limit_satoshis = 0;
 		let is_0reserve = trusted_channel_features.is_some_and(|f| f.is_0reserve());
-		let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(
-			channel_value_satoshis,
-			their_dust_limit_satoshis,
-			config,
-			is_0reserve,
-		);
+		let holder_selected_channel_reserve_satoshis =
+			get_holder_selected_channel_reserve_satoshis(
+				channel_value_satoshis,
+				their_dust_limit_satoshis,
+				config,
+				is_0reserve,
+			)
+			.map_err(|()| APIError::APIMisuseError {
+				err: format!(
+					"The channel value {channel_value_satoshis} is smaller than \
+					{MIN_THEIR_CHAN_RESERVE_SATOSHIS}"
+				),
+			})?;
 		if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS && !is_0reserve {
 			// Protocol level safety check in place, although it should never happen because
 			// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` and `MIN_CHANNEL_VALUE_SATOSHIS`
@@ -14975,12 +15015,20 @@ impl<SP: SignerProvider> InboundV1Channel<SP> {
 		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,
-			msg.common_fields.dust_limit_satoshis,
-			config,
-			trusted_channel_features.is_some_and(|f| f.is_0reserve()),
-		);
+		let holder_selected_channel_reserve_satoshis =
+			get_holder_selected_channel_reserve_satoshis(
+				msg.common_fields.funding_satoshis,
+				msg.common_fields.dust_limit_satoshis,
+				config,
+				trusted_channel_features.is_some_and(|f| f.is_0reserve()),
+			)
+			.map_err(|()| {
+				ChannelError::close(format!(
+					"The channel value {} is smaller than either their dust \
+					limit {}, or {MIN_THEIR_CHAN_RESERVE_SATOSHIS}",
+					msg.common_fields.funding_satoshis, msg.common_fields.dust_limit_satoshis,
+				))
+			})?;
 		let counterparty_pubkeys = ChannelPublicKeys {
 			funding_pubkey: msg.common_fields.funding_pubkey,
 			revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint),
@@ -15237,8 +15285,13 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
 		});
 
 		let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
-			funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve()));
-
+			funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve())
+		).map_err(|()| APIError::APIMisuseError {
+			err: format!(
+				"The channel value {funding_satoshis} is smaller than their dust \
+				limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
+			)
+		})?;
 		let funding_feerate_sat_per_1000_weight = fee_estimator.bounded_sat_per_1000_weight(funding_confirmation_target);
 		let funding_tx_locktime = LockTime::from_height(current_chain_height)
 			.map_err(|_| APIError::APIMisuseError {
@@ -15377,9 +15430,16 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
 		let channel_value_satoshis =
 			our_funding_contribution_sats.saturating_add(msg.common_fields.funding_satoshis);
 		let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
-			channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some());
+			channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some()
+		).map_err(|()| ChannelError::close(format!(
+			"The channel value {channel_value_satoshis} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
+		)))?;
+		let their_dust_limit_satoshis = msg.common_fields.dust_limit_satoshis;
 		let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
-			channel_value_satoshis, msg.common_fields.dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve()));
+			channel_value_satoshis, their_dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve())
+		).map_err(|()| ChannelError::close(format!(
+			"The channel value {channel_value_satoshis} is smaller than their dust limit {their_dust_limit_satoshis}"
+		)))?;
 
 		let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;
 
@@ -17570,6 +17630,10 @@ mod tests {
 		// to channel value
 		test_self_and_counterparty_channel_reserve(10_000_000, 0.50, 0.50);
 		test_self_and_counterparty_channel_reserve(10_000_000, 0.60, 0.50);
+
+		// Make sure we correctly handle reserves greater than the channel value
+		test_self_and_counterparty_channel_reserve(100_000, 1.1, 0.30);
+		test_self_and_counterparty_channel_reserve(100_000, 0.30, 1.1);
 	}
 
 	#[rustfmt::skip]
@@ -17589,7 +17653,19 @@ mod tests {
 		outbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (outbound_selected_channel_reserve_perc * 1_000_000.0) as u32;
 		let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, outbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), channel_value_satoshis, 100_000, 42, &outbound_node_config, 0, 42, None, &logger, None).unwrap();
 
-		let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_selected_channel_reserve_perc) as u64);
+		let outbound_capped_reserve_perc = if outbound_selected_channel_reserve_perc.lt(&1.0) {
+			outbound_selected_channel_reserve_perc
+		} else {
+			1.0
+		};
+
+		let inbound_capped_reserve_perc = if inbound_selected_channel_reserve_perc.lt(&1.0) {
+			inbound_selected_channel_reserve_perc
+		} else {
+			1.0
+		};
+
+		let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_capped_reserve_perc) as u64);
 		assert_eq!(chan.funding.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve);
 
 		let chan_open_channel_msg = chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
@@ -17599,7 +17675,7 @@ mod tests {
 		if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 {
 			let chan_inbound_node = InboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, None).unwrap();
 
-			let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_selected_channel_reserve_perc) as u64);
+			let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_capped_reserve_perc) as u64);
 
 			assert_eq!(chan_inbound_node.funding.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve);
 			assert_eq!(chan_inbound_node.funding.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve);

lightning/src/ln/channel_open_tests.rs

@@ -16,7 +16,8 @@ use crate::chain::{self, ChannelMonitorUpdateStatus};
 use crate::events::{ClosureReason, Event, FundingInfo};
 use crate::ln::channel::{
 	get_holder_selected_channel_reserve_satoshis, ChannelError, InboundV1Channel,
-	OutboundV1Channel, COINBASE_MATURITY, UNFUNDED_CHANNEL_AGE_LIMIT_TICKS,
+	OutboundV1Channel, COINBASE_MATURITY, MIN_THEIR_CHAN_RESERVE_SATOSHIS,
+	UNFUNDED_CHANNEL_AGE_LIMIT_TICKS,
 };
 use crate::ln::channelmanager::{
 	self, TrustedChannelFeatures, BREAKDOWN_TIMEOUT, MAX_UNFUNDED_CHANNEL_PEERS,
@@ -473,7 +474,8 @@ pub fn test_insane_channel_opens() {
 	// funding satoshis
 	let channel_value_sat = 31337; // same as funding satoshis
 	let channel_reserve_satoshis =
-		get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false);
+		get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false)
+			.unwrap();
 	let push_msat = (channel_value_sat - channel_reserve_satoshis) * 1000;
 
 	// Have node0 initiate a channel to node1 with aforementioned parameters
@@ -552,7 +554,13 @@ pub fn test_insane_channel_opens() {
 		},
 	);
 
-	insane_open_helper("Peer never wants payout outputs?", |mut msg| {
+	let crazy_dust_limit = channel_value_sat + 1;
+	let expected_error_str = format!(
+		"Got non-closing error: The channel value \
+		{channel_value_sat} is smaller than either their dust limit {crazy_dust_limit}, or \
+		{MIN_THEIR_CHAN_RESERVE_SATOSHIS}"
+	);
+	insane_open_helper(&expected_error_str, |mut msg| {
 		msg.common_fields.dust_limit_satoshis = msg.common_fields.funding_satoshis + 1;
 		msg
 	});

lightning/src/ln/functional_tests.rs

@@ -415,7 +415,8 @@ pub fn test_inbound_outbound_capacity_is_not_zero() {
 	assert_eq!(channels0.len(), 1);
 	assert_eq!(channels1.len(), 1);
 
-	let reserve = get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false);
+	let reserve =
+		get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false).unwrap();
 	assert_eq!(channels0[0].inbound_capacity_msat, 95000000 - reserve * 1000);
 	assert_eq!(channels1[0].outbound_capacity_msat, 95000000 - reserve * 1000);
 

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -55,8 +55,9 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
 	push_amt -= feerate_per_kw as u64
 		* (commitment_tx_base_weight(&channel_type_features) + 4 * COMMITMENT_TX_WEIGHT_PER_HTLC)
 		/ 1000 * 1000;
-	push_amt -=
-		get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
+	push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
+		.unwrap()
+		* 1000;
 
 	let push = if send_from_initiator { 0 } else { push_amt };
 	let temp_channel_id =
@@ -1002,8 +1003,9 @@ pub fn test_chan_reserve_violation_outbound_htlc_inbound_chan() {
 		&channel_type_features,
 	);
 
-	push_amt -=
-		get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
+	push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
+		.unwrap()
+		* 1000;
 
 	let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt);
 
@@ -1048,8 +1050,9 @@ pub fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
 		MIN_AFFORDABLE_HTLC_COUNT as u64,
 		&channel_type_features,
 	);
-	push_amt -=
-		get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
+	push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
+		.unwrap()
+		* 1000;
 	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, push_amt);
 
 	let (htlc_success_tx_fee_sat, _) =

lightning/src/ln/payment_tests.rs

@@ -5063,7 +5063,7 @@ fn test_htlc_forward_considers_anchor_outputs_value() {
 		create_announced_chan_between_nodes_with_value(&nodes, 1, 2, CHAN_AMT, PUSH_MSAT);
 
 	let channel_reserve_msat =
-		get_holder_selected_channel_reserve_satoshis(CHAN_AMT, 0, &config, false) * 1000;
+		get_holder_selected_channel_reserve_satoshis(CHAN_AMT, 0, &config, false).unwrap() * 1000;
 	let commitment_fee_msat = chan_utils::commit_tx_fee_sat(
 		*nodes[1].fee_estimator.sat_per_kw.lock().unwrap(),
 		2,