Expand expect_payment_failed!() to take error codes and use it more

expect_payment_failed!() was introduced after many of the tests
which could use it were written, so we take this opportunity to
switch them over now, increasing test coverage slightly by always
checking the payment hash expected.

Author: TheBlueMatt

lightning/src/ln/functional_test_utils.rs

@@ -764,13 +764,19 @@ macro_rules! expect_payment_sent {
 }
 
 macro_rules! expect_payment_failed {
-	($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr) => {
+	($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => {
 		let events = $node.node.get_and_clear_pending_events();
 		assert_eq!(events.len(), 1);
 		match events[0] {
-			Event::PaymentFailed { ref payment_hash, rejected_by_dest, .. } => {
+			Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref error_code, ref error_data } => {
 				assert_eq!(*payment_hash, $expected_payment_hash);
 				assert_eq!(rejected_by_dest, $rejected_by_dest);
+				assert!(error_code.is_some());
+				assert!(error_data.is_some());
+				$(
+					assert_eq!(error_code.unwrap(), $expected_error_code);
+					assert_eq!(&error_data.as_ref().unwrap()[..], $expected_error_data);
+				)*
 			},
 			_ => panic!("Unexpected event"),
 		}

lightning/src/ln/functional_tests.rs

@@ -17,7 +17,7 @@ use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
 use ln::msgs;
 use ln::msgs::{ChannelMessageHandler,RoutingMessageHandler,HTLCFailChannelUpdate, ErrorAction};
 use util::enforcing_trait_impls::EnforcingChannelKeys;
-use util::test_utils;
+use util::{byte_utils, test_utils};
 use util::events::{Event, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
 use util::errors::APIError;
 use util::ser::{Writeable, Writer, ReadableArgs};
@@ -949,15 +949,7 @@ fn htlc_fail_async_shutdown() {
 	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates_2.update_fail_htlcs[0]);
 	commitment_signed_dance!(nodes[0], nodes[1], updates_2.commitment_signed, false, true);
 
-	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, .. } => {
-			assert_eq!(our_payment_hash, *payment_hash);
-			assert!(!rejected_by_dest);
-		},
-		_ => panic!("Unexpected event"),
-	}
+	expect_payment_failed!(nodes[0], our_payment_hash, false);
 
 	let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
 	assert_eq!(msg_events.len(), 2);
@@ -1352,15 +1344,7 @@ fn holding_cell_htlc_counting() {
 		_ => panic!("Unexpected event"),
 	}
 
-	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
-			assert_eq!(payment_hash, payment_hash_2);
-			assert!(!rejected_by_dest);
-		},
-		_ => panic!("Unexpected event"),
-	}
+	expect_payment_failed!(nodes[0], payment_hash_2, false);
 
 	// Now forward all the pending HTLCs and claim them back
 	nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &initial_payment_event.msgs[0]);
@@ -2250,15 +2234,7 @@ fn claim_htlc_outputs_shared_tx() {
 		nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
 		check_added_monitors!(nodes[1], 1);
 		connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
-
-		let events = nodes[1].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash, .. } => {
-				assert_eq!(payment_hash, payment_hash_2);
-			},
-			_ => panic!("Unexpected event"),
-		}
+		expect_payment_failed!(nodes[1], payment_hash_2, true);
 
 		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 		assert_eq!(node_txn.len(), 3); // ChannelMonitor: penalty tx, ChannelManager: local commitment + HTLC-timeout
@@ -2323,15 +2299,7 @@ fn claim_htlc_outputs_single_tx() {
 		expect_pending_htlcs_forwardable_ignore!(nodes[0]);
 
 		connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash());
-
-		let events = nodes[1].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash, .. } => {
-				assert_eq!(payment_hash, payment_hash_2);
-			},
-			_ => panic!("Unexpected event"),
-		}
+		expect_payment_failed!(nodes[1], payment_hash_2, true);
 
 		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 		assert_eq!(node_txn.len(), 9);
@@ -2685,7 +2653,7 @@ fn test_simple_commitment_revoked_fail_backward() {
 	// Revoke the old state
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage, 3_000_000);
 
-	route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 3000000);
+	let (_, payment_hash) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 3000000);
 
 	let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
 	nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
@@ -2714,12 +2682,7 @@ fn test_simple_commitment_revoked_fail_backward() {
 				MessageSendEvent::PaymentFailureNetworkUpdate { .. } => {},
 				_ => panic!("Unexpected event"),
 			}
