Remove `commitment_signed_dance` macro
What changed, and why it matters
This commit is a straightforward internal cleanup in the project's test code. It removes a helper macro used only in tests and replaces its remaining uses with an equivalent regular function. There is no change to the actual Lightning protocol logic, network-facing behavior, or production code, so it does not create or fix a security issue.
No security action needed. Treat as normal code-quality/test-maintenance review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change deletes the commitment_signed_dance! macro from functional_test_utils.rs and adds a concrete commitment_signed_dance_return_raa helper function that performs the same test-only sequence: handling a batched commitment_signed, checking monitor counts, running do_main_commitment_signed_dance, and returning the peer’s RevokeAndACK. All call sites in test files are updated to use the new function. The macro previously only had one active arm (with specific literal flags), so this is a refactor with no functional difference.
Changed components
lightning/src/ln/functional_test_utils.rslightning/src/ln/chanmon_update_fail_tests.rslightning/src/ln/functional_tests.rslightning/src/ln/payment_tests.rslightning/src/ln/reorg_tests.rsInspect captured patch +24 / −30
diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 0e6568d..18caa1b 100644
--- a/lightning/src/ln/chanmon_update_fail_tests.rs
+++ b/lightning/src/ln/chanmon_update_fail_tests.rs
@@ -857,7 +857,7 @@ fn test_monitor_update_fail_no_rebroadcast() {
SendEvent::from_event(nodes[0].node.get_and_clear_pending_msg_events().remove(0));
nodes[1].node.handle_update_add_htlc(node_a_id, &send_event.msgs[0]);
let commitment = send_event.commitment_msg;
- let bs_raa = commitment_signed_dance!(nodes[1], nodes[0], commitment, false, true, false, true);
+ let bs_raa = commitment_signed_dance_return_raa(&nodes[1], &nodes[0], &commitment, false);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
nodes[1].node.handle_revoke_and_ack(node_a_id, &bs_raa);
@@ -1006,7 +1006,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
let commitment = updates.commitment_signed;
let bs_revoke_and_ack =
- commitment_signed_dance!(nodes[1], nodes[2], commitment, false, true, false, true);
+ commitment_signed_dance_return_raa(&nodes[1], &nodes[2], &commitment, false);
check_added_monitors!(nodes[0], 0);
// While the second channel is AwaitingRAA, forward a second payment to get it into the
@@ -2084,7 +2084,7 @@ fn monitor_update_claim_fail_no_response() {
let payment_event = SendEvent::from_event(events.pop().unwrap());
nodes[1].node.handle_update_add_htlc(node_a_id, &payment_event.msgs[0]);
let commitment = payment_event.commitment_msg;
- let as_raa = commitment_signed_dance!(nodes[1], nodes[0], commitment, false, true, false, true);
+ let as_raa = commitment_signed_dance_return_raa(&nodes[1], &nodes[0], &commitment, false);
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
nodes[1].node.claim_funds(payment_preimage_1);
diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 90cb43d..7c00bcb 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -2640,29 +2640,6 @@ pub fn expect_htlc_forwarding_fails(
expect_htlc_failure_conditions(events, expected_failure);
}
-#[macro_export]
-/// Performs the "commitment signed dance" - the series of message exchanges which occur after a
-/// commitment update.
-macro_rules! commitment_signed_dance {
- ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr, true /* skip last step */, false /* return extra message */, true /* return last RAA */) => {{
- $crate::ln::functional_test_utils::check_added_monitors(&$node_a, 0);
- assert!($node_a.node.get_and_clear_pending_msg_events().is_empty());
- $node_a.node.handle_commitment_signed_batch_test(
- $node_b.node.get_our_node_id(),
- &$commitment_signed,
- );
- check_added_monitors(&$node_a, 1);
- let (extra_msg_option, bs_revoke_and_ack) =
- $crate::ln::functional_test_utils::do_main_commitment_signed_dance(
- &$node_a,
- &$node_b,
- $fail_backwards,
- );
- assert!(extra_msg_option.is_none());
- bs_revoke_and_ack
- }};
-}
-
/// Runs the commitment_signed dance after the initial commitment_signed is delivered through to
/// the initiator's `revoke_and_ack` response. i.e. [`do_main_commitment_signed_dance`] plus the
/// `revoke_and_ack` response to it.
@@ -2724,6 +2701,22 @@ pub fn do_main_commitment_signed_dance(
(extra_msg_option, bs_revoke_and_ack)
}
+pub fn commitment_signed_dance_return_raa(
+ node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>,
+ commitment_signed: &Vec<msgs::CommitmentSigned>, fail_backwards: bool,
+) -> msgs::RevokeAndACK {
+ check_added_monitors(&node_a, 0);
+ assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
+ node_a
+ .node
+ .handle_commitment_signed_batch_test(node_b.node.get_our_node_id(), commitment_signed);
+ check_added_monitors(&node_a, 1);
+ let (extra_msg_option, bs_revoke_and_ack) =
+ do_main_commitment_signed_dance(&node_a, &node_b, fail_backwards);
+ assert!(extra_msg_option.is_none());
+ bs_revoke_and_ack
+}
+
/// Runs a full commitment_signed dance, delivering a commitment_signed, the responding
/// `revoke_and_ack` and `commitment_signed`, and then the final `revoke_and_ack` response.
///
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index cd47c0c..0d3de52 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1935,7 +1935,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(
assert!(updates.update_fee.is_none());
nodes[1].node.handle_update_fail_htlc(node_c_id, &updates.update_fail_htlcs[0]);
let cs = updates.commitment_signed;
- let bs_raa = commitment_signed_dance!(nodes[1], nodes[2], cs, false, true, false, true);
+ let bs_raa = commitment_signed_dance_return_raa(&nodes[1], &nodes[2], &cs, false);
// Drop the last RAA from 3 -> 2
nodes[2].node.fail_htlc_backwards(&second_payment_hash);
@@ -4453,7 +4453,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno
do_commitment_signed_dance(&nodes[2], &nodes[3], commitment, false, false);
} else {
let cs = six_removes.commitment_signed;
- commitment_signed_dance!(nodes[2], nodes[3], cs, false, true, false, true);
+ commitment_signed_dance_return_raa(&nodes[2], &nodes[3], &cs, false);
}
// D's latest commitment transaction now contains 1st + 2nd + 9th HTLCs (implicitly, they're
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 9c0f0bb..bfdc1bd 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -4066,7 +4066,7 @@ fn test_threaded_payment_retries() {
// `process_pending_htlc_forwards`. Instead, we defer the monitor update check until after
// *we've* called `process_pending_htlc_forwards` when its guaranteed to have two updates.
let cs = bs_fail_updates.commitment_signed;
- let last_raa = commitment_signed_dance!(nodes[0], nodes[1], cs, false, true, false, true);
+ let last_raa = commitment_signed_dance_return_raa(&nodes[0], &nodes[1], &cs, false);
nodes[0].node.handle_revoke_and_ack(node_b_id, &last_raa);
let cur_time = Instant::now();
diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index ef0ced8..8d7b3f5 100644
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -772,7 +772,8 @@ fn test_htlc_preimage_claim_prev_counterparty_commitment_after_current_counterpa
// Handle the fee update on the other side, but don't send the last RAA such that the previous
// commitment is still valid (unrevoked).
nodes[1].node().handle_update_fee(nodes[0].node.get_our_node_id(), &update_fee);
- let _last_revoke_and_ack = commitment_signed_dance!(nodes[1], nodes[0], commit_sig, false, true, false, true);
+ let _last_revoke_and_ack = commitment_signed_dance_return_raa(&nodes[1], &nodes[0], &commit_sig, false);
+
let message = "Channel force-closed".to_owned();
// Force close with the latest commitment, confirm it, and reorg it with the previous commitment.
Why this scored 15/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.