Test auto-retry partial failure cases

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -7851,7 +7851,7 @@ pub mod bench {
 		let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
 		let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
 		let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
-		let router = test_utils::TestRouter { network_graph: Arc::new(NetworkGraph::new(genesis_hash, &logger_a)) };
+		let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)));
 
 		let mut config: UserConfig = Default::default();
 		config.channel_handshake_config.minimum_depth = 1;

lightning/src/ln/payment_tests.rs

@@ -22,7 +22,7 @@ use crate::ln::msgs;
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::ln::outbound_payment::Retry;
 use crate::routing::gossip::RoutingFees;
-use crate::routing::router::{get_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters};
+use crate::routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RouteParameters};
 use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
 use crate::util::test_utils;
 use crate::util::errors::APIError;
@@ -1755,3 +1755,201 @@ fn do_automatic_retries(test: AutoRetry) {
 		}
 	}
 }
+
+#[test]
+fn auto_retry_partial_failure() {
+	// Test that we'll retry appropriately on send partial failure and retry partial failure.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+	let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+	let chan_3_id = create_announced_chan_between_nodes(&nodes, 0, 1, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+
+	// Marshall data to send the payment
+	let amt_msat = 20_000;
+	let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
+	#[cfg(feature = "std")]
+	let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
+	#[cfg(not(feature = "std"))]
+	let payment_expiry_secs = 60 * 60;
+	let mut invoice_features = InvoiceFeatures::empty();
+	invoice_features.set_variable_length_onion_required();
+	invoice_features.set_payment_secret_required();
+	invoice_features.set_basic_mpp_optional();
+	let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())
+		.with_expiry_time(payment_expiry_secs as u64)
+		.with_features(invoice_features);
+	let route_params = RouteParameters {
+		payment_params,
+		final_value_msat: amt_msat,
+		final_cltv_expiry_delta: TEST_FINAL_CLTV,
+	};
+
+	// Ensure the first monitor update (for the initial send path1 over chan_1) succeeds, but the
+	// second (for the initial send path2 over chan_2) fails.
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::Completed);
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
+	// Ensure third monitor update (for the retry1's path1 over chan_1) succeeds, but the fourth (for
+	// the retry1's path2 over chan_3) fails, and monitor updates succeed after that.
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::Completed);
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::Completed);
+
+	// Configure the initial send, retry1 and retry2's paths.
+	let send_route = Route {
+		paths: vec![
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: channelmanager::provided_node_features(),
+				short_channel_id: chan_1_id,
+				channel_features: channelmanager::provided_channel_features(),
+				fee_msat: amt_msat / 2,
+				cltv_expiry_delta: 100,
+			}],
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: channelmanager::provided_node_features(),
+				short_channel_id: chan_2_id,
+				channel_features: channelmanager::provided_channel_features(),
+				fee_msat: amt_msat / 2,
+				cltv_expiry_delta: 100,
+			}],
+		],
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+	};
+	let retry_1_route = Route {
+		paths: vec![
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: channelmanager::provided_node_features(),
+				short_channel_id: chan_1_id,
+				channel_features: channelmanager::provided_channel_features(),
+				fee_msat: amt_msat / 4,
+				cltv_expiry_delta: 100,
+			}],
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: channelmanager::provided_node_features(),
+				short_channel_id: chan_3_id,
+				channel_features: channelmanager::provided_channel_features(),
+				fee_msat: amt_msat / 4,
+				cltv_expiry_delta: 100,
+			}],
+		],
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+	};
+	let retry_2_route = Route {
+		paths: vec![
+			vec![RouteHop {
+				pubkey: nodes[1].node.get_our_node_id(),
+				node_features: channelmanager::provided_node_features(),
+				short_channel_id: chan_1_id,
+				channel_features: channelmanager::provided_channel_features(),
+				fee_msat: amt_msat / 4,
+				cltv_expiry_delta: 100,
+			}],
+		],
+		payment_params: Some(PaymentParameters::from_node_id(nodes[1].node.get_our_node_id())),
+	};
+	nodes[0].router.expect_find_route(Ok(send_route));
+	nodes[0].router.expect_find_route(Ok(retry_1_route));
+	nodes[0].router.expect_find_route(Ok(retry_2_route));
+
+	// Send a payment that will partially fail on send, then partially fail on retry, then succeed.
+	nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(3)).unwrap();
+	let closed_chan_events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(closed_chan_events.len(), 2);
+	match closed_chan_events[0] {
+		Event::ChannelClosed { .. } => {},
+		_ => panic!("Unexpected event"),
+	}
+	match closed_chan_events[1] {
+		Event::ChannelClosed { .. } => {},
+		_ => panic!("Unexpected event"),
+	}
+
+	// Pass the first part of the payment along the path.
+	check_added_monitors!(nodes[0], 5); // three outbound channel updates succeeded, two permanently failed
+	let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+
+	// First message is the first update_add, remaining messages are broadcasting channel updates and
+	// errors for the permfailed channels
+	assert_eq!(msg_events.len(), 5);
+	let mut payment_event = SendEvent::from_event(msg_events.remove(0));
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &payment_event.commitment_msg);
+	check_added_monitors!(nodes[1], 1);
+	let (bs_first_raa, bs_first_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_first_raa);
+	check_added_monitors!(nodes[0], 1);
+	let as_second_htlc_updates = SendEvent::from_node(&nodes[0]);
+
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_first_cs);
+	check_added_monitors!(nodes[0], 1);
+	let as_first_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_first_raa);
+	check_added_monitors!(nodes[1], 1);
+
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.msgs[0]);
+	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.msgs[1]);
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_htlc_updates.commitment_msg);
+	check_added_monitors!(nodes[1], 1);
+	let (bs_second_raa, bs_second_cs) = get_revoke_commit_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
+	check_added_monitors!(nodes[0], 1);
+
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_cs);
+	check_added_monitors!(nodes[0], 1);
+	let as_second_raa = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_raa);
+	check_added_monitors!(nodes[1], 1);
+
+	expect_pending_htlcs_forwardable_ignore!(nodes[1]);
+	nodes[1].node.process_pending_htlc_forwards();
+	expect_payment_claimable!(nodes[1], payment_hash, payment_secret, amt_msat);
+	nodes[1].node.claim_funds(payment_preimage);
+	expect_payment_claimed!(nodes[1], payment_hash, amt_msat);
+	let bs_claim_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+	assert_eq!(bs_claim_update.update_fulfill_htlcs.len(), 1);
+
+	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_claim_update.update_fulfill_htlcs[0]);
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_claim_update.commitment_signed);
+	check_added_monitors!(nodes[0], 1);
+	let (as_third_raa, as_third_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_third_raa);
+	check_added_monitors!(nodes[1], 4);
+	let bs_second_claim_update = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_third_cs);
+	check_added_monitors!(nodes[1], 1);
+	let bs_third_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_third_raa);
+	check_added_monitors!(nodes[0], 1);
+
+	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.update_fulfill_htlcs[0]);
+	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.update_fulfill_htlcs[1]);
+	nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_claim_update.commitment_signed);
+	check_added_monitors!(nodes[0], 1);
+	let (as_fourth_raa, as_fourth_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id());
+
+	nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_fourth_raa);
+	check_added_monitors!(nodes[1], 1);
+
+	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_fourth_cs);
+	check_added_monitors!(nodes[1], 1);
+	let bs_second_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id());
+
+	nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_raa);
+	check_added_monitors!(nodes[0], 1);
+	expect_payment_sent!(nodes[0], payment_preimage);
+}

