Correct error types for outbound splice checking methods
What the developer wrote
Authored by Matt Corallo
Correct error types for outbound splice checking methods
When doing an outbound splice, we check some of the instructions
first in utility methods, then convert errors to `APIError`s. These
utility methods should thus either return an `APIError` or more
generic (string or `()`) error type, but they currently return a
`ChannelError`, which is only approprite when the calling code will
do what the `ChannelError` instructs (including closing the
channel).
Here we fix that by returning `String`s instead.
What changed, and why it matters
This patch fixes a bug in how error types are used during Bitcoin Lightning channel 'splicing' operations. Splicing lets users add or remove funds from an open channel. Two internal helper functions were returning a 'ChannelError' type, which tells the caller to take specific actions such as warning or closing the channel. But the callers actually convert the result into a normal API error and do not follow those instructions. As a result, a splice that should simply fail with a clear message could instead be misinterpreted as a reason to close the channel or send a protocol warning. The fix changes the helpers to return plain text error strings instead, so the correct failure behavior happens.
Review callers of these helpers to confirm all error paths now produce sensible APIError variants and that no remaining ChannelError values are converted without honoring their action semantics. Consider adding regression tests that verify the exact APIError returned to users for insufficient splice inputs.
Security signals we found
Incorrect error-type semantics in channel state machine
Risk of unintended channel close or protocol warning due to ChannelError misuse
Outbound splice input validation helper returns error that may be converted to APIError
No explicit security advisory or CVE referenced in commit
Evidence from the diff
In rust-lightning’s channel.rs, check_splice_contribution_sufficient and check_v2_funding_inputs_sufficient previously returned Result<, ChannelError>. ChannelError carries action semantics (Warn, Close, Ignore, etc.) that the caller is expected to act upon. However, these helpers are called during outbound splice setup and their errors are converted into APIError before being returned to the user, meaning the ChannelError action semantics were not honored. Returning ChannelError::Warn from a path that is not actually handled as a protocol warning is semantically wrong and could lead to inappropriate channel handling. The patch changes both helpers to return Result<, String>, removing the misleading action semantics and updating unit tests to expect the bare string message.
Changed components
lightning/src/ln/channel.rscheck_splice_contribution_sufficientcheck_v2_funding_inputs_sufficientOutbound splicing flowInspect captured patch +13 / −14
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e9614bc..6cff41b 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5982,7 +5982,7 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos
fn check_splice_contribution_sufficient(
channel_balance: Amount, contribution: &SpliceContribution, is_initiator: bool,
funding_feerate: FeeRate,
-) -> Result<Amount, ChannelError> {
+) -> Result<Amount, String> {
let contribution_amount = contribution.value();
if contribution_amount < SignedAmount::ZERO {
let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee(
@@ -5996,10 +5996,10 @@ fn check_splice_contribution_sufficient(
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,
- )))
+ Err(format!(
+ "Available channel balance {channel_balance} is lower than needed for splicing out {}, considering fees of {estimated_fee}",
+ contribution_amount.unsigned_abs(),
+ ))
}
} else {
check_v2_funding_inputs_sufficient(
@@ -6066,7 +6066,7 @@ fn estimate_v2_funding_transaction_fee(
fn check_v2_funding_inputs_sufficient(
contribution_amount: i64, funding_inputs: &[FundingTxInput], is_initiator: bool,
is_splice: bool, funding_feerate_sat_per_1000_weight: u32,
-) -> Result<u64, ChannelError> {
+) -> Result<u64, String> {
let estimated_fee = estimate_v2_funding_transaction_fee(
funding_inputs, &[], is_initiator, is_splice, funding_feerate_sat_per_1000_weight,
);
@@ -6089,10 +6089,9 @@ fn check_v2_funding_inputs_sufficient(
let minimal_input_amount_needed = contribution_amount.saturating_add(estimated_fee as i64);
if (total_input_sats as i64) < minimal_input_amount_needed {
- Err(ChannelError::Warn(format!(
- "Total input amount {} is lower than needed for contribution {}, considering fees of {}. Need more inputs.",
- total_input_sats, contribution_amount, estimated_fee,
- )))
+ Err(format!(
+ "Total input amount {total_input_sats} is lower than needed for contribution {contribution_amount}, considering fees of {estimated_fee}. Need more inputs.",
+ ))
} else {
Ok(estimated_fee)
}
@@ -16205,8 +16204,8 @@ mod tests {
2000,
);
assert_eq!(
- format!("{:?}", res.err().unwrap()),
- "Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.",
+ res.err().unwrap(),
+ "Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.",
);
}
@@ -16241,8 +16240,8 @@ mod tests {
2200,
);
assert_eq!(
- format!("{:?}", res.err().unwrap()),
- "Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.",
+ res.err().unwrap(),
+ "Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.",
);
}
Why this scored 34/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.