Refactor complete_interactive_funding_negotiation_for_both
What the developer wrote
Authored by Jeffrey Czyz
Refactor complete_interactive_funding_negotiation_for_both
Use a single get_and_clear_pending_msg_events() + match pattern for the
initiator's turn, matching the existing acceptor code path. Also add
assertions that all expected initiator inputs and outputs were sent.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
What changed, and why it matters
This commit is a test-only refactor. It rewrites a helper function in the splicing test suite to use a cleaner match statement and adds assertions that expected test inputs/outputs were actually sent. It does not change production code, cryptographic logic, network handling, or any behavior that could affect real users' funds or node security.
No security action required. Treat as normal code-quality/test-maintenance review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change is confined to lightning/src/ln/splicing_tests.rs. It refactors complete_interactive_funding_negotiation_for_both to drain pending message events once per loop iteration and dispatch via match on MessageSendEvent variants, mirroring the existing acceptor path. It also adds two assertions verifying expected_initiator_inputs and expected_initiator_outputs are empty after negotiation. No production code, parsing, state machine, or consensus logic is modified.
Changed components
lightning/src/ln/splicing_tests.rsInspect captured patch +44 / −43
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index b45c3ce..486e386 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -374,53 +374,52 @@ pub fn complete_interactive_funding_negotiation_for_both<'a, 'b, 'c, 'd>(
(Vec::new(), Vec::new())
};
- let mut acceptor_sent_tx_complete = false;
let mut initiator_sent_tx_complete;
+ let mut acceptor_sent_tx_complete = false;
loop {
// Initiator's turn: send TxAddInput, TxAddOutput, or TxComplete
- if !expected_initiator_inputs.is_empty() {
- let tx_add_input =
- get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor);
- let input_prevout = BitcoinOutPoint {
- txid: tx_add_input
- .prevtx
- .as_ref()
- .map(|prevtx| prevtx.compute_txid())
- .or(tx_add_input.shared_input_txid)
- .unwrap(),
- vout: tx_add_input.prevtx_out,
- };
- expected_initiator_inputs.remove(
- expected_initiator_inputs.iter().position(|input| *input == input_prevout).unwrap(),
- );
- acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input);
- initiator_sent_tx_complete = false;
- } else if !expected_initiator_outputs.is_empty() {
- let tx_add_output =
- get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor);
- expected_initiator_outputs.remove(
- expected_initiator_outputs
- .iter()
- .position(|output| {
- *output.script_pubkey == tx_add_output.script
- && output.value.to_sat() == tx_add_output.sats
- })
- .unwrap(),
- );
- acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output);
- initiator_sent_tx_complete = false;
- } else {
- let msg_events = initiator.node.get_and_clear_pending_msg_events();
- assert_eq!(msg_events.len(), 1, "{msg_events:?}");
- if let MessageSendEvent::SendTxComplete { ref msg, .. } = &msg_events[0] {
+ let msg_events = initiator.node.get_and_clear_pending_msg_events();
+ assert_eq!(msg_events.len(), 1, "{msg_events:?}");
+ match &msg_events[0] {
+ MessageSendEvent::SendTxAddInput { msg, .. } => {
+ let input_prevout = BitcoinOutPoint {
+ txid: msg
+ .prevtx
+ .as_ref()
+ .map(|prevtx| prevtx.compute_txid())
+ .or(msg.shared_input_txid)
+ .unwrap(),
+ vout: msg.prevtx_out,
+ };
+ expected_initiator_inputs.remove(
+ expected_initiator_inputs
+ .iter()
+ .position(|input| *input == input_prevout)
+ .unwrap(),
+ );
+ acceptor.node.handle_tx_add_input(node_id_initiator, msg);
+ initiator_sent_tx_complete = false;
+ },
+ MessageSendEvent::SendTxAddOutput { msg, .. } => {
+ expected_initiator_outputs.remove(
+ expected_initiator_outputs
+ .iter()
+ .position(|output| {
+ *output.script_pubkey == msg.script && output.value.to_sat() == msg.sats
+ })
+ .unwrap(),
+ );
+ acceptor.node.handle_tx_add_output(node_id_initiator, msg);
+ initiator_sent_tx_complete = false;
+ },
+ MessageSendEvent::SendTxComplete { msg, .. } => {
acceptor.node.handle_tx_complete(node_id_initiator, msg);
- } else {
- panic!();
- }
- initiator_sent_tx_complete = true;
- if acceptor_sent_tx_complete {
- break;
- }
+ initiator_sent_tx_complete = true;
+ if acceptor_sent_tx_complete {
+ break;
+ }
+ },
+ _ => panic!("Unexpected message event: {:?}", msg_events[0]),
}
// Acceptor's turn: send TxAddInput, TxAddOutput, or TxComplete
@@ -467,6 +466,8 @@ pub fn complete_interactive_funding_negotiation_for_both<'a, 'b, 'c, 'd>(
}
}
+ assert!(expected_initiator_inputs.is_empty(), "Not all initiator inputs were sent");
+ assert!(expected_initiator_outputs.is_empty(), "Not all initiator outputs were sent");
assert!(expected_acceptor_inputs.is_empty(), "Not all acceptor inputs were sent");
assert!(expected_acceptor_scripts.is_empty(), "Not all acceptor outputs were sent");
}
Why this scored 14/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.