Remove wallet argument from FundingTemplate::splice_out
What changed, and why it matters
This commit is a routine API cleanup, not a security fix. It removes an unnecessary wallet/coin-selection argument from the splice_out function because splicing funds out of a Lightning channel only spends from the channel balance and never needs to select wallet inputs. The change refactors shared validation into a helper and makes the function synchronous, but it does not patch any vulnerability and the commit message does not claim it does.
No security action required. Treat as normal refactoring/API cleanup during dependency update review.
Security signals we found
API surface reduction by removing wallet argument from splice_out
Refactoring of shared validation into helper function
No new bounds checks or memory safety fixes introduced
No mention of vulnerability, CVE, or security issue in commit message
Evidence from the diff
The patch changes FundingTemplate::splice_out_sync (and the underlying async splice_out) so they no longer take a CoinSelectionSource/W argument. It extracts validation logic into validate_funding_contribution_params and adds a FundingContribution::new constructor. splice_out now directly returns a FundingContribution with empty inputs and no change output, since fees are paid from the channel balance. Fuzz and test call sites are updated accordingly. No security-relevant bug is corrected; the change is architectural simplification.
Changed components
lightning/src/ln/funding.rslightning/src/ln/splicing_tests.rsfuzz/src/chanmon_consistency.rsfuzz/src/full_stack.rsInspect captured patch +112 / −109
diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index f20f93c..07aae78 100644
--- a/fuzz/src/chanmon_consistency.rs
+++ b/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.
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index c1d7982..f300ded 100644
--- a/fuzz/src/full_stack.rs
+++ b/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,
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index 0a5fb64..470e8bc 100644
--- a/lightning/src/ln/funding.rs
+++ b/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)
///
@@ -287,31 +289,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();
@@ -350,25 +334,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 {
@@ -422,54 +433,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,
+ 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,
- shared_input,
- min_rbf_feerate,
+ &outputs,
+ self.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,
- ) -> Result<FundingContribution, FundingContributionError> {
- if outputs.is_empty() {
- return Err(FundingContributionError::InvalidSpliceValue);
- }
- let FundingTemplate { shared_input, min_rbf_feerate, .. } = self;
- build_funding_contribution!(
+ )?;
+ 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
@@ -760,6 +754,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
}
@@ -1492,17 +1515,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);
@@ -1511,7 +1534,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),
));
}
@@ -2506,7 +2529,7 @@ mod tests {
}
#[test]
- fn test_splice_out_sync_skips_coin_selection_during_rbf() {
+ fn test_splice_out_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.
@@ -2517,12 +2540,11 @@ mod tests {
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]);
}
}
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 9adccd1..5492921 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/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,
@@ -1370,9 +1368,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)
@@ -6420,9 +6417,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");
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.