wip

Author: valentinewallace

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -1959,7 +1959,7 @@ fn test_path_paused_mpp() {
 	// Set it so that the first monitor update (for the path 0 -> 1 -> 3) succeeds, but the second
 	// (for the path 0 -> 2 -> 3) fails.
 	chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
-	chanmon_cfgs[0].persister.set_next_update_ret(Some(ChannelMonitorUpdateStatus::InProgress));
+	chanmon_cfgs[0].persister.set_next_update_ret(ChannelMonitorUpdateStatus::InProgress);
 
 	// Now check that we get the right return value, indicating that the first path succeeded but
 	// the second got a MonitorUpdateInProgress err. This implies

lightning/src/ln/functional_test_utils.rs

@@ -939,7 +939,7 @@ pub fn create_chan_between_nodes_with_value_b<'a, 'b, 'c>(node_a: &Node<'a, 'b,
 }
 
 pub fn create_announced_chan_between_nodes<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {
-	create_announced_chan_between_nodes_with_value(nodes, a, b, 100000, 10001, a_flags, b_flags)
+	create_announced_chan_between_nodes_with_value(nodes, a, b, 100_000, 10_001, a_flags, b_flags)
 }
 
 pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, [u8; 32], Transaction) {

lightning/src/ln/payment_tests.rs

@@ -1754,3 +1754,71 @@ 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(4);
+	let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+	let mut nodes = create_network(4, &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_ann, _, chan_2_id, _) = create_announced_chan_between_nodes(&nodes, 0, 2, channelmanager::provided_init_features(), channelmanager::provided_init_features());
+	let chan_3_id = create_announced_chan_between_nodes(&nodes, 1, 3, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+	let chan_4_id = create_announced_chan_between_nodes(&nodes, 2, 3, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+
+
+	// This amount should force the router to split across nodes[0]'s two channels.
+	let amt_msat = 18_000_000;
+
+	// Marshall data to send the payment
+	let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[3], 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[3].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 path 0 -> 1 -> 3) succeeds, but the
+	// second (for the initial send path 0 -> 2 -> 3) 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 retry0's path 0 -> 1 -> 3) succeeds, but the fourth (for
+	// the retry0's path 0 -> 2 -> 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_update_ret(ChannelMonitorUpdateStatus::Completed);
+
+	// Configure the initial send, retry0 and retry1's paths.
+
+	// Send a payment that will partially fail on send, then partially fail on retry, then succeed.
+	match nodes[0].node.send_payment_with_retry(payment_hash, &Some(payment_secret), PaymentId(payment_hash.0), route_params, Retry::Attempts(1)) {
+		Err(PaymentSendFailure::PartialFailure { results, ..}) => {
+			assert_eq!(results[0], Ok(()));
+			assert_eq!(results[1], Err(APIError::ChannelUnavailable {
+				err: "ChannelMonitor storage failure".to_owned(),
+			}));
+		},
+		_ => panic!("Unexpected result"),
+	}
+
+	// Pass the first part of the payment along the path.
+	check_added_monitors!(nodes[0], 1);
+	let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
+	assert_eq!(msg_events.len(), 1);
+	pass_along_path(&nodes[0], &[&nodes[1], &nodes[3]], amt_msat, payment_hash, Some(payment_secret), msg_events.pop().unwrap(), true, None);
+
+	//
+}

lightning/src/util/test_utils.rs

@@ -75,6 +75,16 @@ impl chaininterface::FeeEstimator for TestFeeEstimator {
 
 pub struct TestRouter<'a> {
 	pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
+	pub next_routes: Mutex<VecDeque<Route>>,
+}
+
+impl<'a> TestRouter<'a> {
+	pub fn new(network_graph: Arc<NetworkGraph<&'a TestLogger>>) -> Self {
+		Self {
+			network_graph,
+			next_routes: Mutex::new(VecDeque::new()),
+		}
+	}
 }
 
 impl<'a> TestRouter<'a> {
@@ -88,6 +98,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(route) = self.next_routes.lock().unwrap().pop_front() {
+			return Ok(route)
+		}
 		let logger = TestLogger::new();
 		find_route(
 			payer, params, &self.network_graph, first_hops, &logger,
@@ -101,6 +114,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 {
@@ -224,10 +246,9 @@ impl<'a> chain::Watch<EnforcingSigner> for TestChainMonitor<'a> {
 }
 
 pub struct TestPersister {
-	pub update_ret: Mutex<chain::ChannelMonitorUpdateStatus>,
-	/// If this is set to Some(), after the next return, we'll always return this until update_ret
-	/// is changed:
-	pub next_update_ret: Mutex<Option<chain::ChannelMonitorUpdateStatus>>,
+	/// The queue of update statuses we'll return. If only one is queued, we'll always return it. If
+	/// none are queued, ::Completed will always be returned.
+	pub update_rets: Mutex<VecDeque<chain::ChannelMonitorUpdateStatus>>,
 	/// When we get an update_persisted_channel call with no ChannelMonitorUpdate, we insert the
 	/// MonitorUpdateId here.
 	pub chain_sync_monitor_persistences: Mutex<HashMap<OutPoint, HashSet<MonitorUpdateId>>>,
@@ -238,35 +259,39 @@ pub struct TestPersister {
 impl TestPersister {
 	pub fn new() -> Self {
 		Self {
-			update_ret: Mutex::new(chain::ChannelMonitorUpdateStatus::Completed),
-			next_update_ret: Mutex::new(None),
+			update_rets: Mutex::new(VecDeque::new()),
 			chain_sync_monitor_persistences: Mutex::new(HashMap::new()),
 			offchain_monitor_updates: Mutex::new(HashMap::new()),
 		}
 	}
 
+	/// Clear the queue of update statuses and set the one we'll always return.
 	pub fn set_update_ret(&self, ret: chain::ChannelMonitorUpdateStatus) {
-		*self.update_ret.lock().unwrap() = ret;
+		let mut update_rets = self.update_rets.lock().unwrap();
+		update_rets.clear();
+		update_rets.push_front(ret);
 	}
 
-	pub fn set_next_update_ret(&self, next_ret: Option<chain::ChannelMonitorUpdateStatus>) {
-		*self.next_update_ret.lock().unwrap() = next_ret;
+	/// Queue an update status to return.
+	pub fn set_next_update_ret(&self, next_ret: chain::ChannelMonitorUpdateStatus) {
+		self.update_rets.lock().unwrap().push_back(next_ret);
 	}
 }
 impl<Signer: keysinterface::Sign> chainmonitor::Persist<Signer> for TestPersister {
 	fn persist_new_channel(&self, _funding_txo: OutPoint, _data: &channelmonitor::ChannelMonitor<Signer>, _id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
-		let ret = self.update_ret.lock().unwrap().clone();
-		if let Some(next_ret) = self.next_update_ret.lock().unwrap().take() {
-			*self.update_ret.lock().unwrap() = next_ret;
-		}
-		ret
+		let mut update_rets = self.update_rets.lock().unwrap();
+		if update_rets.len() > 1 { return update_rets.pop_front().unwrap() }
+		else if update_rets.len() == 1 { return *update_rets.front().clone().unwrap() }
+		chain::ChannelMonitorUpdateStatus::Completed
 	}
 
 	fn update_persisted_channel(&self, funding_txo: OutPoint, update: &Option<channelmonitor::ChannelMonitorUpdate>, _data: &channelmonitor::ChannelMonitor<Signer>, update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
-		let ret = self.update_ret.lock().unwrap().clone();
-		if let Some(next_ret) = self.next_update_ret.lock().unwrap().take() {
-			*self.update_ret.lock().unwrap() = next_ret;
-		}
+		let ret = {
+			let mut update_rets = self.update_rets.lock().unwrap();
+			if update_rets.len() > 1 { update_rets.pop_front().unwrap() }
+			else if update_rets.len() == 1 { *update_rets.front().clone().unwrap() }
+			else { chain::ChannelMonitorUpdateStatus::Completed }
+		};
 		if update.is_none() {
 			self.chain_sync_monitor_persistences.lock().unwrap().entry(funding_txo).or_insert(HashSet::new()).insert(update_id);
 		} else {