Keep InteractiveTxConstructor contributed inputs and outputs
What the developer wrote
Authored by Jeffrey Czyz
Keep InteractiveTxConstructor contributed inputs and outputs
Instead of popping each input and output to contribute during an
interactive tx session, clone the necessary parts and keep around the
original inputs and outputs. This will let us reuse them later when
constructing an error. The tradeoff is using additional memory to avoid
more code complexity required to extract the sent input and outputs from
NegotiationContext.
What changed, and why it matters
This commit is a small internal refactor in the code that builds Bitcoin transactions for Lightning channel operations. It changes how the list of inputs and outputs is walked through during the interactive transaction protocol: instead of permanently removing each item from the list as it is sent, the code now keeps the original list and advances an index. The commit message says this is done so the original inputs and outputs can be reused later when constructing an error. There is no direct security fix visible in the diff itself, and no public references claim otherwise.
No immediate security action is required. Treat as a normal code refactor. If reviewing the series, verify that the follow-up commit which uses the preserved inputs/outputs for error construction does so safely and does not introduce information-leak or denial-of-service issues.
Security signals we found
Refactor preserves data that was previously consumed by pop()
Cloning of transaction data (prev_tx, script_pubkey) introduced to keep originals
No validation, state machine, or cryptographic checks are changed
Commit message frames change as preparation for future error construction, not as a security fix
Evidence from the diff
The patch modifies InteractiveTxConstructor in lightning/src/ln/interactivetxs.rs. Previously maybe_send_message used Vec::pop() to consume inputs_to_contribute and outputs_to_contribute. The patch introduces next_input_index/next_output_index fields and next_input_to_contribute/next_output_to_contribute helpers that return references and advance an index, leaving the original vectors intact. Message construction now clones the needed fields (prev_tx, script_pubkey) rather than moving them. The stated purpose is to retain contributed inputs/outputs for later reuse when building an error. The diff does not change protocol state transitions, validation logic, or error handling behavior; it only preserves ownership of data that was previously consumed.
Changed components
lightning/src/ln/interactivetxs.rsInteractiveTxConstructorInteractiveTxMessageSendDual-funded channel interactive transaction constructionInspect captured patch +41 / −10
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 44511f5..7063edd 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -1827,6 +1827,8 @@ pub(super) struct InteractiveTxConstructor {
channel_id: ChannelId,
inputs_to_contribute: Vec<(SerialId, InputOwned)>,
outputs_to_contribute: Vec<(SerialId, OutputOwned)>,
+ next_input_index: Option<usize>,
+ next_output_index: Option<usize>,
}
#[allow(clippy::enum_variant_names)] // Clippy doesn't like the repeated `Tx` prefix here
@@ -1979,12 +1981,17 @@ impl InteractiveTxConstructor {
// In the same manner and for the same rationale as the inputs above, we'll shuffle the outputs.
outputs_to_contribute.sort_unstable_by_key(|(serial_id, _)| *serial_id);
+ let next_input_index = (!inputs_to_contribute.is_empty()).then_some(0);
+ let next_output_index = (!outputs_to_contribute.is_empty()).then_some(0);
+
let mut constructor = Self {
state_machine,
initiator_first_message: None,
channel_id,
inputs_to_contribute,
outputs_to_contribute,
+ next_input_index,
+ next_output_index,
};
// We'll store the first message for the initiator.
if is_initiator {
@@ -1998,22 +2005,24 @@ impl InteractiveTxConstructor {
}
fn maybe_send_message(&mut self) -> Result<InteractiveTxMessageSend, AbortReason> {
+ let channel_id = self.channel_id;
+
// We first attempt to send inputs we want to add, then outputs. Once we are done sending
// them both, then we always send tx_complete.
- if let Some((serial_id, input)) = self.inputs_to_contribute.pop() {
+ if let Some((serial_id, input)) = self.next_input_to_contribute() {
let satisfaction_weight = input.satisfaction_weight();
let msg = match input {
InputOwned::Single(single) => msgs::TxAddInput {
- channel_id: self.channel_id,
- serial_id,
- prevtx: Some(single.prev_tx),
+ channel_id,
+ serial_id: *serial_id,
+ prevtx: Some(single.prev_tx.clone()),
prevtx_out: single.input.previous_output.vout,
sequence: single.input.sequence.to_consensus_u32(),
shared_input_txid: None,
},
InputOwned::Shared(shared) => msgs::TxAddInput {
- channel_id: self.channel_id,
- serial_id,
+ channel_id,
+ serial_id: *serial_id,
prevtx: None,
prevtx_out: shared.input.previous_output.vout,
sequence: shared.input.sequence.to_consensus_u32(),
@@ -2022,22 +2031,44 @@ impl InteractiveTxConstructor {
};
do_state_transition!(self, sent_tx_add_input, (&msg, satisfaction_weight))?;
Ok(InteractiveTxMessageSend::TxAddInput(msg))
- } else if let Some((serial_id, output)) = self.outputs_to_contribute.pop() {
+ } else if let Some((serial_id, output)) = self.next_output_to_contribute() {
let msg = msgs::TxAddOutput {
- channel_id: self.channel_id,
- serial_id,
+ channel_id,
+ serial_id: *serial_id,
sats: output.tx_out().value.to_sat(),
script: output.tx_out().script_pubkey.clone(),
};
do_state_transition!(self, sent_tx_add_output, &msg)?;
Ok(InteractiveTxMessageSend::TxAddOutput(msg))
} else {
- let msg = msgs::TxComplete { channel_id: self.channel_id };
+ let msg = msgs::TxComplete { channel_id };
do_state_transition!(self, sent_tx_complete, &msg)?;
Ok(InteractiveTxMessageSend::TxComplete(msg))
}
}
+ fn next_input_to_contribute(&mut self) -> Option<&(SerialId, InputOwned)> {
+ match self.next_input_index {
+ Some(index) => {
+ self.next_input_index =
+ index.checked_add(1).filter(|index| *index < self.inputs_to_contribute.len());
+ self.inputs_to_contribute.get(index)
+ },
+ None => None,
+ }
+ }
+
+ fn next_output_to_contribute(&mut self) -> Option<&(SerialId, OutputOwned)> {
+ match self.next_output_index {
+ Some(index) => {
+ self.next_output_index =
+ index.checked_add(1).filter(|index| *index < self.outputs_to_contribute.len());
+ self.outputs_to_contribute.get(index)
+ },
+ None => None,
+ }
+ }
+
pub fn handle_tx_add_input(
&mut self, msg: &msgs::TxAddInput,
) -> Result<InteractiveTxMessageSend, AbortReason> {
Why this scored 11/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.