Detect duplicate inputs and outputs upon building FundingBuilder
What the developer wrote
Authored by Wilmer Paulino
Detect duplicate inputs and outputs upon building FundingBuilder
While this is already enforced when we get to the interactive
negotiation phase, we choose to fail early anyway.
What changed, and why it matters
This commit adds an early safety check in the code that builds Bitcoin funding transactions for Lightning channels. It now rejects duplicate inputs and duplicate outputs right when the transaction is being constructed, rather than waiting until later negotiation. Duplicate inputs could let someone try to spend the same coin twice in one transaction, and duplicate outputs could create bookkeeping confusion. The commit message says this was already blocked later in the process, so this is a fail-early hardening change.
Treat as a low-risk hardening improvement. Reviewers may verify that the duplicate checks correctly compare outpoints for inputs and script_pubkeys for outputs, and that no other duplicate-sensitive fields are missed. No urgent security response is indicated by the commit materials alone.
Security signals we found
Duplicate-input detection added to transaction builder
Duplicate-output detection added to transaction builder
Existing error variant reused for early rejection
Commit message describes change as fail-early hardening, not vulnerability fix
No CVE, advisory, or researcher attribution present in commit
Evidence from the diff
In lightning/src/ln/funding.rs, validate_inputs() and FundingBuilderInner::build() now scan for duplicate outpoints among inputs and duplicate script_pubkeys among outputs, returning FundingContributionError::InvalidSpliceValue if found. The error documentation and Display string were updated to mention duplicates. Two unit tests verify the new behavior. The commit message explicitly states the condition was already enforced during interactive negotiation, so this is defense-in-depth/early-failure hardening rather than a fix for an exploitable bypass.
Changed components
lightning/src/ln/funding.rsFundingBuilderInner::build()validate_inputs()FundingContributionError::InvalidSpliceValueInspect captured patch +52 / −5
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index b8d0539..e1f5d8f 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -132,8 +132,8 @@ pub enum FundingContributionError {
/// The minimum RBF feerate.
min_rbf_feerate: FeeRate,
},
- /// The splice value is invalid (zero, empty outputs, exceeds the maximum money supply, or
- /// splices out more than the available channel balance).
+ /// The splice value is invalid (zero, empty outputs, duplicate inputs or outputs, exceeds the
+ /// maximum money supply, or splices out more than the available channel balance).
InvalidSpliceValue,
/// An input's `prevtx` is too large to fit in a `tx_add_input` message.
PrevTxTooLarge,
@@ -164,7 +164,10 @@ impl core::fmt::Display for FundingContributionError {
write!(f, "Feerate {} is below minimum RBF feerate {}", feerate, min_rbf_feerate)
},
FundingContributionError::InvalidSpliceValue => {
- write!(f, "Invalid splice value (zero, empty, exceeds limit, or overdraws balance)")
+ write!(
+ f,
+ "Invalid splice value (zero, empty, duplicate, exceeds limit, or overdraws balance)"
+ )
},
FundingContributionError::PrevTxTooLarge => {
write!(f, "Input prevtx is too large to fit in a tx_add_input message")
@@ -514,7 +517,14 @@ fn estimate_transaction_fee(
fn validate_inputs(inputs: &[FundingTxInput]) -> Result<(), FundingContributionError> {
let mut total_value = Amount::ZERO;
- for input in inputs {
+ for (idx, input) in inputs.iter().enumerate() {
+ if inputs[..idx]
+ .iter()
+ .any(|existing_input| existing_input.utxo.outpoint == input.utxo.outpoint)
+ {
+ return Err(FundingContributionError::InvalidSpliceValue);
+ }
+
use crate::util::ser::Writeable;
const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
channel_id: ChannelId([0; 32]),
@@ -1362,7 +1372,14 @@ impl<State> FundingBuilderInner<State> {
)?;
let mut value_removed = Amount::ZERO;
- for output in self.outputs.iter() {
+ for (idx, output) in self.outputs.iter().enumerate() {
+ if self.outputs[..idx]
+ .iter()
+ .any(|existing_output| existing_output.script_pubkey == output.script_pubkey)
+ {
+ return Err(FundingContributionError::InvalidSpliceValue);
+ }
+
value_removed = match value_removed.checked_add(output.value) {
Some(sum) if sum <= Amount::MAX_MONEY => sum,
_ => return Err(FundingContributionError::InvalidSpliceValue),
@@ -2325,6 +2342,36 @@ mod tests {
);
}
+ #[test]
+ fn test_funding_builder_rejects_duplicate_inputs() {
+ let feerate = FeeRate::from_sat_per_kwu(2000);
+ let input = funding_input_sats(100_000);
+
+ let result = FundingTemplate::new(None, None, None, Amount::ZERO)
+ .without_prior_contribution(feerate, FeeRate::MAX)
+ .add_inputs(vec![input.clone(), input])
+ .unwrap()
+ .build();
+
+ assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),));
+ }
+
+ #[test]
+ fn test_funding_builder_rejects_duplicate_outputs() {
+ let feerate = FeeRate::from_sat_per_kwu(2000);
+ let first_output = funding_output_sats(25_000);
+ let second_output = funding_output_sats(30_000);
+ assert_ne!(first_output, second_output);
+ assert_eq!(first_output.script_pubkey, second_output.script_pubkey);
+
+ let result = FundingTemplate::new(None, None, None, Amount::MAX_MONEY)
+ .without_prior_contribution(feerate, FeeRate::MAX)
+ .add_outputs(vec![first_output, second_output])
+ .build();
+
+ assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),));
+ }
+
#[test]
fn test_funding_builder_remove_input_updates_manual_input_request() {
let feerate = FeeRate::from_sat_per_kwu(2000);
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.