Move some additional test macros into functions

This marginally reduces the quantity of code compiled in tests
further.

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 39b88bd3b2002d8cbc0a32af39f25a1dd10f1465..f8ccce05847a7d426a5b05118c25670febb4d6b0 100644 (file)
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -1555,23 +1555,30 @@ macro_rules! commitment_signed_dance {
                        bs_revoke_and_ack
                }
        };
-       ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, true /* return extra message */) => {
-               {
-                       let (extra_msg_option, bs_revoke_and_ack) = $crate::ln::functional_test_utils::do_main_commitment_signed_dance(&$node_a, &$node_b, $fail_backwards);
-                       $node_a.node.handle_revoke_and_ack(&$node_b.node.get_our_node_id(), &bs_revoke_and_ack);
-                       $crate::ln::functional_test_utils::check_added_monitors(&$node_a, 1);
-                       extra_msg_option
-               }
-       };
        ($node_a: expr, $node_b: expr, (), $fail_backwards: expr, true /* skip last step */, false /* no extra message */) => {
-               assert!(commitment_signed_dance!($node_a, $node_b, (), $fail_backwards, true, true).is_none());
+               assert!($crate::ln::functional_test_utils::commitment_signed_dance_through_cp_raa(&$node_a, &$node_b, $fail_backwards).is_none());
        };
        ($node_a: expr, $node_b: expr, $commitment_signed: expr, $fail_backwards: expr) => {
                $crate::ln::functional_test_utils::do_commitment_signed_dance(&$node_a, &$node_b, &$commitment_signed, $fail_backwards, false);
        }
 }
 
