← Watch feed
Use FundingTxInput in InteractiveTxConstructorArgs
Public commit record
63/100 · Adequate
What the developer wrote
Authored by Jeffrey Czyz
Use FundingTxInput in InteractiveTxConstructorArgs
InteractiveTxConstructor::new converts inputs given in channel.rs to
InputOwned. Instead of converting FundingTxInput in channel.rs to the
type needed by that constructor, pass in FundingTxInput and have it
converted directly to InputOwned there. This saves one conversion and
Vec allocation. It also lets us use the satisfaction_weight in an
upcoming refactor.
✓ Specific, descriptive subject✓ Provides detailed explanatory context
What changed, and why it matters
This is a routine internal code cleanup in a Bitcoin Lightning library. It changes how funding transaction inputs are passed between components so that one data type (FundingTxInput) is carried further through the code instead of being converted earlier. The commit message says this removes an unnecessary conversion and memory allocation, and prepares for a future refactor. There is no indication of a security fix or behavior change that would affect users.
Recommended action
No security action required. Treat as normal refactoring/code-quality change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors InteractiveTxConstructorArgs::inputs_to_contribute from Vec<(TxIn, Transaction)> to Vec
Changed components
lightning/src/ln/interactivetxs.rslightning/src/ln/channel.rslightning/src/ln/funding.rsInspect captured patch +45 / −84
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index de894de..b967c02 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -6617,14 +6617,6 @@ impl FundingNegotiationContext {
}
}
- let funding_inputs = self
- .our_funding_inputs
- .into_iter()
- .map(|FundingTxInput { utxo, sequence, prevtx }| {
- (TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() }, prevtx)
- })
- .collect();
-
let constructor_args = InteractiveTxConstructorArgs {
entropy_source,
holder_node_id,
@@ -6633,7 +6625,7 @@ impl FundingNegotiationContext {
feerate_sat_per_kw: self.funding_feerate_sat_per_1000_weight,
is_initiator: self.is_initiator,
funding_tx_locktime: self.funding_tx_locktime,
- inputs_to_contribute: funding_inputs,
+ inputs_to_contribute: self.our_funding_inputs,
shared_funding_input: self.shared_funding_input,
shared_funding_output: SharedOwnedOutput::new(
shared_funding_output,
@@ -13669,12 +13661,6 @@ where
value: Amount::from_sat(funding.get_value_satoshis()),
script_pubkey: funding.get_funding_redeemscript().to_p2wsh(),
};
- let inputs_to_contribute = our_funding_inputs
- .into_iter()
- .map(|FundingTxInput { utxo, sequence, prevtx }| {
- (TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() }, prevtx)
- })
- .collect();
let interactive_tx_constructor = Some(InteractiveTxConstructor::new(
InteractiveTxConstructorArgs {
@@ -13685,7 +13671,7 @@ where
feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight,
funding_tx_locktime: funding_negotiation_context.funding_tx_locktime,
is_initiator: false,
- inputs_to_contribute,
+ inputs_to_contribute: our_funding_inputs,
shared_funding_input: None,
shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_contribution_sats),
outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(),
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index b0b8cd4..df9b111 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -198,6 +198,11 @@ impl FundingTxInput {
FundingTxInput::new(prevtx, vout, witness_weight, Script::is_p2tr)
}
+ #[cfg(test)]
+ pub(crate) fn new_p2pkh(prevtx: Transaction, vout: u32) -> Result<Self, ()> {
+ FundingTxInput::new(prevtx, vout, Weight::ZERO, Script::is_p2pkh)
+ }
+
/// The sequence number to use in the [`TxIn`].
///
/// [`TxIn`]: bitcoin::TxIn
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 3d65996..bb8b9df 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -1920,7 +1920,7 @@ where
pub feerate_sat_per_kw: u32,
pub is_initiator: bool,
pub funding_tx_locktime: AbsoluteLockTime,
- pub inputs_to_contribute: Vec<(TxIn, Transaction)>,
+ pub inputs_to_contribute: Vec<FundingTxInput>,
pub shared_funding_input: Option<SharedOwnedInput>,
pub shared_funding_output: SharedOwnedOutput,
pub outputs_to_contribute: Vec<TxOut>,
@@ -1959,21 +1959,14 @@ impl InteractiveTxConstructor {
shared_funding_output.clone(),
);
- // Check for the existence of prevouts'
- for (txin, tx) in inputs_to_contribute.iter() {
- let vout = txin.previous_output.vout as usize;
- if tx.output.get(vout).is_none() {
- return Err(AbortReason::PrevTxOutInvalid);
- }
- }
let mut inputs_to_contribute: Vec<(SerialId, InputOwned)> = inputs_to_contribute
.into_iter()
- .map(|(txin, tx)| {
+ .map(|FundingTxInput { utxo, sequence, prevtx: prev_tx }| {
let serial_id = generate_holder_serial_id(entropy_source, is_initiator);
- let vout = txin.previous_output.vout as usize;
- let prev_output = tx.output.get(vout).unwrap().clone(); // checked above
+ let txin = TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() };
+ let prev_output = utxo.output;
let input =
- InputOwned::Single(SingleOwnedInput { input: txin, prev_tx: tx, prev_output });
+ InputOwned::Single(SingleOwnedInput { input: txin, prev_tx, prev_output });
(serial_id, input)
})
.collect();
@@ -2283,12 +2276,12 @@ mod tests {
struct TestSession {
description: &'static str,
- inputs_a: Vec<(TxIn, Transaction)>,
+ inputs_a: Vec<FundingTxInput>,
a_shared_input: Option<(OutPoint, TxOut, u64)>,
/// The funding output, with the value contributed
shared_output_a: (TxOut, u64),
outputs_a: Vec<TxOut>,
- inputs_b: Vec<(TxIn, Transaction)>,
+ inputs_b: Vec<FundingTxInput>,
b_shared_input: Option<(OutPoint, TxOut, u64)>,
/// The funding output, with the value contributed
shared_output_b: (TxOut, u64),
@@ -2558,20 +2551,22 @@ mod tests {
}
}
- fn generate_inputs(outputs: &[TestOutput]) -> Vec<(TxIn, Transaction)> {
+ fn generate_inputs(outputs: &[TestOutput]) -> Vec<FundingTxInput> {
let tx = generate_tx(outputs);
- let txid = tx.compute_txid();
- tx.output
+ outputs
.iter()
.enumerate()
- .map(|(idx, _)| {
- let txin = TxIn {
- previous_output: OutPoint { txid, vout: idx as u32 },
- script_sig: Default::default(),
- sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
- witness: Default::default(),
- };
- (txin, tx.clone())
+ .map(|(idx, output)| match output {
+ TestOutput::P2WPKH(_) => {
+ FundingTxInput::new_p2wpkh(tx.clone(), idx as u32).unwrap()
+ },
+ TestOutput::P2WSH(_) => {
+ FundingTxInput::new_p2wsh(tx.clone(), idx as u32, Weight::from_wu(42)).unwrap()
+ },
+ TestOutput::P2TR(_) => {
+ FundingTxInput::new_p2tr_key_spend(tx.clone(), idx as u32).unwrap()
+ },
+ TestOutput::P2PKH(_) => FundingTxInput::new_p2pkh(tx.clone(), idx as u32).unwrap(),
})
.collect()
}
@@ -2619,37 +2614,26 @@ mod tests {
(generate_txout(&TestOutput::P2WSH(value)), local_value)
}
- fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, Transaction)> {
+ fn generate_fixed_number_of_inputs(count: u16) -> Vec<FundingTxInput> {
// Generate transactions with a total `count` number of outputs such that no transaction has a
// serialized length greater than u16::MAX.
let max_outputs_per_prevtx = 1_500;
let mut remaining = count;
- let mut inputs: Vec<(TxIn, Transaction)> = Vec::with_capacity(count as usize);
+ let mut inputs: Vec<FundingTxInput> = Vec::with_capacity(count as usize);
while remaining > 0 {
let tx_output_count = remaining.min(max_outputs_per_prevtx);
remaining -= tx_output_count;
+ let outputs = vec![TestOutput::P2WPKH(1_000_000); tx_output_count as usize];
+
// Use unique locktime for each tx so outpoints are different across transactions
- let tx = generate_tx_with_locktime(
- &vec![TestOutput::P2WPKH(1_000_000); tx_output_count as usize],
- (1337 + remaining).into(),
- );
- let txid = tx.compute_txid();
+ let tx = generate_tx_with_locktime(&outputs, (1337 + remaining).into());
- let mut temp: Vec<(TxIn, Transaction)> = tx
- .output
+ let mut temp: Vec<FundingTxInput> = outputs
.iter()
.enumerate()
- .map(|(idx, _)| {
- let input = TxIn {
- previous_output: OutPoint { txid, vout: idx as u32 },
- script_sig: Default::default(),
- sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
- witness: Default::default(),
- };
- (input, tx.clone())
- })
+ .map(|(idx, _)| FundingTxInput::new_p2wpkh(tx.clone(), idx as u32).unwrap())
.collect();
inputs.append(&mut temp);
@@ -2860,13 +2844,11 @@ mod tests {
});
let tx = generate_tx(&[TestOutput::P2WPKH(1_000_000)]);
- let invalid_sequence_input = TxIn {
- previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
- ..Default::default()
- };
+ let mut invalid_sequence_input = FundingTxInput::new_p2wpkh(tx.clone(), 0).unwrap();
+ invalid_sequence_input.set_sequence(Default::default());
do_test_interactive_tx_constructor(TestSession {
description: "Invalid input sequence from initiator",
- inputs_a: vec![(invalid_sequence_input, tx.clone())],
+ inputs_a: vec![invalid_sequence_input],
a_shared_input: None,
shared_output_a: generate_funding_txout(1_000_000, 1_000_000),
outputs_a: vec![],
@@ -2876,14 +2858,10 @@ mod tests {
outputs_b: vec![],
expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)),
});
- let duplicate_input = TxIn {
- previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
- sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
- ..Default::default()
- };
+ let duplicate_input = FundingTxInput::new_p2wpkh(tx.clone(), 0).unwrap();
do_test_interactive_tx_constructor(TestSession {
description: "Duplicate prevout from initiator",
- inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())],
+ inputs_a: vec![duplicate_input.clone(), duplicate_input],
a_shared_input: None,
shared_output_a: generate_funding_txout(1_000_000, 1_000_000),
outputs_a: vec![],
@@ -2894,35 +2872,27 @@ mod tests {
expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeB)),
});
// Non-initiator uses same prevout as initiator.
- let duplicate_input = TxIn {
- previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
- sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
- ..Default::default()
- };
+ let duplicate_input = FundingTxInput::new_p2wpkh(tx.clone(), 0).unwrap();
do_test_interactive_tx_constructor(TestSession {
description: "Non-initiator uses same prevout as initiator",
- inputs_a: vec![(duplicate_input.clone(), tx.clone())],
+ inputs_a: vec![duplicate_input.clone()],
a_shared_input: None,
shared_output_a: generate_funding_txout(1_000_000, 905_000),
outputs_a: vec![],
- inputs_b: vec![(duplicate_input.clone(), tx.clone())],
+ inputs_b: vec![duplicate_input],
b_shared_input: None,
shared_output_b: generate_funding_txout(1_000_000, 95_000),
outputs_b: vec![],
expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
});
- let duplicate_input = TxIn {
- previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
- sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
- ..Default::default()
- };
+ let duplicate_input = FundingTxInput::new_p2wpkh(tx.clone(), 0).unwrap();
do_test_interactive_tx_constructor(TestSession {
description: "Non-initiator uses same prevout as initiator",
- inputs_a: vec![(duplicate_input.clone(), tx.clone())],
+ inputs_a: vec![duplicate_input.clone()],
a_shared_input: None,
shared_output_a: generate_funding_txout(1_000_000, 1_000_000),
outputs_a: vec![],
- inputs_b: vec![(duplicate_input.clone(), tx.clone())],
+ inputs_b: vec![duplicate_input],
b_shared_input: None,
shared_output_b: generate_funding_txout(1_000_000, 0),
outputs_b: vec![],
Risk score
Our methodology →Why this scored 14/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.