What changed, and why it matters
This commit adds 'splice-out' support to the Lightning Dev Kit, allowing users to remove funds from an existing channel while keeping the channel open. The change introduces new code paths that handle negative contributions (removing funds) and adds checks to ensure the user cannot remove more than their channel balance after accounting for fees and reserve requirements. It is a feature addition, not a documented security fix, but it touches sensitive financial-validation logic.
Review the splice-out validation logic for off-by-one, rounding, and fee-estimation edge cases; complete the TODO enforcing the channel reserve minimum after splice-out; and add fuzz/negative tests for maliciously crafted output sets and feerates.
Security signals we found
New financial-validation function added: check_splice_contribution_sufficient
Splice-out allows negative contribution values, a historically sensitive code path
Fee estimation now includes user-controlled outputs, increasing attack surface for fee manipulation
Remaining TODO: 'Check that channel balance does not go below the channel reserve'
Error handling changed from generic 'Insufficient inputs for splicing' to more specific messages
No CVE, advisory, or vendor security disclosure present in commit or references
Evidence from the diff
The patch extends SpliceContribution with a SpliceOut variant, updates fee estimation to account for user-provided outputs, and adds check_splice_contribution_sufficient to validate that splice-out amounts plus estimated fees do not exceed the local channel balance. It also adjusts the funding contribution for splice-out by subtracting fees from the channel balance and propagates user funding outputs through FundingNegotiationContext. A TODO remains to enforce the channel reserve for post-splice balance.
Changed components
lightning/src/ln/channel.rslightning/src/ln/funding.rslightning/src/ln/interactivetxs.rslightning/src/ln/splicing_tests.rsInspect captured patch +183 / −74
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e23001e..7c1b11d 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -24,9 +24,9 @@ use bitcoin::hashes::Hash;
use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1};
use bitcoin::secp256k1::{PublicKey, SecretKey};
-#[cfg(splicing)]
-use bitcoin::Sequence;
use bitcoin::{secp256k1, sighash, TxIn};
+#[cfg(splicing)]
+use bitcoin::{FeeRate, Sequence};
use crate::chain::chaininterface::{
fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator,
@@ -5879,18 +5879,60 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos
cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis))
}
+#[cfg(splicing)]
+fn check_splice_contribution_sufficient(
+ channel_balance: Amount, contribution: &SpliceContribution, is_initiator: bool,
+ funding_feerate: FeeRate,
+) -> Result<Amount, ChannelError> {
+ let contribution_amount = contribution.value();
+ if contribution_amount < SignedAmount::ZERO {
+ let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee(
+ contribution.inputs(),
+ contribution.outputs(),
+ is_initiator,
+ true, // is_splice
+ funding_feerate.to_sat_per_kwu() as u32,
+ ));
+
+ if channel_balance >= contribution_amount.unsigned_abs() + estimated_fee {
+ Ok(estimated_fee)
+ } else {
+ Err(ChannelError::Warn(format!(
+ "Available channel balance {} is lower than needed for splicing out {}, considering fees of {}",
+ channel_balance, contribution_amount.unsigned_abs(), estimated_fee,
+ )))
+ }
+ } else {
+ check_v2_funding_inputs_sufficient(
+ contribution_amount.to_sat(),
+ contribution.inputs(),
+ is_initiator,
+ true,
+ funding_feerate.to_sat_per_kwu() as u32,
+ )
+ .map(Amount::from_sat)
+ }
+}
+
/// Estimate our part of the fee of the new funding transaction.
#[allow(dead_code)] // TODO(dual_funding): TODO(splicing): Remove allow once used.
#[rustfmt::skip]
fn estimate_v2_funding_transaction_fee(
- funding_inputs: &[FundingTxInput], is_initiator: bool, is_splice: bool,
+ funding_inputs: &[FundingTxInput], outputs: &[TxOut], is_initiator: bool, is_splice: bool,
funding_feerate_sat_per_1000_weight: u32,
) -> u64 {
- let mut weight: u64 = funding_inputs
+ let input_weight: u64 = funding_inputs
.iter()
.map(|input| BASE_INPUT_WEIGHT.saturating_add(input.utxo.satisfaction_weight))
.fold(0, |total_weight, input_weight| total_weight.saturating_add(input_weight));
+ let output_weight: u64 = outputs
+ .iter()
+ .map(|txout| txout.weight().to_wu())
+ .fold(0, |total_weight, output_weight| total_weight.saturating_add(output_weight));
+
+ let mut weight = input_weight.saturating_add(output_weight);
+
// The initiator pays for all common fields and the shared output in the funding transaction.
if is_initiator {
weight = weight
@@ -5927,7 +5969,7 @@ fn check_v2_funding_inputs_sufficient(
is_splice: bool, funding_feerate_sat_per_1000_weight: u32,
) -> Result<u64, ChannelError> {
let estimated_fee = estimate_v2_funding_transaction_fee(
- funding_inputs, is_initiator, is_splice, funding_feerate_sat_per_1000_weight,
+ funding_inputs, &[], is_initiator, is_splice, funding_feerate_sat_per_1000_weight,
);
let mut total_input_sats = 0u64;
@@ -5975,6 +6017,9 @@ pub(super) struct FundingNegotiationContext {
/// The funding inputs we will be contributing to the channel.
#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
pub our_funding_inputs: Vec<FundingTxInput>,
+ /// The funding outputs we will be contributing to the channel.
+ #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
+ pub our_funding_outputs: Vec<TxOut>,
/// The change output script. This will be used if needed or -- if not set -- generated using
/// `SignerProvider::get_destination_script`.
#[allow(dead_code)] // TODO(splicing): Remove once splicing is enabled.
@@ -6004,10 +6049,8 @@ impl FundingNegotiationContext {
debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_)));
}
- // Add output for funding tx
// Note: For the error case when the inputs are insufficient, it will be handled after
// the `calculate_change_output_value` call below
- let mut funding_outputs = Vec::new();
let shared_funding_output = TxOut {
value: Amount::from_sat(funding.get_value_satoshis()),
@@ -6015,34 +6058,37 @@ impl FundingNegotiationContext {
};
// Optionally add change output
- if self.our_funding_contribution > SignedAmount::ZERO {
- let change_value_opt = calculate_change_output_value(
+ let change_value_opt = if self.our_funding_contribution > SignedAmount::ZERO {
+ calculate_change_output_value(
&self,
self.shared_funding_input.is_some(),
&shared_funding_output.script_pubkey,
- &funding_outputs,
context.holder_dust_limit_satoshis,
- )?;
- if let Some(change_value) = change_value_opt {
- let change_script = if let Some(script) = self.change_script {
- script
- } else {
- signer_provider
- .get_destination_script(context.channel_keys_id)
- .map_err(|_err| AbortReason::InternalError("Error getting change script"))?
- };
- let mut change_output =
- TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script };
- let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu();
- let change_output_fee =
- fee_for_weight(self.funding_feerate_sat_per_1000_weight, change_output_weight);
- let change_value_decreased_with_fee =
- change_value.saturating_sub(change_output_fee);
- // Check dust limit again
- if change_value_decreased_with_fee > context.holder_dust_limit_satoshis {
- change_output.value = Amount::from_sat(change_value_decreased_with_fee);
- funding_outputs.push(change_output);
- }
+ )?
+ } else {
+ None
+ };
+
+ let mut funding_outputs = self.our_funding_outputs;
+
+ if let Some(change_value) = change_value_opt {
+ let change_script = if let Some(script) = self.change_script {
+ script
+ } else {
+ signer_provider
+ .get_destination_script(context.channel_keys_id)
+ .map_err(|_err| AbortReason::InternalError("Error getting change script"))?
+ };
+ let mut change_output =
+ TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script };
+ let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu();
+ let change_output_fee =
+ fee_for_weight(self.funding_feerate_sat_per_1000_weight, change_output_weight);
+ let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee);
+ // Check dust limit again
+ if change_value_decreased_with_fee > context.holder_dust_limit_satoshis {
+ change_output.value = Amount::from_sat(change_value_decreased_with_fee);
+ funding_outputs.push(change_output);
}
}
@@ -10630,47 +10676,78 @@ where
// TODO(splicing): check for quiescence
let our_funding_contribution = contribution.value();
+ if our_funding_contribution == SignedAmount::ZERO {
+ return Err(APIError::APIMisuseError {
+ err: format!(
+ "Channel {} cannot be spliced; contribution cannot be zero",
+ self.context.channel_id(),
+ ),
+ });
+ }
+
if our_funding_contribution > SignedAmount::MAX_MONEY {
return Err(APIError::APIMisuseError {
err: format!(
- "Channel {} cannot be spliced; contribution exceeds total bitcoin supply: {}",
+ "Channel {} cannot be spliced in; contribution exceeds total bitcoin supply: {}",
self.context.channel_id(),
our_funding_contribution,
),
});
}
- if our_funding_contribution < SignedAmount::ZERO {
+ if our_funding_contribution < -SignedAmount::MAX_MONEY {
return Err(APIError::APIMisuseError {
err: format!(
- "TODO(splicing): Splice-out not supported, only splice in; channel ID {}, contribution {}",
- self.context.channel_id(), our_funding_contribution,
- ),
+ "Channel {} cannot be spliced out; contribution exhausts total bitcoin supply: {}",
+ self.context.channel_id(),
+ our_funding_contribution,
+ ),
});
}
- // TODO(splicing): Once splice-out is supported, check that channel balance does not go below 0
- // (or below channel reserve)
-
// Note: post-splice channel value is not yet known at this point, counterparty contribution is not known
// (Cannot test for miminum required post-splice channel value)
- // Check that inputs are sufficient to cover our contribution.
- let _fee = check_v2_funding_inputs_sufficient(
- our_funding_contribution.to_sat(),
- contribution.inputs(),
- true,
- true,
- funding_feerate_per_kw,
+ let channel_balance = Amount::from_sat(self.funding.get_value_to_self_msat() / 1000);
+ let fees = check_splice_contribution_sufficient(
+ channel_balance,
+ &contribution,
+ true, // is_initiator
+ FeeRate::from_sat_per_kwu(funding_feerate_per_kw as u64),
)
- .map_err(|err| APIError::APIMisuseError {
- err: format!(
- "Insufficient inputs for splicing; channel ID {}, err {}",
- self.context.channel_id(),
- err,
- ),
+ .map_err(|e| {
+ let splice_type = if our_funding_contribution < SignedAmount::ZERO {
+ "spliced out"
+ } else {
+ "spliced in"
+ };
+ APIError::APIMisuseError {
+ err: format!(
+ "Channel {} cannot be {}; {}",
+ self.context.channel_id(),
+ splice_type,
+ e,
+ ),
+ }
})?;
+ // Fees for splice-out are paid from the channel balance whereas fees for splice-in are paid
+ // by the funding inputs.
+ let adjusted_funding_contribution = if our_funding_contribution < SignedAmount::ZERO {
+ let adjusted_funding_contribution = our_funding_contribution
+ - fees.to_signed().expect("fees should never exceed Amount::MAX_MONEY");
+
+ // TODO(splicing): Check that channel balance does not go below the channel reserve
+ let _post_channel_balance = AddSigned::checked_add_signed(
+ channel_balance.to_sat(),
+ adjusted_funding_contribution.to_sat(),
+ );
+
+ adjusted_funding_contribution
+ } else {
+ our_funding_contribution
+ };
+
for FundingTxInput { utxo, prevtx, .. } in contribution.inputs().iter() {
const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
channel_id: ChannelId([0; 32]),
@@ -10693,14 +10770,15 @@ where
}
let prev_funding_input = self.funding.to_splice_funding_input();
- let (our_funding_inputs, change_script) = contribution.into_tx_parts();
+ let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts();
let funding_negotiation_context = FundingNegotiationContext {
is_initiator: true,
- our_funding_contribution,
+ our_funding_contribution: adjusted_funding_contribution,
funding_tx_locktime: LockTime::from_consensus(locktime),
funding_feerate_sat_per_1000_weight: funding_feerate_per_kw,
shared_funding_input: Some(prev_funding_input),
our_funding_inputs,
+ our_funding_outputs,
change_script,
};
@@ -10716,7 +10794,7 @@ where
Ok(msgs::SpliceInit {
channel_id: self.context.channel_id,
- funding_contribution_satoshis: our_funding_contribution.to_sat(),
+ funding_contribution_satoshis: adjusted_funding_contribution.to_sat(),
funding_feerate_per_kw,
locktime,
funding_pubkey,
@@ -10825,6 +10903,7 @@ where
funding_feerate_sat_per_1000_weight: msg.funding_feerate_per_kw,
shared_funding_input: Some(prev_funding_input),
our_funding_inputs: Vec::new(),
+ our_funding_outputs: Vec::new(),
change_script: None,
};
@@ -12523,6 +12602,7 @@ where
funding_feerate_sat_per_1000_weight,
shared_funding_input: None,
our_funding_inputs: funding_inputs,
+ our_funding_outputs: Vec::new(),
change_script: None,
};
let chan = Self {
@@ -12677,6 +12757,7 @@ where
funding_feerate_sat_per_1000_weight: msg.funding_feerate_sat_per_1000_weight,
shared_funding_input: None,
our_funding_inputs: our_funding_inputs.clone(),
+ our_funding_outputs: Vec::new(),
change_script: None,
};
let shared_funding_output = TxOut {
@@ -12702,7 +12783,7 @@ where
inputs_to_contribute,
shared_funding_input: None,
shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_contribution_sats),
- outputs_to_contribute: Vec::new(),
+ outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(),
}
).map_err(|err| {
let reason = ClosureReason::ProcessingError { err: err.to_string() };
@@ -15870,43 +15951,43 @@ mod tests {
// 2 inputs, initiator, 2000 sat/kw feerate
assert_eq!(
- estimate_v2_funding_transaction_fee(&two_inputs, true, false, 2000),
+ estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 2000),
1520,
);
// higher feerate
assert_eq!(
- estimate_v2_funding_transaction_fee(&two_inputs, true, false, 3000),
+ estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 3000),
2280,
);
// only 1 input
assert_eq!(
- estimate_v2_funding_transaction_fee(&one_input, true, false, 2000),
+ estimate_v2_funding_transaction_fee(&one_input, &[], true, false, 2000),
974,
);
// 0 inputs
assert_eq!(
- estimate_v2_funding_transaction_fee(&[], true, false, 2000),
+ estimate_v2_funding_transaction_fee(&[], &[], true, false, 2000),
428,
);
// not initiator
assert_eq!(
- estimate_v2_funding_transaction_fee(&[], false, false, 2000),
+ estimate_v2_funding_transaction_fee(&[], &[], false, false, 2000),
0,
);
// splice initiator
assert_eq!(
- estimate_v2_funding_transaction_fee(&one_input, true, true, 2000),
+ estimate_v2_funding_transaction_fee(&one_input, &[], true, true, 2000),
1746,
);
// splice acceptor
assert_eq!(
- estimate_v2_funding_transaction_fee(&one_input, false, true, 2000),
+ estimate_v2_funding_transaction_fee(&one_input, &[], false, true, 2000),
546,
);
}
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index 21bc42b..8a51741 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -10,7 +10,7 @@
//! Types pertaining to funding channels.
#[cfg(splicing)]
-use bitcoin::{Amount, ScriptBuf, SignedAmount};
+use bitcoin::{Amount, ScriptBuf, SignedAmount, TxOut};
use bitcoin::{Script, Sequence, Transaction, Weight};
use crate::events::bump_transaction::{Utxo, EMPTY_SCRIPT_SIG_WEIGHT};
@@ -32,6 +32,12 @@ pub enum SpliceContribution {
/// generated using [`SignerProvider::get_destination_script`].
change_script: Option<ScriptBuf>,
},
+ /// When funds are removed from a channel.
+ SpliceOut {
+ /// The outputs to include in the splice's funding transaction. The total value of all
+ /// outputs will be the amount that is removed.
+ outputs: Vec<TxOut>,
+ },
}
#[cfg(splicing)]
@@ -41,18 +47,38 @@ impl SpliceContribution {
SpliceContribution::SpliceIn { value, .. } => {
value.to_signed().unwrap_or(SignedAmount::MAX)
},
+ SpliceContribution::SpliceOut { outputs } => {
+ let value_removed = outputs
+ .iter()
+ .map(|txout| txout.value)
+ .sum::<Amount>()
+ .to_signed()
+ .unwrap_or(SignedAmount::MAX);
+ -value_removed
+ },
}
}
pub(super) fn inputs(&self) -> &[FundingTxInput] {
match self {
SpliceContribution::SpliceIn { inputs, .. } => &inputs[..],
+ SpliceContribution::SpliceOut { .. } => &[],
}
}
- pub(super) fn into_tx_parts(self) -> (Vec<FundingTxInput>, Option<ScriptBuf>) {
+ pub(super) fn outputs(&self) -> &[TxOut] {
match self {
- SpliceContribution::SpliceIn { inputs, change_script, .. } => (inputs, change_script),
+ SpliceContribution::SpliceIn { .. } => &[],
+ SpliceContribution::SpliceOut { outputs } => &outputs[..],
+ }
+ }
+
+ pub(super) fn into_tx_parts(self) -> (Vec<FundingTxInput>, Vec<TxOut>, Option<ScriptBuf>) {
+ match self {
+ SpliceContribution::SpliceIn { inputs, change_script, .. } => {
+ (inputs, vec![], change_script)
+ },
+ SpliceContribution::SpliceOut { outputs } => (vec![], outputs, None),
}
}
}
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index db6b2ba..216addb 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -2069,7 +2069,7 @@ impl InteractiveTxConstructor {
/// - `change_output_dust_limit` - The dust limit (in sats) to consider.
pub(super) fn calculate_change_output_value(
context: &FundingNegotiationContext, is_splice: bool, shared_output_funding_script: &ScriptBuf,
- funding_outputs: &Vec<TxOut>, change_output_dust_limit: u64,
+ change_output_dust_limit: u64,
) -> Result<Option<u64>, AbortReason> {
assert!(context.our_funding_contribution > SignedAmount::ZERO);
let our_funding_contribution_satoshis = context.our_funding_contribution.to_sat() as u64;
@@ -2083,6 +2083,7 @@ pub(super) fn calculate_change_output_value(
our_funding_inputs_weight = our_funding_inputs_weight.saturating_add(weight);
}
+ let funding_outputs = &context.our_funding_outputs;
let total_output_satoshis =
funding_outputs.iter().fold(0u64, |total, out| total.saturating_add(out.value.to_sat()));
let our_funding_outputs_weight = funding_outputs.iter().fold(0u64, |weight, out| {
@@ -3177,17 +3178,18 @@ mod tests {
funding_feerate_sat_per_1000_weight,
shared_funding_input: None,
our_funding_inputs: inputs,
+ our_funding_outputs: outputs,
change_script: None,
};
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 300),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
Ok(Some(gross_change - fees - common_fees)),
);
// There is leftover for change, without common fees
let context = FundingNegotiationContext { is_initiator: false, ..context };
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 300),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
Ok(Some(gross_change - fees)),
);
@@ -3198,7 +3200,7 @@ mod tests {
..context
};
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 300),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
Err(AbortReason::InsufficientFees),
);
@@ -3209,7 +3211,7 @@ mod tests {
..context
};
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 300),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
Ok(None),
);
@@ -3220,7 +3222,7 @@ mod tests {
..context
};
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 100),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 100),
Ok(Some(262)),
);
@@ -3232,7 +3234,7 @@ mod tests {
..context
};
assert_eq!(
- calculate_change_output_value(&context, false, &ScriptBuf::new(), &outputs, 300),
+ calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
Ok(Some(4060)),
);
}
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 6061632..2445a2d 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -319,7 +319,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() {
);
match res {
Err(APIError::APIMisuseError { err }) => {
- assert!(err.contains("Insufficient inputs for splicing"))
+ assert!(err.contains("Need more inputs"))
},
_ => panic!("Wrong error {:?}", res.err().unwrap()),
}
Why this scored 32/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.