Remove wallet argument from FundingTemplate::splice_out

It does not require coin selection, so the wallet argument is not
necessary.

Author: wpaulino

fuzz/src/chanmon_consistency.rs

@@ -1504,7 +1504,6 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 	                  counterparty_node_id: &PublicKey,
 	                  channel_id: &ChannelId,
 	                  wallet: &TestWalletSource,
-	                  logger: Arc<dyn Logger + MaybeSend + MaybeSync>,
 	                  funding_feerate_sat_per_kw: FeeRate| {
 		// We conditionally splice out `MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS` only when the node
 		// has double the balance required to send a payment upon a `0xff` byte. We do this to
@@ -1524,12 +1523,7 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 				value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS),
 				script_pubkey: wallet.get_change_script().unwrap(),
 			}];
-			funding_template.splice_out_sync(
-				outputs,
-				feerate,
-				FeeRate::MAX,
-				&WalletSync::new(wallet, logger.clone()),
-			)
+			funding_template.splice_out(outputs, feerate, FeeRate::MAX)
 		});
 	};
 
@@ -2450,30 +2444,26 @@ pub fn do_test<Out: Output + MaybeSend + MaybeSync>(data: &[u8], out: Out) {
 			0xa4 => {
 				let cp_node_id = nodes[1].get_our_node_id();
 				let wallet = &wallets[0];
-				let logger = Arc::clone(&loggers[0]);
 				let feerate_sat_per_kw = fee_estimators[0].feerate_sat_per_kw();
-				splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw);
 			},
 			0xa5 => {
 				let cp_node_id = nodes[0].get_our_node_id();
 				let wallet = &wallets[1];
-				let logger = Arc::clone(&loggers[1]);
 				let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw();
-				splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw);
 			},
 			0xa6 => {
 				let cp_node_id = nodes[2].get_our_node_id();
 				let wallet = &wallets[1];
-				let logger = Arc::clone(&loggers[1]);
 				let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw();
-				splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw);
 			},
 			0xa7 => {
 				let cp_node_id = nodes[1].get_our_node_id();
 				let wallet = &wallets[2];
-				let logger = Arc::clone(&loggers[2]);
 				let feerate_sat_per_kw = fee_estimators[2].feerate_sat_per_kw();
-				splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw);
+				splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw);
 			},
 
 			// Sync node by 1 block to cover confirmation of a transaction.

fuzz/src/full_stack.rs

@@ -1083,13 +1083,9 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger + MaybeSend + MaybeSync>
 						value: Amount::from_sat(splice_out_sats),
 						script_pubkey: wallet.get_change_script().unwrap(),
 					}];
