From 70acdf93d1280e77b5aa9385a25702b1f7915c50 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 20 May 2022 02:56:59 +0000 Subject: [PATCH] Make `expect_payment_failed_conditions` a function This reduces macro generated code in tests a good bit, and moves us one step further away from using macros everywhere when we don't need to. --- lightning/src/ln/functional_test_utils.rs | 108 ++++++++++++---------- lightning/src/ln/functional_tests.rs | 4 +- lightning/src/ln/onion_route_tests.rs | 14 +-- lightning/src/ln/payment_tests.rs | 8 +- lightning/src/ln/priv_short_conf_tests.rs | 6 +- 5 files changed, 75 insertions(+), 65 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 594f02fc..2cada934 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -15,7 +15,7 @@ use chain::channelmonitor::ChannelMonitor; use chain::transaction::OutPoint; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId, MIN_CLTV_EXPIRY_DELTA}; -use routing::gossip::{P2PGossipSync, NetworkGraph}; +use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate}; use routing::router::{PaymentParameters, Route, get_route}; use ln::features::{InitFeatures, InvoiceFeatures}; use ln::msgs; @@ -1467,8 +1467,10 @@ impl<'a> PaymentFailedConditions<'a> { #[cfg(test)] macro_rules! expect_payment_failed_with_update { ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => { - expect_payment_failed_conditions!($node, $expected_payment_hash, $rejected_by_dest, - $crate::ln::functional_test_utils::PaymentFailedConditions::new().blamed_scid($scid).blamed_chan_closed($chan_closed)); + $crate::ln::functional_test_utils::expect_payment_failed_conditions( + &$node, $expected_payment_hash, $rejected_by_dest, + $crate::ln::functional_test_utils::PaymentFailedConditions::new() + .blamed_scid($scid).blamed_chan_closed($chan_closed)); } } @@ -1480,64 +1482,72 @@ macro_rules! expect_payment_failed { $( conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data); )* - expect_payment_failed_conditions!($node, $expected_payment_hash, $rejected_by_dest, conditions); + $crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $rejected_by_dest, conditions); }; } -#[cfg(test)] -macro_rules! expect_payment_failed_conditions { - ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $conditions: expr) => { - let events = $node.node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - let expected_payment_id = match events[0] { - Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data, ref path, ref retry, ref payment_id, ref network_update, .. } => { - assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash"); - assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value"); - assert!(retry.is_some(), "expected retry.is_some()"); - assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path"); - assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path"); - +pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>( + node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_rejected_by_dest: bool, + conditions: PaymentFailedConditions<'e> +) { + let mut events = node.node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + let expected_payment_id = match events.pop().unwrap() { + Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update, + #[cfg(test)] + error_code, + #[cfg(test)] + error_data, .. } => { + assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash"); + assert_eq!(rejected_by_dest, expected_rejected_by_dest, "unexpected rejected_by_dest value"); + assert!(retry.is_some(), "expected retry.is_some()"); + assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path"); + assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path"); + + #[cfg(test)] + { assert!(error_code.is_some(), "expected error_code.is_some() = true"); assert!(error_data.is_some(), "expected error_data.is_some() = true"); - if let Some((code, data)) = $conditions.expected_htlc_error_data { + if let Some((code, data)) = conditions.expected_htlc_error_data { assert_eq!(error_code.unwrap(), code, "unexpected error code"); assert_eq!(&error_data.as_ref().unwrap()[..], data, "unexpected error data"); } + } - if let Some(chan_closed) = $conditions.expected_blamed_chan_closed { - match network_update { - &Some($crate::routing::gossip::NetworkUpdate::ChannelUpdateMessage { ref msg }) if !chan_closed => { - if let Some(scid) = $conditions.expected_blamed_scid { - assert_eq!(msg.contents.short_channel_id, scid); - } - assert_eq!(msg.contents.flags & 2, 0); - }, - &Some($crate::routing::gossip::NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => { - if let Some(scid) = $conditions.expected_blamed_scid { - assert_eq!(short_channel_id, scid); - } - assert!(is_permanent); - }, - Some(_) => panic!("Unexpected update type"), - None => panic!("Expected update"), - } + if let Some(chan_closed) = conditions.expected_blamed_chan_closed { + match network_update { + Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !chan_closed => { + if let Some(scid) = conditions.expected_blamed_scid { + assert_eq!(msg.contents.short_channel_id, scid); + } + const CHAN_DISABLED_FLAG: u8 = 2; + assert_eq!(msg.contents.flags & CHAN_DISABLED_FLAG, 0); + }, + Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => { + if let Some(scid) = conditions.expected_blamed_scid { + assert_eq!(short_channel_id, scid); + } + assert!(is_permanent); + }, + Some(_) => panic!("Unexpected update type"), + None => panic!("Expected update"), } + } - payment_id.unwrap() - }, - _ => panic!("Unexpected event"), - }; - if !$conditions.expected_mpp_parts_remain { - $node.node.abandon_payment(expected_payment_id); - let events = $node.node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - match events[0] { - Event::PaymentFailed { ref payment_hash, ref payment_id } => { - assert_eq!(*payment_hash, $expected_payment_hash, "unexpected second payment_hash"); - assert_eq!(*payment_id, expected_payment_id); - } - _ => panic!("Unexpected second event"), + payment_id.unwrap() + }, + _ => panic!("Unexpected event"), + }; + if !conditions.expected_mpp_parts_remain { + node.node.abandon_payment(expected_payment_id); + let events = node.node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { ref payment_hash, ref payment_id } => { + assert_eq!(*payment_hash, expected_payment_hash, "unexpected second payment_hash"); + assert_eq!(*payment_id, expected_payment_id); } + _ => panic!("Unexpected second event"), } } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index c7365452..db41777f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -9725,7 +9725,7 @@ fn do_test_dup_htlc_second_rejected(test_for_second_fail_panic: bool) { nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &fail_updates_1.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[1], fail_updates_1.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); + expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); claim_payment(&nodes[0], &[&nodes[1]], our_payment_preimage); } @@ -9830,7 +9830,7 @@ fn test_inconsistent_mpp_params() { nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &fail_updates_2.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[2], fail_updates_2.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); + expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); nodes[0].node.send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None).unwrap(); check_added_monitors!(nodes[0], 1); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index eabd2222..b1a90ab3 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -854,7 +854,7 @@ fn test_phantom_onion_hmac_failure() { .blamed_scid(phantom_scid) .blamed_chan_closed(true) .expected_htlc_error_data(0x8000 | 0x4000 | 5, &sha256_of_onion); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } #[test] @@ -927,7 +927,7 @@ fn test_phantom_invalid_onion_payload() { .blamed_scid(phantom_scid) .blamed_chan_closed(true) .expected_htlc_error_data(0x4000 | 22, &error_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, true, fail_conditions); } #[test] @@ -983,7 +983,7 @@ fn test_phantom_final_incorrect_cltv_expiry() { let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) .expected_htlc_error_data(18, &error_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } #[test] @@ -1028,7 +1028,7 @@ fn test_phantom_failure_too_low_cltv() { let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) .expected_htlc_error_data(17, &error_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } #[test] @@ -1076,7 +1076,7 @@ fn test_phantom_failure_too_low_recv_amt() { let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) .expected_htlc_error_data(0x4000 | 15, &error_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, true, fail_conditions); } #[test] @@ -1123,7 +1123,7 @@ fn test_phantom_dust_exposure_failure() { .blamed_scid(channel.0.contents.short_channel_id) .blamed_chan_closed(false) .expected_htlc_error_data(0x1000 | 7, &err_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } #[test] @@ -1174,5 +1174,5 @@ fn test_phantom_failure_reject_payment() { let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) .expected_htlc_error_data(0x4000 | 15, &error_data); - expect_payment_failed_conditions!(nodes[0], payment_hash, true, fail_conditions); + expect_payment_failed_conditions(&nodes[0], payment_hash, true, fail_conditions); } diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 07e531c5..d6d5ff99 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -70,7 +70,7 @@ fn retry_single_path_payment() { check_added_monitors!(nodes[1], 1); nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[1], htlc_updates.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); // Rebalance the channel so the retry succeeds. send_payment(&nodes[2], &vec!(&nodes[1])[..], 3_000_000); @@ -173,7 +173,7 @@ fn mpp_retry() { check_added_monitors!(nodes[2], 1); nodes[0].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &htlc_updates.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[2], htlc_updates.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); // Rebalance the channel so the second half of the payment can succeed. send_payment(&nodes[3], &vec!(&nodes[2])[..], 1_500_000); @@ -251,7 +251,7 @@ fn do_mpp_receive_timeout(send_partial_mpp: bool) { check_added_monitors!(nodes[1], 1); commitment_signed_dance!(nodes[0], nodes[1], htlc_fail_updates_1_0.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain().expected_htlc_error_data(23, &[][..])); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain().expected_htlc_error_data(23, &[][..])); } else { // Pass half of the payment along the second path. pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash, Some(payment_secret), events.remove(0), true, None); @@ -521,7 +521,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { confirm_transaction(&nodes[0], &first_htlc_timeout_tx); } nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clear(); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().mpp_parts_remain()); // Finally, retry the payment (which was reloaded from the ChannelMonitor when nodes[0] was // reloaded) via a route over the new channel, which work without issue and eventually be diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 051703a7..1785280d 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -460,7 +460,7 @@ fn test_inbound_scid_privacy() { nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); - expect_payment_failed_conditions!(nodes[0], payment_hash_2, false, + expect_payment_failed_conditions(&nodes[0], payment_hash_2, false, PaymentFailedConditions::new().blamed_scid(last_hop[0].short_channel_id.unwrap()) .blamed_chan_closed(true).expected_htlc_error_data(0x4000|10, &[0; 0])); } @@ -537,7 +537,7 @@ fn test_scid_alias_returned() { err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes()); err_data.extend_from_slice(&msg.encode()); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap()) .blamed_chan_closed(false).expected_htlc_error_data(0x1000|7, &err_data)); @@ -560,7 +560,7 @@ fn test_scid_alias_returned() { err_data.extend_from_slice(&(msg.serialized_length() as u16 + 2).to_be_bytes()); err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes()); err_data.extend_from_slice(&msg.encode()); - expect_payment_failed_conditions!(nodes[0], payment_hash, false, + expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap()) .blamed_chan_closed(false).expected_htlc_error_data(0x1000|12, &err_data)); } -- 2.30.2