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

fuzz/src/router.rs

@@ -248,6 +248,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 							outbound_capacity_msat: capacity.saturating_mul(1000),
 							next_outbound_htlc_limit_msat: capacity.saturating_mul(1000),
 							next_outbound_htlc_minimum_msat: 0,
+							next_splice_out_maximum_sat: capacity,
 							inbound_htlc_minimum_msat: None,
 							inbound_htlc_maximum_msat: None,
 							config: None,

lightning/src/ln/channel_open_tests.rs

@@ -473,7 +473,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 +553,12 @@ 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 their dust limit {crazy_dust_limit}"
+	);
+	insane_open_helper(&expected_error_str, |mut msg| {
 		msg.common_fields.dust_limit_satoshis = msg.common_fields.funding_satoshis + 1;
 		msg
 	});

lightning/src/ln/channel_state.rs

@@ -399,6 +399,8 @@ pub struct ChannelDetails {
 	/// an upper-bound. This is intended for use when routing, allowing us to ensure we pick a
 	/// route which is valid.
 	pub next_outbound_htlc_minimum_msat: u64,
+	/// The maximum value of the next splice out from our channel balance.
+	pub next_splice_out_maximum_sat: u64,
 	/// The available inbound capacity for the remote peer to send HTLCs to us. This does not
 	/// include any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not
 	/// available for inclusion in new inbound HTLCs).
@@ -533,6 +535,7 @@ impl ChannelDetails {
 				outbound_capacity_msat: 0,
 				next_outbound_htlc_limit_msat: 0,
 				next_outbound_htlc_minimum_msat: u64::MAX,
+				next_splice_out_maximum_sat: 0,
 			}
 		});
 		let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