-					let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger));
-					if let Ok(contribution) = funding_template.splice_out_sync(
-						outputs,
-						feerate,
-						FeeRate::MAX,
-						&wallet_sync,
-					) {
+					if let Ok(contribution) =
+						funding_template.splice_out(outputs, feerate, FeeRate::MAX)
+					{
 						let _ = channelmanager.funding_contributed(
 							&chan_id,
 							&counterparty,

lightning/src/ln/funding.rs

@@ -206,10 +206,12 @@ impl PriorContribution {
 /// For a fresh splice (no pending splice to replace), build a new contribution using one of
 /// the splice methods:
 /// - [`FundingTemplate::splice_in_sync`] to add funds to the channel
-/// - [`FundingTemplate::splice_out_sync`] to remove funds from the channel
+/// - [`FundingTemplate::splice_out`] to remove funds from the channel
 /// - [`FundingTemplate::splice_in_and_out_sync`] to do both
 ///
-/// These perform coin selection and require `min_feerate` and `max_feerate` parameters.
+/// These require `min_feerate` and `max_feerate` parameters. The splice-in variants perform
+/// coin selection when wallet inputs are needed, while splice-out spends only from the channel
+/// balance.
 ///
 /// # Replace By Fee (RBF)
 ///
@@ -285,31 +287,13 @@ macro_rules! build_funding_contribution {
 		let max_feerate: FeeRate = $max_feerate;
 		let force_coin_selection: bool = $force_coin_selection;
 
-		if feerate > max_feerate {
-			return Err(FundingContributionError::FeeRateExceedsMaximum { feerate, max_feerate });
-		}
-
-		if let Some(min_rbf_feerate) = min_rbf_feerate {
-			if feerate < min_rbf_feerate {
-				return Err(FundingContributionError::FeeRateBelowRbfMinimum { feerate, min_rbf_feerate });
-			}
-		}
-
-		// Validate user-provided amounts are within MAX_MONEY before coin selection to
-		// ensure FundingContribution::net_value() arithmetic cannot overflow. With all
-		// amounts bounded by MAX_MONEY (~2.1e15 sat), the worst-case net_value()
-		// computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18).
-		if value_added > Amount::MAX_MONEY {
-			return Err(FundingContributionError::InvalidSpliceValue);
-		}
-
-		let mut value_removed = Amount::ZERO;
-		for txout in outputs.iter() {
-			value_removed = match value_removed.checked_add(txout.value) {
-				Some(sum) if sum <= Amount::MAX_MONEY => sum,
-				_ => return Err(FundingContributionError::InvalidSpliceValue),
-			};
-		}
+		let value_removed = validate_funding_contribution_params(
+			value_added,
+			&outputs,
+			min_rbf_feerate,
+			feerate,
+			max_feerate,
+		)?;
 
 		let is_splice = shared_input.is_some();
 
@@ -348,25 +332,52 @@ macro_rules! build_funding_contribution {
 
 		let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection;
 
-		// The caller creating a FundingContribution is always the initiator for fee estimation
-		// purposes — this is conservative, overestimating rather than underestimating fees if
-		// the node ends up as the acceptor.
-		let estimated_fee = estimate_transaction_fee(&inputs, &outputs, change_output.as_ref(), true, is_splice, feerate);
-		debug_assert!(estimated_fee <= Amount::MAX_MONEY);
-
-		let contribution = FundingContribution {
+		Ok(FundingContribution::new(
 			value_added,
-			estimated_fee,
-			inputs,
 			outputs,
+			inputs,
 			change_output,
 			feerate,
 			max_feerate,
 			is_splice,
+		))
+	}};
+}
+
+fn validate_funding_contribution_params(
+	value_added: Amount, outputs: &[TxOut], min_rbf_feerate: Option<FeeRate>, feerate: FeeRate,
+	max_feerate: FeeRate,
+) -> Result<Amount, FundingContributionError> {
+	if feerate > max_feerate {
+		return Err(FundingContributionError::FeeRateExceedsMaximum { feerate, max_feerate });
+	}
+
+	if let Some(min_rbf_feerate) = min_rbf_feerate {
+		if feerate < min_rbf_feerate {
+			return Err(FundingContributionError::FeeRateBelowRbfMinimum {
+				feerate,
+				min_rbf_feerate,
+			});
+		}
+	}
+
+	// Validate user-provided amounts are within MAX_MONEY before coin selection to
+	// ensure FundingContribution::net_value() arithmetic cannot overflow. With all
+	// amounts bounded by MAX_MONEY (~2.1e15 sat), the worst-case net_value()
+	// computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18).
+	if value_added > Amount::MAX_MONEY {
+		return Err(FundingContributionError::InvalidSpliceValue);
+	}
+
+	let mut value_removed = Amount::ZERO;
+	for txout in outputs.iter() {
+		value_removed = match value_removed.checked_add(txout.value) {
+			Some(sum) if sum <= Amount::MAX_MONEY => sum,
+			_ => return Err(FundingContributionError::InvalidSpliceValue),
 		};
+	}
 
-		Ok(contribution)
-	}};
+	Ok(value_removed)
 }
 
 impl FundingTemplate {
@@ -410,44 +421,37 @@ impl FundingTemplate {
 		)
 	}
 
-	/// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to
-	/// perform coin selection.
+	/// Creates a [`FundingContribution`] for removing funds from a channel.
+	///
+	/// Fees are paid from the channel balance, so this does not perform coin selection or spend
+	/// wallet inputs.
 	///
 	/// `outputs` are the complete set of withdrawal outputs for this contribution. When
 	/// replacing a prior contribution via RBF, use [`FundingTemplate::prior_contribution`] to
 	/// inspect the prior parameters. To keep existing withdrawals and add new ones, include the
 	/// prior's outputs: combine [`FundingContribution::outputs`] with the new outputs.
-	pub async fn splice_out<W: CoinSelectionSource + MaybeSend>(
-		self, outputs: Vec<TxOut>, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W,
-	) -> Result<FundingContribution, FundingContributionError> {
-		if outputs.is_empty() {
-			return Err(FundingContributionError::InvalidSpliceValue);
-		}
-		let FundingTemplate { shared_input, min_rbf_feerate, .. } = self;
-		build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, false, wallet, await)
-	}
-
-	/// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to
-	/// perform coin selection.
-	///
-	/// See [`FundingTemplate::splice_out`] for details.
-	pub fn splice_out_sync<W: CoinSelectionSourceSync>(
-		self, outputs: Vec<TxOut>, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W,
+	pub fn splice_out(
+		self, outputs: Vec<TxOut>, min_feerate: FeeRate, max_feerate: FeeRate,
 	) -> Result<FundingContribution, FundingContributionError> {
 		if outputs.is_empty() {
 			return Err(FundingContributionError::InvalidSpliceValue);
 		}
-		let FundingTemplate { shared_input, min_rbf_feerate, .. } = self;
-		build_funding_contribution!(
+		validate_funding_contribution_params(
+			Amount::ZERO,
+			&outputs,
+			self.min_rbf_feerate,
+			min_feerate,
+			max_feerate,
+		)?;
+		Ok(FundingContribution::new(
 			Amount::ZERO,
 			outputs,
-			shared_input,
-			min_rbf_feerate,
+			vec![],
+			None,
 			min_feerate,
 			max_feerate,
-			false,
-			wallet,
-		)
+			self.shared_input.is_some(),
+		))
 	}
 
 	/// Creates a [`FundingContribution`] for both adding and removing funds from a channel using
@@ -708,6 +712,35 @@ impl_writeable_tlv_based!(FundingContribution, {
 });
 
 impl FundingContribution {
+	fn new(
+		value_added: Amount, outputs: Vec<TxOut>, inputs: Vec<FundingTxInput>,
+		change_output: Option<TxOut>, feerate: FeeRate, max_feerate: FeeRate, is_splice: bool,
+	) -> Self {
+		// The caller creating a FundingContribution is always the initiator for fee estimation
+		// purposes — this is conservative, overestimating rather than underestimating fees if the
+		// node ends up as the acceptor.
+		let estimated_fee = estimate_transaction_fee(
+			&inputs,
+			&outputs,
+			change_output.as_ref(),
+			true,
+			is_splice,
+			feerate,
+		);
+		debug_assert!(estimated_fee <= Amount::MAX_MONEY);
+
+		Self {
+			value_added,
+			estimated_fee,
+			inputs,
+			outputs,
+			change_output,
+			feerate,
+			max_feerate,
+			is_splice,
+		}
+	}
+
 	pub(super) fn feerate(&self) -> FeeRate {
 		self.feerate
 	}
@@ -1440,17 +1473,17 @@ mod tests {
 			));
 		}
 
-		// splice_out_sync with single output value > MAX_MONEY
+		// splice_out with single output value > MAX_MONEY
 		{
 			let template = FundingTemplate::new(None, None, None);
 			let outputs = vec![funding_output_sats(over_max.to_sat())];
 			assert!(matches!(
-				template.splice_out_sync(outputs, feerate, feerate, UnreachableWallet),
+				template.splice_out(outputs, feerate, feerate),
 				Err(FundingContributionError::InvalidSpliceValue),
 			));
 		}
 
-		// splice_out_sync with multiple outputs summing > MAX_MONEY
+		// splice_out with multiple outputs summing > MAX_MONEY
 		{
 			let template = FundingTemplate::new(None, None, None);
 			let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1);
@@ -1459,7 +1492,7 @@ mod tests {
 				funding_output_sats(half_over.to_sat()),
 			];
 			assert!(matches!(
-				template.splice_out_sync(outputs, feerate, feerate, UnreachableWallet),
+				template.splice_out(outputs, feerate, feerate),
 				Err(FundingContributionError::InvalidSpliceValue),
 			));
 		}
