Re-order ConstructedTransaction::new checks
What the developer wrote
Authored by Jeffrey Czyz
Re-order ConstructedTransaction::new checks
An upcoming commit will include the contributed inputs and outputs in
an error whenever ConstructedTransaction::new fails. In order to DRY up
that logic, this commit updates the constructor to create the resulting
object prior to performing any checks. This way a conversion method can
be added that extracts the necessary input and output data.
What changed, and why it matters
This commit is a simple internal code cleanup in the rust-lightning library. It rearranges the order in which a transaction-building constructor validates its inputs, moving the creation of the transaction object earlier so that future error messages can include more detail. There is no functional security change visible in the diff itself.
No security action required. Treat as normal code-review refactoring. Monitor the follow-up commit referenced in the message if it is included in the same release, since it will expose input/output data in error paths and should be reviewed for information-disclosure implications.
Security signals we found
No change to validation rules or error conditions
Refactoring only: checks moved after object construction
Commit message describes the change as DRY-up for future error reporting
No new dependencies, unsafe code, or cryptographic changes
Evidence from the diff
The patch refactors ConstructedTransaction::new in lightning/src/ln/interactivetxs.rs. Previously, the constructor checked for the presence of the shared funding input/output before assembling the transaction. Now it builds the ConstructedTransaction first, then performs the same checks against the assembled object. The validation logic is preserved: MissingFundingInput, MissingFundingOutput, and TransactionTooLarge are still returned under the same conditions. The commit message explicitly frames this as preparation for a later change that will include contributed inputs/outputs in error output.
Changed components
lightning/src/ln/interactivetxs.rsConstructedTransaction::newInspect captured patch +24 / −30
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 3c683fc..00318e0 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -248,26 +248,13 @@ impl_writeable_tlv_based!(ConstructedTransaction, {
impl ConstructedTransaction {
fn new(context: NegotiationContext) -> Result<Self, AbortReason> {
- if let Some(shared_funding_input) = &context.shared_funding_input {
- if !context.inputs.iter().any(|(_, input)| {
- input.txin().previous_output == shared_funding_input.input.previous_output
- }) {
- return Err(AbortReason::MissingFundingInput);
- }
- }
- if !context
- .outputs
- .iter()
- .any(|(_, output)| *output.tx_out() == context.shared_funding_output.tx_out)
- {
- return Err(AbortReason::MissingFundingOutput);
- }
-
let satisfaction_weight =
Weight::from_wu(context.inputs.iter().fold(0u64, |value, (_, input)| {
value.saturating_add(input.satisfaction_weight().to_wu())
}));
+ let lock_time = context.tx_locktime;
+
let mut inputs: Vec<(TxIn, TxInMetadata)> =
context.inputs.into_values().map(|input| input.into_txin_and_metadata()).collect();
let mut outputs: Vec<(TxOut, TxOutMetadata)> =
@@ -275,35 +262,42 @@ impl ConstructedTransaction {
inputs.sort_unstable_by_key(|(_, input)| input.serial_id);
outputs.sort_unstable_by_key(|(_, output)| output.serial_id);
+ let (input, input_metadata): (Vec<TxIn>, Vec<TxInMetadata>) = inputs.into_iter().unzip();
+ let (output, output_metadata): (Vec<TxOut>, Vec<TxOutMetadata>) =
+ outputs.into_iter().unzip();
+
let shared_input_index =
context.shared_funding_input.as_ref().and_then(|shared_funding_input| {
- inputs
+ input
.iter()
- .position(|(txin, _)| {
+ .position(|txin| {
txin.previous_output == shared_funding_input.input.previous_output
})
.map(|position| position as u32)
});
- let (input, input_metadata): (Vec<TxIn>, Vec<TxInMetadata>) = inputs.into_iter().unzip();
- let (output, output_metadata): (Vec<TxOut>, Vec<TxOutMetadata>) =
- outputs.into_iter().unzip();
+ let tx = ConstructedTransaction {
+ holder_is_initiator: context.holder_is_initiator,
+ input_metadata,
+ output_metadata,
+ tx: Transaction { version: Version::TWO, lock_time, input, output },
+ shared_input_index,
+ };
+
+ if context.shared_funding_input.is_some() && tx.shared_input_index.is_none() {
+ return Err(AbortReason::MissingFundingInput);
+ }
- let tx =
- Transaction { version: Version::TWO, lock_time: context.tx_locktime, input, output };
+ if !tx.tx.output.iter().any(|txout| *txout == context.shared_funding_output.tx_out) {
+ return Err(AbortReason::MissingFundingOutput);
+ }
- let tx_weight = tx.weight().checked_add(satisfaction_weight).unwrap_or(Weight::MAX);
+ let tx_weight = tx.tx.weight().checked_add(satisfaction_weight).unwrap_or(Weight::MAX);
if tx_weight > Weight::from_wu(MAX_STANDARD_TX_WEIGHT as u64) {
return Err(AbortReason::TransactionTooLarge);
}
- Ok(Self {
- holder_is_initiator: context.holder_is_initiator,
- input_metadata,
- output_metadata,
- tx,
- shared_input_index,
- })
+ Ok(tx)
}
pub fn tx(&self) -> &Transaction {
Why this scored 12/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.