@@ -582,6 +585,7 @@ impl ChannelDetails {
 			outbound_capacity_msat: balance.outbound_capacity_msat,
 			next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat,
 			next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat,
+			next_splice_out_maximum_sat: balance.next_splice_out_maximum_sat,
 			user_channel_id: context.get_user_id(),
 			confirmations_required: channel.minimum_depth(),
 			confirmations: Some(funding.get_funding_tx_confirmations(best_block_height)),
@@ -621,6 +625,7 @@ impl_writeable_tlv_based!(ChannelDetails, {
 	(20, inbound_capacity_msat, required),
 	(21, next_outbound_htlc_minimum_msat, (default_value, 0)),
 	(22, confirmations_required, option),
+	(23, next_splice_out_maximum_sat, (default_value, u64::from(outbound_capacity_msat.0.unwrap()) / 1000)),
 	(24, force_close_spend_delay, option),
 	(26, is_outbound, required),
 	(28, is_channel_ready, required),
@@ -725,6 +730,7 @@ mod tests {
 			outbound_capacity_msat: 24_300,
 			next_outbound_htlc_limit_msat: 20_000,
 			next_outbound_htlc_minimum_msat: 132,
+			next_splice_out_maximum_sat: 20,
 			inbound_capacity_msat: 42,
 			unspendable_punishment_reserve: Some(8273),
 			confirmations_required: Some(5),

lightning/src/ln/channelmanager.rs

@@ -3887,7 +3887,7 @@ impl<
 		override_config: Option<UserConfig>,
 		trusted_channel_features: Option<TrustedChannelFeatures>,
 	) -> Result<ChannelId, APIError> {
-		if channel_value_satoshis < 1000 {
+		if channel_value_satoshis < crate::ln::channel::MIN_CHANNEL_VALUE_SATOSHIS {
 			return Err(APIError::APIMisuseError {
 				err: format!(
 					"Channel value must be at least 1000 satoshis. It was {channel_value_satoshis}"
@@ -8121,6 +8121,7 @@ impl<
 									outbound_capacity_msat: 0,
 									next_outbound_htlc_limit_msat: 0,
 									next_outbound_htlc_minimum_msat: u64::MAX,
+									next_splice_out_maximum_sat: 0,
 								}
 							});
 							let is_in_range = (balances.next_outbound_htlc_minimum_msat

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/funding.rs

@@ -192,7 +192,7 @@ impl core::fmt::Display for FundingContributionError {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(super) struct PriorContribution {
 	contribution: FundingContribution,
-	/// The holder's balance, used for feerate adjustment.
+	/// The holder's spliceable balance, used for feerate adjustment.
 	///
 	/// This value is captured at [`ChannelManager::splice_channel`] time and may become stale
 	/// if balances change before the contribution is used. Staleness is acceptable here because
@@ -203,12 +203,12 @@ pub(super) struct PriorContribution {
 	///
 	/// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel
 	/// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed
-	holder_balance: Amount,
+	spliceable_balance: Amount,
 }
 
 impl PriorContribution {
-	pub(super) fn new(contribution: FundingContribution, holder_balance: Amount) -> Self {
-		Self { contribution, holder_balance }
+	pub(super) fn new(contribution: FundingContribution, spliceable_balance: Amount) -> Self {
+		Self { contribution, spliceable_balance }
 	}
 }
 
@@ -632,14 +632,14 @@ impl FundingContribution {
 	/// `target_feerate`. If dropping change leaves surplus value, that surplus remains in the
 	/// channel contribution.
 	///
-	/// For input-less contributions, `holder_balance` must be provided to cover the outputs and
+	/// For input-less contributions, `spliceable_balance` must be provided to cover the outputs and
 	/// fees from the channel balance.
 	///
 	/// Returns `None` if the request would require new wallet inputs or cannot accommodate the
 	/// requested feerate.
 	fn amend_without_coin_selection(
 		self, inputs: FundingInputs, outputs: &[TxOut], target_feerate: FeeRate,
-		max_feerate: FeeRate, holder_balance: Amount,
+		max_feerate: FeeRate, spliceable_balance: Amount,
 	) -> Option<Self> {
 		// NOTE: The contribution returned is not guaranteed to be valid. We defer doing so until
 		// `compute_feerate_adjustment`.
@@ -717,7 +717,7 @@ impl FundingContribution {
 		let new_contribution_at_current_feerate =
 			adjust_for_inputs_and_outputs(self, inputs, outputs)?;
 		let mut new_contribution_at_target_feerate = new_contribution_at_current_feerate
-			.at_feerate(target_feerate, holder_balance, true)
+			.at_feerate(target_feerate, spliceable_balance, true)
 			.ok()?;
 		new_contribution_at_target_feerate.max_feerate = max_feerate;
 
@@ -771,7 +771,7 @@ impl FundingContribution {
 	///
 	/// Returns `Err` if the contribution cannot accommodate the target feerate.
 	fn compute_feerate_adjustment(
-		&self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool,
+		&self, target_feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool,
 	) -> Result<(Amount, Option<Amount>), FeeRateAdjustmentError> {
 		if target_feerate < self.feerate {
 			return Err(FeeRateAdjustmentError::FeeRateTooLow {
@@ -864,10 +864,12 @@ impl FundingContribution {
 			let total_cost = target_fee
 				.checked_add(value_removed)
 				.ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?;
-			if total_cost > holder_balance {
+			if total_cost > spliceable_balance {
 				return Err(FeeRateAdjustmentError::FeeBufferInsufficient {
 					source: "channel balance - withdrawal outputs",
-					available: holder_balance.checked_sub(value_removed).unwrap_or(Amount::ZERO),
+					available: spliceable_balance
+						.checked_sub(value_removed)
+						.unwrap_or(Amount::ZERO),
 					required: target_fee,
 				});
 			}
@@ -879,10 +881,10 @@ impl FundingContribution {
 	/// estimate, and feerate. Returns the adjusted contribution, or an error if the feerate
 	/// can't be accommodated.
 	fn at_feerate(
-		mut self, feerate: FeeRate, holder_balance: Amount, is_initiator: bool,
+		mut self, feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool,
 	) -> Result<Self, FeeRateAdjustmentError> {
 		let (new_estimated_fee, new_change) =
-			self.compute_feerate_adjustment(feerate, holder_balance, is_initiator)?;
+			self.compute_feerate_adjustment(feerate, spliceable_balance, is_initiator)?;
 		match new_change {
 			Some(value) => self.change_output.as_mut().unwrap().value = value,
 			None => self.change_output = None,
@@ -899,9 +901,9 @@ impl FundingContribution {
 	/// This adjusts the change output so the acceptor pays their target fee at the target
 	/// feerate.
 	pub(super) fn for_acceptor_at_feerate(
-		self, feerate: FeeRate, holder_balance: Amount,
+		self, feerate: FeeRate, spliceable_balance: Amount,
 	) -> Result<Self, FeeRateAdjustmentError> {
-		self.at_feerate(feerate, holder_balance, false)
+		self.at_feerate(feerate, spliceable_balance, false)
 	}
 
 	/// Adjusts the contribution's change output for the minimum RBF feerate.
@@ -910,9 +912,9 @@ impl FundingContribution {
 	/// below the minimum RBF feerate, this adjusts the change output so the initiator pays fees
 	/// at the minimum RBF feerate.
 	pub(super) fn for_initiator_at_feerate(
-		self, feerate: FeeRate, holder_balance: Amount,
+		self, feerate: FeeRate, spliceable_balance: Amount,
 	) -> Result<Self, FeeRateAdjustmentError> {
-		self.at_feerate(feerate, holder_balance, true)
+		self.at_feerate(feerate, spliceable_balance, true)
 	}
 
 	/// Returns the net value at the given target feerate without mutating `self`.
@@ -921,10 +923,10 @@ impl FundingContribution {
 	/// can't be accommodated) and computes the adjusted net value (returning `Ok` with the value
 	/// accounting for the target feerate).
 	fn net_value_at_feerate(
-		&self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool,
+		&self, target_feerate: FeeRate, spliceable_balance: Amount, is_initiator: bool,
 	) -> Result<SignedAmount, FeeRateAdjustmentError> {
 		let (new_estimated_fee, new_change) =
-			self.compute_feerate_adjustment(target_feerate, holder_balance, is_initiator)?;
+			self.compute_feerate_adjustment(target_feerate, spliceable_balance, is_initiator)?;
 
 		let prev_fee = self
 			.estimated_fee
@@ -952,17 +954,17 @@ impl FundingContribution {
 	/// Returns the net value at the given target feerate without mutating `self`,
 	/// assuming acceptor fee responsibility.
 	pub(super) fn net_value_for_acceptor_at_feerate(
-		&self, target_feerate: FeeRate, holder_balance: Amount,
+		&self, target_feerate: FeeRate, spliceable_balance: Amount,
 	) -> Result<SignedAmount, FeeRateAdjustmentError> {
-		self.net_value_at_feerate(target_feerate, holder_balance, false)
+		self.net_value_at_feerate(target_feerate, spliceable_balance, false)
 	}
 
 	/// Returns the net value at the given target feerate without mutating `self`,
 	/// assuming initiator fee responsibility.
 	pub(super) fn net_value_for_initiator_at_feerate(
-		&self, target_feerate: FeeRate, holder_balance: Amount,
+		&self, target_feerate: FeeRate, spliceable_balance: Amount,
 	) -> Result<SignedAmount, FeeRateAdjustmentError> {
-		self.net_value_at_feerate(target_feerate, holder_balance, true)
+		self.net_value_at_feerate(target_feerate, spliceable_balance, true)
 	}
 
 	/// The net value contributed to a channel by the splice.
@@ -1059,13 +1061,13 @@ impl<State> FundingBuilderInner<State> {
 	fn build_from_prior_contribution(
 		&mut self, contribution: PriorContribution,
 	) -> Result<FundingContribution, FundingContributionError> {
-		let PriorContribution { contribution, holder_balance } = contribution;
+		let PriorContribution { contribution, spliceable_balance } = contribution;
 
 		if self.request_matches_prior(&contribution) {
 			// Same request, but the feerate may have changed. Adjust the prior contribution
 			// to the new feerate if possible.
 			return contribution
-				.for_initiator_at_feerate(self.feerate, holder_balance)
+				.for_initiator_at_feerate(self.feerate, spliceable_balance)
 				.map(|mut adjusted| {
 					adjusted.max_feerate = self.max_feerate;
 					adjusted
@@ -1084,7 +1086,7 @@ impl<State> FundingBuilderInner<State> {
 				&self.outputs,
 				self.feerate,
 				self.max_feerate,
-				holder_balance,
+				spliceable_balance,
 			)
 			.ok_or_else(|| FundingContributionError::MissingCoinSelectionSource);
 	}
@@ -2181,8 +2183,8 @@ mod tests {
 		};
 
 		// Balance of 55,000 sats can't cover outputs (50,000) + target_fee at 50k sat/kwu.
-		let holder_balance = Amount::from_sat(55_000);
-		let result = contribution.for_acceptor_at_feerate(target_feerate, holder_balance);
+		let spliceable_balance = Amount::from_sat(55_000);
+		let result = contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance);
 		assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. })));
 	}
 
@@ -2601,8 +2603,8 @@ mod tests {
 		};
 
 		// Balance of 40,000 sats is less than outputs (50,000) + target_fee.
-		let holder_balance = Amount::from_sat(40_000);
-		let result = contribution.for_acceptor_at_feerate(target_feerate, holder_balance);
+		let spliceable_balance = Amount::from_sat(40_000);
+		let result = contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance);
 		assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. })));
 	}
 
@@ -2627,9 +2629,9 @@ mod tests {
 		};
 
 		// Balance of 100,000 sats is more than outputs (50,000) + target_fee.
-		let holder_balance = Amount::from_sat(100_000);
+		let spliceable_balance = Amount::from_sat(100_000);
 		let contribution =
-			contribution.for_acceptor_at_feerate(target_feerate, holder_balance).unwrap();
+			contribution.for_acceptor_at_feerate(target_feerate, spliceable_balance).unwrap();
 		let expected_target_fee =
 			estimate_transaction_fee(&[], &outputs, None, false, true, target_feerate);
 		assert_eq!(contribution.estimated_fee, expected_target_fee);
@@ -2657,8 +2659,9 @@ mod tests {
 		};
 
 		// Balance of 40,000 sats is less than outputs (50,000) + target_fee.
-		let holder_balance = Amount::from_sat(40_000);
-		let result = contribution.net_value_for_acceptor_at_feerate(target_feerate, holder_balance);
+		let spliceable_balance = Amount::from_sat(40_000);
+		let result =
+			contribution.net_value_for_acceptor_at_feerate(target_feerate, spliceable_balance);
 		assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. })));
 	}
 

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, _) =
@@ -2581,7 +2584,7 @@ fn test_0reserve_no_outputs() {
 	do_test_0reserve_no_outputs_p2a_anchor();
 }
 
-fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>(
+pub(crate) fn setup_0reserve_no_outputs_channels<'a, 'b, 'c, 'd>(
 	nodes: &'a Vec<Node<'b, 'c, 'd>>, channel_value_sat: u64, dust_limit_satoshis: u64,
 ) -> (ChannelId, Transaction) {
 	let node_a_id = nodes[0].node.get_our_node_id();

lightning/src/ln/payment_tests.rs

@@ -5043,7 +5043,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,