@@ -2454,23 +2487,22 @@ mod tests {
 	}
 
 	#[test]
-	fn test_splice_out_sync_skips_coin_selection_during_rbf() {
-		// When splice_out_sync is called on a template with min_rbf_feerate set (user
-		// choosing a fresh splice-out instead of rbf_sync), coin selection should NOT run.
-		// Fees come from the channel balance.
+	fn test_splice_out_skips_coin_selection_during_rbf() {
+		// When splice_out is called on a template with min_rbf_feerate set (user
+		// choosing a fresh splice-out instead of rbf_sync), fees still come from the
+		// channel balance rather than wallet inputs.
 		let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000);
 		let feerate = FeeRate::from_sat_per_kwu(5000);
 		let withdrawal = funding_output_sats(20_000);
 
 		let template =
 			FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None);
 
-		// UnreachableWallet panics if coin selection runs — verifying it is skipped.
-		let contribution = template
-			.splice_out_sync(vec![withdrawal.clone()], feerate, FeeRate::MAX, UnreachableWallet)
-			.unwrap();
+		let contribution =
+			template.splice_out(vec![withdrawal.clone()], feerate, FeeRate::MAX).unwrap();
 		assert_eq!(contribution.value_added, Amount::ZERO);
 		assert!(contribution.inputs.is_empty());
