Add some basic test coverage of monitor payment data reloading

Author: TheBlueMatt

lightning/src/ln/functional_test_utils.rs

@@ -14,7 +14,7 @@ use chain::{BestBlock, Confirm, Listen, Watch};
 use chain::channelmonitor::ChannelMonitor;
 use chain::transaction::OutPoint;
 use ln::{PaymentPreimage, PaymentHash, PaymentSecret};
-use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
+use ln::channelmanager::{ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, PaymentId};
 use routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
 use routing::router::{Route, get_route};
 use routing::scorer::Scorer;
@@ -1156,10 +1156,11 @@ macro_rules! expect_payment_failed {
 	}
 }
 
-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) {
-	origin_node.node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
+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 {
+	let payment_id = origin_node.node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)).unwrap();
 	check_added_monitors!(origin_node, expected_paths.len());
 	pass_along_route(origin_node, expected_paths, recv_value, our_payment_hash, our_payment_secret);
+	payment_id
 }
 
 pub fn pass_along_path<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_path: &[&Node<'a, 'b, 'c>], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: Option<PaymentSecret>, ev: MessageSendEvent, payment_received_expected: bool, expected_preimage: Option<PaymentPreimage>) {
@@ -1222,10 +1223,10 @@ pub fn pass_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
 	}
 }
 
-pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret) {
+pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64) -> (PaymentPreimage, PaymentHash, PaymentSecret, PaymentId) {
 	let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(expected_route.last().unwrap());
-	send_along_route_with_secret(origin_node, route, &[expected_route], recv_value, our_payment_hash, our_payment_secret);
-	(our_payment_preimage, our_payment_hash, our_payment_secret)
+	let payment_id = send_along_route_with_secret(origin_node, route, &[expected_route], recv_value, our_payment_hash, our_payment_secret);
+	(our_payment_preimage, our_payment_hash, our_payment_secret, payment_id)
 }
 
 pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
@@ -1339,7 +1340,8 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
 		assert_eq!(hop.pubkey, node.node.get_our_node_id());
 	}
 
-	send_along_route(origin_node, route, expected_route, recv_value)
+	let res = send_along_route(origin_node, route, expected_route, recv_value);
+	(res.0, res.1, res.2)
 }
 
 pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {

lightning/src/ln/functional_tests.rs

@@ -3689,7 +3689,7 @@ fn test_funding_peer_disconnect() {
 	nodes[0].net_graph_msg_handler.handle_channel_update(&as_update).unwrap();
 
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
-	let (payment_preimage, _, _) = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000);
+	let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000).0;
 	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
 
 	// Check that after deserialization and reconnection we can still generate an identical
@@ -4100,7 +4100,7 @@ fn test_no_txn_manager_serialize_deserialize() {
 	send_payment(&nodes[0], &[&nodes[1]], 1000000);
 }
 
-fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
+fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool, confirm_commitment_tx: bool, payment_timeout: bool) {
 	// When a Channel is closed, any outbound HTLCs which were relayed through it are simply
 	// dropped when the Channel is. From there, the ChannelManager relies on the ChannelMonitor
 	// having a copy of the relevant fail-/claim-back data and processes the HTLC fail/claim when
@@ -4121,7 +4121,7 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 
 	// Route a payment, but force-close the channel before the HTLC fulfill message arrives at
 	// nodes[0].
-	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1]], 10000000);
+	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 10000000);
 	nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap();
 	check_closed_broadcast!(nodes[0], true);
 	check_added_monitors!(nodes[0], 1);
@@ -4137,6 +4137,7 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 	assert_eq!(node_txn[0], node_txn[1]);
 	check_spends!(node_txn[1], funding_tx);
 	check_spends!(node_txn[2], node_txn[1]);
+	let timeout_txn = vec![node_txn[2].clone()];
 
 	assert!(nodes[1].node.claim_funds(payment_preimage));
 	check_added_monitors!(nodes[1], 1);
