Remove input value fields from ConstructedTransaction
What the developer wrote
Authored by Jeffrey Czyz
Remove input value fields from ConstructedTransaction
The local and remote input values are used to determine which node sends
tx_signatures first. Instead of persisting these values, compute them
only when needed from the input metadata. The spec states that the
entire shared input value is to be included for the node sending the
corresponding tx_add_input, so it isn't necessary to know the local and
remote balances which the metadata does not contain.
What changed, and why it matters
This commit refactors how a Lightning node decides which side sends transaction signatures first during an interactive funding transaction. Previously, the node stored separate running totals for local and remote input values inside a data structure called ConstructedTransaction. Now it computes those totals on demand from the input metadata. The change is intended to match the protocol specification, which says the entire shared input value should be counted toward whichever node proposed that input—not split into local and remote portions. There is no direct evidence in the commit that this fixes an active security bug, but it removes a place where stored and computed values could become inconsistent and affect the ordering rule.
Review whether any persisted ConstructedTransaction records from older versions need migration or compatibility handling, since the TLV serialization changed. Confirm that the new is_local classification matches the spec's definition of which node 'sent' a shared input. Otherwise treat as a normal correctness/refactoring patch.
Security signals we found
Removed persisted input-value totals that could diverge from input metadata
Ordering rule for tx_signatures now derived directly from canonical input metadata
Shared input value counted entirely for the contributing party, matching protocol spec
Serialization format changed (TLV fields 9 and 11 removed, field 13 renumbered to 9)
Evidence from the diff
The patch removes local_inputs_value_satoshis and remote_inputs_value_satoshis fields from ConstructedTransaction and from its TLV serialization. It adds two helper methods, local_contributed_input_value() and remote_contributed_input_value(), which iterate over input_metadata and sum prev_output.value, using is_local(holder_is_initiator) to classify inputs. The shared input is counted in full toward the side that added it, consistent with BOLT specification language. The tx_signatures ordering decision now uses these computed values instead of the removed persisted fields. Test code is updated to stop providing the removed fields.
Changed components
lightning/src/ln/interactivetxs.rsConstructedTransaction struct and serializationInteractiveTxSigningSession creation / tx_signatures orderingInspect captured patch +25 / −23
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 10f3a31..05b7332 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -197,14 +197,9 @@ impl Display for AbortReason {
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct ConstructedTransaction {
holder_is_initiator: bool,
-
input_metadata: Vec<TxInMetadata>,
output_metadata: Vec<TxOutMetadata>,
tx: Transaction,
-
- local_inputs_value_satoshis: u64,
- remote_inputs_value_satoshis: u64,
-
shared_input_index: Option<u32>,
}
@@ -249,9 +244,7 @@ impl_writeable_tlv_based!(ConstructedTransaction, {
(3, input_metadata, required),
(5, output_metadata, required),
(7, tx, required),
- (9, local_inputs_value_satoshis, required),
- (11, remote_inputs_value_satoshis, required),
- (13, shared_input_index, option),
+ (9, shared_input_index, option),
});
impl ConstructedTransaction {
@@ -271,12 +264,6 @@ impl ConstructedTransaction {
return Err(AbortReason::MissingFundingOutput);
}
- let local_inputs_value_satoshis = context
- .inputs
- .iter()
- .fold(0u64, |value, (_, input)| value.saturating_add(input.local_value()));
- let remote_inputs_value_satoshis = context.remote_inputs_value();
-
let satisfaction_weight =
Weight::from_wu(context.inputs.iter().fold(0u64, |value, (_, input)| {
value.saturating_add(input.satisfaction_weight().to_wu())
@@ -313,14 +300,9 @@ impl ConstructedTransaction {
Ok(Self {
holder_is_initiator: context.holder_is_initiator,
-
input_metadata,
output_metadata,
tx,
-
- local_inputs_value_satoshis,
- remote_inputs_value_satoshis,
-
shared_input_index,
})
}
@@ -337,6 +319,26 @@ impl ConstructedTransaction {
self.tx().compute_txid()
}
+ /// Returns the total input value from all local contributions, including the entire shared
+ /// input value if applicable.
+ fn local_contributed_input_value(&self) -> Amount {
+ self.input_metadata
+ .iter()
+ .filter(|input| input.is_local(self.holder_is_initiator))
+ .map(|input| input.prev_output.value)
+ .sum()
+ }
+
+ /// Returns the total input value from all remote contributions, including the entire shared
+ /// input value if applicable.
+ fn remote_contributed_input_value(&self) -> Amount {
+ self.input_metadata
+ .iter()
+ .filter(|input| !input.is_local(self.holder_is_initiator))
+ .map(|input| input.prev_output.value)
+ .sum()
+ }
+
/// Adds provided holder witnesses to holder inputs of unsigned transaction.
///
/// Note that it is assumed that the witness count equals the holder input count.
@@ -1379,11 +1381,13 @@ macro_rules! define_state_transitions {
let tx = context.validate_tx()?;
// Strict ordering prevents deadlocks during tx_signatures exchange
+ let local_contributed_input_value = tx.local_contributed_input_value();
+ let remote_contributed_input_value = tx.remote_contributed_input_value();
let holder_sends_tx_signatures_first =
- if tx.local_inputs_value_satoshis == tx.remote_inputs_value_satoshis {
+ if local_contributed_input_value == remote_contributed_input_value {
holder_node_id.serialize() < counterparty_node_id.serialize()
} else {
- tx.local_inputs_value_satoshis < tx.remote_inputs_value_satoshis
+ local_contributed_input_value < remote_contributed_input_value
};
let signing_session = InteractiveTxSigningSession {
@@ -3306,8 +3310,6 @@ mod tests {
input_metadata,
output_metadata: vec![], // N/A for test
tx: transaction.clone(),
- local_inputs_value_satoshis: 0, // N/A for test
- remote_inputs_value_satoshis: 0, // N/A for test
shared_input_index: None,
};
Why this scored 26/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.