+		assert!(contribution.change_output.is_none());
 		assert_eq!(contribution.outputs, vec![withdrawal]);
 	}
 }

lightning/src/ln/splicing_tests.rs

@@ -271,9 +271,7 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>(
 	let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64);
 	let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap();
 	let feerate = funding_template.min_rbf_feerate().unwrap_or(floor_feerate);
-	let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger);
-	let funding_contribution =
-		funding_template.splice_out_sync(outputs, feerate, FeeRate::MAX, &wallet).unwrap();
+	let funding_contribution = funding_template.splice_out(outputs, feerate, FeeRate::MAX).unwrap();
 	match initiator.node.funding_contributed(
 		&channel_id,
 		&node_id_acceptor,
@@ -1367,9 +1365,8 @@ fn fails_initiating_concurrent_splices(reconnect: bool) {
 	let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64);
 
 	let funding_template = nodes[0].node.splice_channel(&channel_id, &node_1_id).unwrap();
-	let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
 	let funding_contribution =
-		funding_template.splice_out_sync(outputs.clone(), feerate, FeeRate::MAX, &wallet).unwrap();
+		funding_template.splice_out(outputs.clone(), feerate, FeeRate::MAX).unwrap();
 	nodes[0]
 		.node
 		.funding_contributed(&channel_id, &node_1_id, funding_contribution.clone(), None)
@@ -6336,9 +6333,7 @@ fn test_splice_revalidation_at_quiescence() {
 
 	let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64);
 	let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap();
-	let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger);
-	let contribution =
-		funding_template.splice_out_sync(outputs, feerate, FeeRate::MAX, &wallet).unwrap();
+	let contribution = funding_template.splice_out(outputs, feerate, FeeRate::MAX).unwrap();
 
 	nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap();
 	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty(), "stfu should be delayed");