@@ -4151,15 +4152,30 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 	header.prev_blockhash = nodes[0].best_block_hash();
 	connect_block(&nodes[0], &Block { header, txdata: vec![node_txn[1].clone()]});
 
+	if confirm_commitment_tx {
+		connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
+	}
+
+	header.prev_blockhash = nodes[0].best_block_hash();
+	let claim_block = Block { header, txdata: if payment_timeout { timeout_txn } else { claim_txn } };
+
+	if payment_timeout {
+		assert!(confirm_commitment_tx); // Otherwise we're spending below our CSV!
+		connect_block(&nodes[0], &claim_block);
+		connect_blocks(&nodes[0], ANTI_REORG_DELAY - 2);
+	}
+
 	// Now connect the HTLC claim transaction with the ChainMonitor-generated ChannelMonitor update
 	// returning TemporaryFailure. This should cause the claim event to never make its way to the
 	// ChannelManager.
 	chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap().clear();
 	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
 
-	header.prev_blockhash = nodes[0].best_block_hash();
-	let claim_block = Block { header, txdata: claim_txn };
-	connect_block(&nodes[0], &claim_block);
+	if payment_timeout {
+		connect_blocks(&nodes[0], 1);
+	} else {
+		connect_block(&nodes[0], &claim_block);
+	}
 
 	let funding_txo = OutPoint { txid: funding_tx.txid(), index: 0 };
 	let mon_updates: Vec<_> = chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap()
@@ -4181,7 +4197,11 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 	let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
 	get_monitor!(nodes[0], chan_id).write(&mut chan_0_monitor_serialized).unwrap();
 	nodes[0].chain_monitor.chain_monitor.channel_monitor_updated(funding_txo, mon_updates[0]).unwrap();
-	expect_payment_sent!(nodes[0], payment_preimage);
+	if payment_timeout {
+		expect_payment_failed!(nodes[0], payment_hash, true);
+	} else {
+		expect_payment_sent!(nodes[0], payment_preimage);
+	}
 
 	// If we persist the ChannelManager after we get the PaymentSent event, we shouldn't get it
 	// twice.
@@ -4221,6 +4241,8 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 
 	if persist_manager_post_event {
 		assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+	} else if payment_timeout {
+		expect_payment_failed!(nodes[0], payment_hash, true);
 	} else {
 		expect_payment_sent!(nodes[0], payment_preimage);
 	}
@@ -4235,8 +4257,12 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool) {
 
 #[test]
 fn test_dup_htlc_onchain_fails_on_reload() {
-	do_test_dup_htlc_onchain_fails_on_reload(true);
-	do_test_dup_htlc_onchain_fails_on_reload(false);
+	do_test_dup_htlc_onchain_fails_on_reload(true, true, true);
+	do_test_dup_htlc_onchain_fails_on_reload(true, true, false);
+	do_test_dup_htlc_onchain_fails_on_reload(true, false, false);
+	do_test_dup_htlc_onchain_fails_on_reload(false, true, true);
+	do_test_dup_htlc_onchain_fails_on_reload(false, true, false);
+	do_test_dup_htlc_onchain_fails_on_reload(false, false, false);
 }
 
 #[test]

lightning/src/ln/onion_route_tests.rs

@@ -479,7 +479,7 @@ fn test_onion_failure() {
 
 	// Test a positive test-case with one extra msat, meeting the minimum.
 	bogus_route.paths[0][route_len-1].fee_msat = amt_to_forward + 1;
-	let (preimage, _, _) = send_along_route(&nodes[0], bogus_route, &[&nodes[1], &nodes[2]], amt_to_forward+1);
+	let preimage = send_along_route(&nodes[0], bogus_route, &[&nodes[1], &nodes[2]], amt_to_forward+1).0;
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], preimage);
 
 	//TODO: with new config API, we will be able to generate both valid and