-			let events = nodes[0].node.get_and_clear_pending_events();
-			assert_eq!(events.len(), 1);
-			match events[0] {
-				Event::PaymentFailed { .. } => {},
-				_ => panic!("Unexpected event"),
-			}
+			expect_payment_failed!(nodes[0], payment_hash, false);
 		},
 		_ => panic!("Unexpected event"),
 	}
@@ -3687,26 +3650,10 @@ fn test_htlc_timeout() {
 
 	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_timeout_updates.update_fail_htlcs[0]);
 	commitment_signed_dance!(nodes[0], nodes[1], htlc_timeout_updates.commitment_signed, false);
-	let events = nodes[0].node.get_and_clear_pending_events();
-	match &events[0] {
-		&Event::PaymentFailed { payment_hash, rejected_by_dest, error_code, ref error_data } => {
-			assert_eq!(payment_hash, our_payment_hash);
-			assert!(rejected_by_dest);
-			assert_eq!(error_code.unwrap(), 0x4000 | 15);
-			// 100_000 msat as u64, followed by a height of 123 as u32
-			assert_eq!(&error_data.as_ref().unwrap()[..], &[
-				((100_000u64 >> 7*8) & 0xff) as u8,
-				((100_000u64 >> 6*8) & 0xff) as u8,
-				((100_000u64 >> 5*8) & 0xff) as u8,
-				((100_000u64 >> 4*8) & 0xff) as u8,
-				((100_000u64 >> 3*8) & 0xff) as u8,
-				((100_000u64 >> 2*8) & 0xff) as u8,
-				((100_000u64 >> 1*8) & 0xff) as u8,
-				((100_000u64 >> 0*8) & 0xff) as u8,
-				0, 0, 0, 123]);
-		},
-		_ => panic!("Unexpected event"),
-	}
+	// 100_000 msat as u64, followed by a height of 123 as u32
+	let mut expected_failure_data = byte_utils::be64_to_array(100_000).to_vec();
+	expected_failure_data.extend_from_slice(&byte_utils::be32_to_array(123));
+	expect_payment_failed!(nodes[0], our_payment_hash, true, 0x4000 | 15, &expected_failure_data[..]);
 }
 
 #[test]
@@ -4332,14 +4279,7 @@ fn test_static_spendable_outputs_timeout_tx() {
 	let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 	nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
 	connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
-	let events = nodes[1].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::PaymentFailed { payment_hash, .. } => {
-			assert_eq!(payment_hash, our_payment_hash);
-		},
-		_ => panic!("Unexpected event"),
-	}
+	expect_payment_failed!(nodes[1], our_payment_hash, true);
 
 	let spend_txn = check_spendable_outputs!(nodes[1], 1);
 	assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote (*2), timeout_tx.output (*1)
@@ -4685,13 +4625,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() {
 			_ => { panic!("Unexpected event"); }
 		}
 	}
-	let events = nodes[0].node.get_and_clear_pending_events();
-	match events[0] {
-		Event::PaymentFailed { ref payment_hash, .. } => {
-			assert_eq!(*payment_hash, duplicate_payment_hash);
-		}
-		_ => panic!("Unexpected event"),
-	}
+	expect_payment_failed!(nodes[0], duplicate_payment_hash, false);
 
 	// Solve 2nd HTLC by broadcasting on B's chain HTLC-Success Tx from C
 	nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![htlc_success_txn[0].clone()] }, 200);
@@ -5049,14 +4983,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 	let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 	nodes[0].block_notifier.block_connected(&Block { header: header_201, txdata: vec![htlc_timeout.clone()] }, 201);
 	connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
-	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	match events[0] {
-		Event::PaymentFailed { payment_hash, .. } => {
-			assert_eq!(payment_hash, our_payment_hash);
-		},
-		_ => panic!("Unexpected event"),
-	}
+	expect_payment_failed!(nodes[0], our_payment_hash, true);
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
 	let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -5207,15 +5134,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
 		check_closed_broadcast!(nodes[0], false);
 		check_added_monitors!(nodes[0], 1);
 	} else {
-		let events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => {
-				assert_eq!(payment_hash, our_payment_hash);
-				assert!(rejected_by_dest);
-			},
-			_ => panic!("Unexpected event"),
-		}
+		expect_payment_failed!(nodes[0], our_payment_hash, true);
 	}
 }
 
@@ -6578,29 +6497,15 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
 		assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
 		timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone());
 		let parent_hash  = connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash());
