From cfb9eb96397e392475f6bc65368617372f41dc64 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 16 Mar 2023 18:27:52 +0000 Subject: [PATCH] Move some additional test macros into functions This marginally reduces the quantity of code compiled in tests further. --- lightning/src/ln/functional_test_utils.rs | 108 +++++++++++++--------- lightning/src/ln/functional_tests.rs | 31 +------ 2 files changed, 66 insertions(+), 73 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 39b88bd3b..f8ccce058 100644 --- 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 { + 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, 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>(node: &H, + expected_payment_preimage: PaymentPreimage, expected_fee_msat_opt: Option>, + 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 fc3768792..fac15fc5a 100644 --- 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] -- 2.39.5