f comment and make the tests more robust

Author: TheBlueMatt

lightning/src/ln/reload_tests.rs

@@ -19,6 +19,7 @@ use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
 use crate::util::enforcing_trait_impls::EnforcingSigner;
 use crate::util::test_utils;
+use crate::util::errors::APIError;
 use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::config::UserConfig;
@@ -851,20 +852,26 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
 	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
 	commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
 
+	// Store the `ChannelManager` before handling the `PendingHTLCsForwardable`/`HTLCIntercepted`
+	// events, expecting either event (and the HTLC itself) to be missing on reload even though its
+	// present when we serialized.
 	let node_encoded = nodes[1].node.encode();
 
+	let mut intercept_id = None;
+	let mut expected_outbound_amount_msat = None;
 	if use_intercept {
 		let events = nodes[1].node.get_and_clear_pending_events();
 		assert_eq!(events.len(), 1);
-		let (intercept_id, expected_outbound_amount_msat) = match events[0] {
+		(intercept_id, expected_outbound_amount_msat) = match events[0] {
 			crate::util::events::Event::HTLCIntercepted {
 				intercept_id, expected_outbound_amount_msat, ..
 			} => {
-				(intercept_id, expected_outbound_amount_msat)
+				(Some(intercept_id), Some(expected_outbound_amount_msat))
 			},
 			_ => panic!()
 		};
-		nodes[1].node.forward_intercepted_htlc(intercept_id, &chan_id_2, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap();
+		nodes[1].node.forward_intercepted_htlc(intercept_id.unwrap(), &chan_id_2,
+			nodes[2].node.get_our_node_id(), expected_outbound_amount_msat.unwrap()).unwrap();
 	}
 
 	expect_pending_htlcs_forwardable!(nodes[1]);
@@ -894,8 +901,20 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht
 	let chan_1_monitor_serialized = get_monitor!(nodes[1], chan_id_2).encode();
 	reload_node!(nodes[1], node_encoded, &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], persister, new_chain_monitor, nodes_1_deserialized);
 
+	// Note that this checks that this is the only event on nodes[1], implying the
+	// `HTLCIntercepted` event has been removed in the `use_intercept` case.
 	check_closed_event!(nodes[1], 1, ClosureReason::OutdatedChannelManager);
 
+	if use_intercept {
+		// Attempt to forward the HTLC back out over nodes[1]' still-open channel, ensuring we get
+		// a intercept-doesn't-exist error.
+		let forward_err = nodes[1].node.forward_intercepted_htlc(intercept_id.unwrap(), &chan_id_1,
+			nodes[0].node.get_our_node_id(), expected_outbound_amount_msat.unwrap()).unwrap_err();
+		assert_eq!(forward_err, APIError::APIMisuseError {
+			err: format!("Payment with intercept id {} not found", log_bytes!(intercept_id.unwrap().0))
+		});
+	}
+
 	let bs_commitment_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 	assert_eq!(bs_commitment_tx.len(), 1);