-		let events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash, .. } => {
-				assert_eq!(payment_hash, dust_hash);
-			},
-			_ => panic!("Unexpected event"),
-		}
+		expect_payment_failed!(nodes[0], dust_hash, true);
 		assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT);
 		// We fail non-dust-HTLC 2 by broadcast of local HTLC-timeout tx on local commitment tx
 		let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 		assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
 		nodes[0].block_notifier.block_connected(&Block { header: header_2, txdata: vec![timeout_tx[0].clone()]}, 7);
 		let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 		connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash());
-		let events = nodes[0].node.get_and_clear_pending_events();
-		assert_eq!(events.len(), 1);
-		match events[0] {
-			Event::PaymentFailed { payment_hash, .. } => {
-				assert_eq!(payment_hash, non_dust_hash);
-			},
-			_ => panic!("Unexpected event"),
-		}
+		expect_payment_failed!(nodes[0], non_dust_hash, true);
 	} else {
 		// We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC
 		nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![bs_commitment_tx[0].clone()]}, 1);
@@ -6611,28 +6516,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) {
 		let parent_hash  = connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash());
 		let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 		if !revoked {
-			let events = nodes[0].node.get_and_clear_pending_events();
-			assert_eq!(events.len(), 1);
-			match events[0] {
-				Event::PaymentFailed { payment_hash, .. } => {
-					assert_eq!(payment_hash, dust_hash);
-				},
-				_ => panic!("Unexpected event"),
-			}
+			expect_payment_failed!(nodes[0], dust_hash, true);
 			assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT);
 			// We fail non-dust-HTLC 2 by broadcast of local timeout tx on remote commitment tx
 			nodes[0].block_notifier.block_connected(&Block { header: header_2, txdata: vec![timeout_tx[0].clone()]}, 7);
 			assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0);
 			let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
 			connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash());
-			let events = nodes[0].node.get_and_clear_pending_events();
-			assert_eq!(events.len(), 1);
-			match events[0] {
-				Event::PaymentFailed { payment_hash, .. } => {
-					assert_eq!(payment_hash, non_dust_hash);
-				},
-				_ => panic!("Unexpected event"),
-			}
+			expect_payment_failed!(nodes[0], non_dust_hash, true);
 		} else {
 			// If revoked, both dust & non-dust HTLCs should have been failed after ANTI_REORG_DELAY confs of revoked
 			// commitment tx
@@ -6938,7 +6829,7 @@ fn test_check_htlc_underpaying() {
 	// Create some initial channels
 	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
 
-	let (payment_preimage, _) = route_payment(&nodes[0], &[&nodes[1]], 10_000);
+	let (payment_preimage, payment_hash) = route_payment(&nodes[0], &[&nodes[1]], 10_000);
 
 	// Node 3 is expecting payment of 100_000 but receive 10_000,
 	// fail htlc like we didn't know the preimage.
@@ -6963,25 +6854,10 @@ fn test_check_htlc_underpaying() {
 	nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &update_fail_htlc);
 	commitment_signed_dance!(nodes[0], nodes[1], commitment_signed, false, true);
 
-	let events = nodes[0].node.get_and_clear_pending_events();
-	assert_eq!(events.len(), 1);
-	if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, ref error_data } = &events[0] {
-		assert_eq!(*rejected_by_dest, true);
-		assert_eq!(error_code.unwrap(), 0x4000|15);
-		// 10_000 msat as u64, followed by a height of 99 as u32
-		assert_eq!(&error_data.as_ref().unwrap()[..], &[
-			((10_000u64 >> 7*8) & 0xff) as u8,
-			((10_000u64 >> 6*8) & 0xff) as u8,
-			((10_000u64 >> 5*8) & 0xff) as u8,
-			((10_000u64 >> 4*8) & 0xff) as u8,
-			((10_000u64 >> 3*8) & 0xff) as u8,
-			((10_000u64 >> 2*8) & 0xff) as u8,
-			((10_000u64 >> 1*8) & 0xff) as u8,
-			((10_000u64 >> 0*8) & 0xff) as u8,
-			0, 0, 0, 99]);
-	} else {
-		panic!("Unexpected event");
-	}
+	// 10_000 msat as u64, followed by a height of 99 as u32
+	let mut expected_failure_data = byte_utils::be64_to_array(10_000).to_vec();
+	expected_failure_data.extend_from_slice(&byte_utils::be32_to_array(99));
+	expect_payment_failed!(nodes[0], payment_hash, true, 0x4000|15, &expected_failure_data[..]);
 	nodes[1].node.get_and_clear_pending_events();
 }