-
+/// 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.
+///
+/// Returns any additional message `node_b` generated in addition to the `revoke_and_ack` response.
+pub fn commitment_signed_dance_through_cp_raa(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> Option<MessageSendEvent> {
+       let (extra_msg_option, bs_revoke_and_ack) = do_main_commitment_signed_dance(node_a, node_b, fail_backwards);
+       node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack);
+       check_added_monitors(node_a, 1);
+       extra_msg_option
+}
+
+/// Does the main logic in the commitment_signed dance. After the first `commitment_signed` has
+/// been delivered, this method picks up and delivers the response `revoke_and_ack` and
+/// `commitment_signed`, returning the recipient's `revoke_and_ack` and any extra message it may
+/// have included.
 pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, fail_backwards: bool) -> (Option<MessageSendEvent>, msgs::RevokeAndACK) {
        let (as_revoke_and_ack, as_commitment_signed) = get_revoke_commit_msgs!(node_a, node_b.node.get_our_node_id());
        check_added_monitors!(node_b, 0);
@@ -1600,6 +1607,11 @@ pub fn do_main_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<
        (extra_msg_option, 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.
+///
+/// If `skip_last_step` is unset, also checks for the payment failure update for the previous hop
+/// on failure or that no new messages are left over on success.
 pub fn do_commitment_signed_dance(node_a: &Node<'_, '_, '_>, node_b: &Node<'_, '_, '_>, commitment_signed: &msgs::CommitmentSigned, fail_backwards: bool, skip_last_step: bool) {
        check_added_monitors!(node_a, 0);
        assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
@@ -1741,6 +1753,44 @@ macro_rules! expect_payment_claimed {
        }
 }
 
+pub fn expect_payment_sent<CM: AChannelManager, H: NodeHolder<CM=CM>>(node: &H,
+       expected_payment_preimage: PaymentPreimage, expected_fee_msat_opt: Option<Option<u64>>,
+       expect_per_path_claims: bool,
+) {
+       let events = node.node().get_and_clear_pending_events();
+       let expected_payment_hash = PaymentHash(
+               bitcoin::hashes::sha256::Hash::hash(&expected_payment_preimage.0).into_inner());
+       if expect_per_path_claims {
+               assert!(events.len() > 1);
+       } else {
+               assert_eq!(events.len(), 1);
+       }
+       let expected_payment_id = match events[0] {
+               Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
+                       assert_eq!(expected_payment_preimage, *payment_preimage);
+                       assert_eq!(expected_payment_hash, *payment_hash);
+                       if let Some(expected_fee_msat) = expected_fee_msat_opt {
+                               assert_eq!(*fee_paid_msat, expected_fee_msat);
+                       } else {
+                               assert!(fee_paid_msat.is_some());
+                       }
+                       payment_id.unwrap()
+               },
+               _ => panic!("Unexpected event"),
+       };
+       if expect_per_path_claims {
+               for i in 1..events.len() {
+                       match events[i] {
+                               Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
+                                       assert_eq!(payment_id, expected_payment_id);
+                                       assert_eq!(payment_hash, Some(expected_payment_hash));
+                               },
+                               _ => panic!("Unexpected event"),
+                       }
+               }
+       }
+}
+
 #[cfg(test)]
 #[macro_export]
 macro_rules! expect_payment_sent_without_paths {
@@ -1760,40 +1810,10 @@ macro_rules! expect_payment_sent {
        ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr) => {
                $crate::expect_payment_sent!($node, $expected_payment_preimage, $expected_fee_msat_opt, true);
        };
-       ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => { {
-               use bitcoin::hashes::Hash as _;
-               let events = $node.node.get_and_clear_pending_events();
-               let expected_payment_hash = $crate::ln::PaymentHash(
-                       bitcoin::hashes::sha256::Hash::hash(&$expected_payment_preimage.0).into_inner());
-               if $expect_paths {
-                       assert!(events.len() > 1);
-               } else {
-                       assert_eq!(events.len(), 1);
-               }
-               let expected_payment_id = match events[0] {
-                       $crate::events::Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
-                               assert_eq!($expected_payment_preimage, *payment_preimage);
-                               assert_eq!(expected_payment_hash, *payment_hash);
-                               assert!(fee_paid_msat.is_some());
-                               if $expected_fee_msat_opt.is_some() {
-                                       assert_eq!(*fee_paid_msat, $expected_fee_msat_opt);
-                               }
-                               payment_id.unwrap()
-                       },
-                       _ => panic!("Unexpected event"),
-               };
-               if $expect_paths {
-                       for i in 1..events.len() {
-                               match events[i] {
-                                       $crate::events::Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
-                                               assert_eq!(payment_id, expected_payment_id);
-                                               assert_eq!(payment_hash, Some(expected_payment_hash));
-                                       },
-                                       _ => panic!("Unexpected event"),
-                               }
-                       }
-               }
-       } }
+       ($node: expr, $expected_payment_preimage: expr, $expected_fee_msat_opt: expr, $expect_paths: expr) => {
+               $crate::ln::functional_test_utils::expect_payment_sent(&$node, $expected_payment_preimage,
+                       $expected_fee_msat_opt.map(|o| Some(o)), $expect_paths);
+       }
 }
 
 #[cfg(test)]

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index fc37687926420f4a8b2bb92605ce3588a29fab00..fac15fc5aaab9d17b429ae5ca8661fc03fb95918 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -4897,15 +4897,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
        commitment_signed_dance!(nodes[0], nodes[1], &updates.commitment_signed, false);
-
-       let events = nodes[0].node.get_and_clear_pending_events();
-       match events[0] {
-               Event::PaymentSent { ref payment_preimage, ref payment_hash, .. } => {
-                       assert_eq!(*payment_preimage, our_payment_preimage);
-                       assert_eq!(*payment_hash, duplicate_payment_hash);
-               }
-               _ => panic!("Unexpected event"),
-       }
+       expect_payment_sent(&nodes[0], our_payment_preimage, None, true);
 }
 
 #[test]
@@ -9481,26 +9473,7 @@ fn test_inconsistent_mpp_params() {
        pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);
 
        do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 3);
-       match events[0] {
-               Event::PaymentSent { payment_hash, .. } => { // The payment was abandoned earlier, so the fee paid will be None
-                       assert_eq!(payment_hash, our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
-       match events[1] {
-               Event::PaymentPathSuccessful { payment_hash, .. } => {
-                       assert_eq!(payment_hash.unwrap(), our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
-       match events[2] {
-               Event::PaymentPathSuccessful { payment_hash, .. } => {
-                       assert_eq!(payment_hash.unwrap(), our_payment_hash);
-               },
-               _ => panic!("Unexpected event")
-       }
+       expect_payment_sent(&nodes[0], our_payment_preimage, Some(None), true);
 }
 
 #[test]