From: Matt Corallo <git@bluematt.me>
Date: Sat, 4 Dec 2021 23:41:01 +0000 (+0000)
Subject: DRY up payment failure macros in functional_test_utils
X-Git-Tag: v0.0.104~7^2~4
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=8c9615e8d6be6d4e3ff2d861a8a30266217672b2;p=rust-lightning

DRY up payment failure macros in functional_test_utils

... with a more extensible expectation-checking framework for them.
---

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 78bf834a6..84f4860d4 100644
--- a/lightning/src/ln/functional_test_utils.rs
+++ b/lightning/src/ln/functional_test_utils.rs
@@ -336,10 +336,11 @@ pub fn create_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(node_a: &'a Node<'b,
 }
 
 #[macro_export]
+/// Gets an RAA and CS which were sent in response to a commitment update
 macro_rules! get_revoke_commit_msgs {
 	($node: expr, $node_id: expr) => {
 		{
-			use util::events::MessageSendEvent;
+			use $crate::util::events::MessageSendEvent;
 			let events = $node.node.get_and_clear_pending_msg_events();
 			assert_eq!(events.len(), 2);
 			(match events[0] {
@@ -400,8 +401,8 @@ macro_rules! get_event {
 	}
 }
 
-#[cfg(test)]
 #[macro_export]
+/// Gets an UpdateHTLCs MessageSendEvent
 macro_rules! get_htlc_update_msgs {
 	($node: expr, $node_id: expr) => {
 		{
@@ -933,6 +934,9 @@ impl SendEvent {
 	}
 }
 
+#[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 */) => {
 		{
@@ -999,7 +1003,7 @@ macro_rules! commitment_signed_dance {
 		{
 			commitment_signed_dance!($node_a, $node_b, $commitment_signed, $fail_backwards, true);
 			if $fail_backwards {
-				expect_pending_htlcs_forwardable!($node_a);
+				$crate::expect_pending_htlcs_forwardable!($node_a);
 				check_added_monitors!($node_a, 1);
 
 				let channel_state = $node_a.node.channel_state.lock().unwrap();
@@ -1057,6 +1061,8 @@ macro_rules! get_route_and_payment_hash {
 	}}
 }
 
+#[macro_export]
+/// Clears (and ignores) a PendingHTLCsForwardable event
 macro_rules! expect_pending_htlcs_forwardable_ignore {
 	($node: expr) => {{
 		let events = $node.node.get_and_clear_pending_events();
@@ -1068,9 +1074,11 @@ macro_rules! expect_pending_htlcs_forwardable_ignore {
 	}}
 }
 
+#[macro_export]
+/// Handles a PendingHTLCsForwardable event
 macro_rules! expect_pending_htlcs_forwardable {
 	($node: expr) => {{
-		expect_pending_htlcs_forwardable_ignore!($node);
+		$crate::expect_pending_htlcs_forwardable_ignore!($node);
 		$node.node.process_pending_htlc_forwards();
 
 		// Ensure process_pending_htlc_forwards is idempotent.
@@ -1199,60 +1207,95 @@ macro_rules! expect_payment_forwarded {
 	}
 }
 
+pub struct PaymentFailedConditions<'a> {
+	pub(crate) expected_htlc_error_data: Option<(u16, &'a [u8])>,
+	pub(crate) expected_blamed_scid: Option<u64>,
+	pub(crate) expected_blamed_chan_closed: Option<bool>,
+}
+
+impl<'a> PaymentFailedConditions<'a> {
+	pub fn new() -> Self {
+		Self {
+			expected_htlc_error_data: None,
+			expected_blamed_scid: None,
+			expected_blamed_chan_closed: None,
+		}
+	}
+	pub fn blamed_scid(mut self, scid: u64) -> Self {
+		self.expected_blamed_scid = Some(scid);
+		self
+	}
+	pub fn blamed_chan_closed(mut self, closed: bool) -> Self {
+		self.expected_blamed_chan_closed = Some(closed);
+		self
+	}
+	pub fn expected_htlc_error_data(mut self, code: u16, data: &'a [u8]) -> Self {
+		self.expected_htlc_error_data = Some((code, data));
+		self
+	}
+}
+
 #[cfg(test)]
 macro_rules! expect_payment_failed_with_update {
 	($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => {
-		let events = $node.node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, ref path, ref retry, .. } => {
-				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().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
-				assert!(error_code.is_some(), "expected error_code.is_some() = true");
-				assert!(error_data.is_some(), "expected error_data.is_some() = true");
-				match network_update {
-					&Some(NetworkUpdate::ChannelUpdateMessage { ref msg }) if !$chan_closed => {
-						assert_eq!(msg.contents.short_channel_id, $scid);
-						assert_eq!(msg.contents.flags & 2, 0);
-					},
-					&Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if $chan_closed => {
-						assert_eq!(short_channel_id, $scid);
-						assert!(is_permanent);
-					},
-					Some(_) => panic!("Unexpected update type"),
-					None => panic!("Expected update"),
-				}
-			},
-			_ => panic!("Unexpected event"),
-		}
+		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));
 	}
 }
 
 #[cfg(test)]
 macro_rules! expect_payment_failed {
 	($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
+		let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new();
+		$(
+			conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data);
+		)*
+		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);
 		match events[0] {
-			Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, ref path, ref retry, .. } => {
+			Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data, ref path, ref retry, 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().payee.pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
+
 				assert!(error_code.is_some(), "expected error_code.is_some() = true");
 				assert!(error_data.is_some(), "expected error_data.is_some() = true");
-				$(
-					assert_eq!(error_code.unwrap(), $expected_error_code, "unexpected error code");
-					assert_eq!(&error_data.as_ref().unwrap()[..], $expected_error_data, "unexpected 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::network_graph::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::network_graph::NetworkUpdate::ChannelClosed { 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"),
+					}
+				}
 			},
 			_ => panic!("Unexpected event"),
-		}
-	}
+		};
+	};
 }
 
 pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId {
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 3a30cb7f1..1bf4f67de 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -23,7 +23,7 @@ use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RAAC
 use ln::channel::{Channel, ChannelError};
 use ln::{chan_utils, onion_utils};
 use ln::chan_utils::{HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, HTLCOutputInCommitment};
-use routing::network_graph::{NetworkUpdate, RoutingFees};
+use routing::network_graph::RoutingFees;
 use routing::router::{Payee, Route, RouteHop, RouteHint, RouteHintHop, RouteParameters, find_route, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index d836b736a..a3733fb5e 100644
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -16,7 +16,6 @@ use ln::channelmanager::BREAKDOWN_TIMEOUT;
 use ln::features::InitFeatures;
 use ln::msgs::ChannelMessageHandler;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
-use routing::network_graph::NetworkUpdate;
 
 use bitcoin::blockdata::script::Builder;
 use bitcoin::blockdata::opcodes;
diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs
index 570dc3d0e..fe8fd6827 100644
--- a/lightning/src/ln/reorg_tests.rs
+++ b/lightning/src/ln/reorg_tests.rs
@@ -15,7 +15,6 @@ use chain::{Confirm, Watch};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
 use ln::features::InitFeatures;
 use ln::msgs::ChannelMessageHandler;
-use routing::network_graph::NetworkUpdate;
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
 use util::test_utils;
diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
index ffd809bc7..791e87377 100644
--- a/lightning/src/ln/shutdown_tests.rs
+++ b/lightning/src/ln/shutdown_tests.rs
@@ -13,7 +13,6 @@ use chain::keysinterface::KeysInterface;
 use chain::transaction::OutPoint;
 use ln::channelmanager::PaymentSendFailure;
 use routing::router::{Payee, get_route};
-use routing::network_graph::NetworkUpdate;
 use ln::features::{InitFeatures, InvoiceFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler, ErrorAction};