lightning/src/util/test_utils.rs

@@ -20,6 +20,7 @@ use crate::chain::keysinterface;
 use crate::ln::channelmanager;
 use crate::ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
 use crate::ln::{msgs, wire};
+use crate::ln::msgs::LightningError;
 use crate::ln::script::ShutdownScript;
 use crate::routing::gossip::NetworkGraph;
 use crate::routing::router::{find_route, InFlightHtlcs, Route, RouteHop, RouteParameters, Router, ScorerAccountingForInFlightHtlcs};
@@ -75,11 +76,17 @@ impl chaininterface::FeeEstimator for TestFeeEstimator {
 
 pub struct TestRouter<'a> {
 	pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
+	pub next_routes: Mutex<VecDeque<Result<Route, LightningError>>>,
 }
 
 impl<'a> TestRouter<'a> {
 	pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>) -> Self {
-		Self { network_graph }
+		Self { network_graph, next_routes: Mutex::new(VecDeque::new()), }
+	}
+
+	pub fn expect_find_route(&self, result: Result<Route, LightningError>) {
+		let mut expected_routes = self.next_routes.lock().unwrap();
+		expected_routes.push_back(result);
 	}
 }
 
@@ -88,6 +95,9 @@ impl<'a> Router for TestRouter<'a> {
 		&self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&channelmanager::ChannelDetails]>,
 		inflight_htlcs: &InFlightHtlcs
 	) -> Result<Route, msgs::LightningError> {
+		if let Some(find_route_res) = self.next_routes.lock().unwrap().pop_front() {
+			return find_route_res
+		}
 		let logger = TestLogger::new();
 		find_route(
 			payer, params, &self.network_graph, first_hops, &logger,
@@ -101,6 +111,15 @@ impl<'a> Router for TestRouter<'a> {
 	fn notify_payment_probe_failed(&self, _path: &[&RouteHop], _short_channel_id: u64) {}
 }
 
+impl<'a> Drop for TestRouter<'a> {
+	fn drop(&mut self) {
+		if std::thread::panicking() {
+			return;
+		}
+		assert!(self.next_routes.lock().unwrap().is_empty());
+	}
+}
+
 pub struct OnlyReadsKeysInterface {}
 
 impl EntropySource for OnlyReadsKeysInterface {