Merge pull request #3586 from TheBlueMatt/2025-02-router-fixes

Fix router-backtrack cases in last-hop hints

fuzz/src/chanmon_consistency.rs

@@ -53,7 +53,7 @@ use lightning::ln::channelmanager::{
 use lightning::ln::functional_test_utils::*;
 use lightning::ln::inbound_payment::ExpandedKey;
 use lightning::ln::msgs::{
-	self, ChannelMessageHandler, CommitmentUpdate, DecodeError, Init, UpdateAddHTLC,
+	ChannelMessageHandler, CommitmentUpdate, DecodeError, Init, UpdateAddHTLC,
 };
 use lightning::ln::script::ShutdownScript;
 use lightning::ln::types::ChannelId;
@@ -118,7 +118,7 @@ impl Router for FuzzRouter {
 	fn find_route(
 		&self, _payer: &PublicKey, _params: &RouteParameters,
 		_first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs,
-	) -> Result<Route, msgs::LightningError> {
+	) -> Result<Route, &'static str> {
 		unreachable!()
 	}
 

fuzz/src/full_stack.rs

@@ -44,7 +44,7 @@ use lightning::ln::channelmanager::{
 };
 use lightning::ln::functional_test_utils::*;
 use lightning::ln::inbound_payment::ExpandedKey;
-use lightning::ln::msgs::{self, DecodeError};
+use lightning::ln::msgs::DecodeError;
 use lightning::ln::peer_handler::{
 	IgnoringMessageHandler, MessageHandler, PeerManager, SocketDescriptor,
 };
@@ -151,11 +151,8 @@ impl Router for FuzzRouter {
 	fn find_route(
 		&self, _payer: &PublicKey, _params: &RouteParameters,
 		_first_hops: Option<&[&ChannelDetails]>, _inflight_htlcs: InFlightHtlcs,
-	) -> Result<Route, msgs::LightningError> {
+	) -> Result<Route, &'static str> {
-		Err(msgs::LightningError {
+		Err("Not implemented")
-			err: String::from("Not implemented"),
-			action: msgs::ErrorAction::IgnoreError,
-		})
 	}
 
 	fn create_blinded_payment_paths<T: secp256k1::Signing + secp256k1::Verification>(

lightning/src/ln/functional_test_utils.rs

@@ -2169,7 +2169,7 @@ macro_rules! get_payment_preimage_hash {
 }
 
 /// Gets a route from the given sender to the node described in `payment_params`.
-pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result<Route, msgs::LightningError> {
+pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result<Route, &'static str> {
 	let scorer = TestScorer::new();
 	let keys_manager = TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 	let random_seed_bytes = keys_manager.get_secure_random_bytes();
@@ -2181,7 +2181,7 @@ pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result<Rou
 }
 
 /// Like `get_route` above, but adds a random CLTV offset to the final hop.
-pub fn find_route(send_node: &Node, route_params: &RouteParameters) -> Result<Route, msgs::LightningError> {
+pub fn find_route(send_node: &Node, route_params: &RouteParameters) -> Result<Route, &'static str> {
 	let scorer = TestScorer::new();
 	let keys_manager = TestKeysInterface::new(&[0u8; 32], Network::Testnet);
 	let random_seed_bytes = keys_manager.get_secure_random_bytes();

lightning/src/ln/outbound_payment.rs

@@ -2430,7 +2430,6 @@ mod tests {
 	use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
 	use crate::ln::inbound_payment::ExpandedKey;
 	use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, NodeFeatures};
-	use crate::ln::msgs::{ErrorAction, LightningError};
 	use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, PendingOutboundPayment, Retry, RetryableSendFailure, StaleExpiration};
 	#[cfg(feature = "std")]
 	use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY;
@@ -2532,8 +2531,7 @@ mod tests {
 		let payment_params = PaymentParameters::from_node_id(
 			PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0);
 		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 0);
-		router.expect_find_route(route_params.clone(),
+		router.expect_find_route(route_params.clone(), Err(""));
-			Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }));
 
 		let pending_events = Mutex::new(VecDeque::new());
 		if on_retry {
@@ -2863,13 +2861,11 @@ mod tests {
 		);
 		assert!(outbound_payments.has_pending_payments());
 
-		router.expect_find_route(
+		let route_params = RouteParameters::from_payment_params_and_value(
-			RouteParameters::from_payment_params_and_value(
+			PaymentParameters::from_bolt12_invoice(&invoice),
-				PaymentParameters::from_bolt12_invoice(&invoice),
+			invoice.amount_msats(),
-				invoice.amount_msats(),
-			),
-			Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }),
 		);
+		router.expect_find_route(route_params, Err(""));
 
 		assert_eq!(
 			outbound_payments.send_payment_for_bolt12_invoice(

lightning/src/ln/reload_tests.rs

@@ -964,6 +964,7 @@ fn do_test_partial_claim_before_restart(persist_both_monitors: bool) {
 
 		// Ensure that the remaining channel is fully operation and not blocked (and that after a
 		// cycle of commitment updates the payment preimage is ultimately pruned).
+		nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id());
 		send_payment(&nodes[0], &[&nodes[2], &nodes[3]], 100_000);
 		assert!(!get_monitor!(nodes[3], chan_id_not_persisted).get_stored_preimages().contains_key(&payment_hash));
 	}

lightning/src/util/test_utils.rs

@@ -28,7 +28,6 @@ use crate::ln::chan_utils::CommitmentTransaction;
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::channelmanager;
 use crate::ln::inbound_payment::ExpandedKey;
-use crate::ln::msgs::LightningError;
 use crate::ln::script::ShutdownScript;
 use crate::ln::types::ChannelId;
 use crate::ln::{msgs, wire};
@@ -139,7 +138,7 @@ pub struct TestRouter<'a> {
 		TestScorer,
 	>,
 	pub network_graph: Arc<NetworkGraph<&'a TestLogger>>,
-	pub next_routes: Mutex<VecDeque<(RouteParameters, Option<Result<Route, LightningError>>)>>,
+	pub next_routes: Mutex<VecDeque<(RouteParameters, Option<Result<Route, &'static str>>)>>,
 	pub next_blinded_payment_paths: Mutex<Vec<BlindedPaymentPath>>,
 	pub scorer: &'a RwLock<TestScorer>,
 }
@@ -167,7 +166,7 @@ impl<'a> TestRouter<'a> {
 		}
 	}
 
-	pub fn expect_find_route(&self, query: RouteParameters, result: Result<Route, LightningError>) {
+	pub fn expect_find_route(&self, query: RouteParameters, result: Result<Route, &'static str>) {
 		let mut expected_routes = self.next_routes.lock().unwrap();
 		expected_routes.push_back((query, Some(result)));
 	}
@@ -187,7 +186,7 @@ impl<'a> Router for TestRouter<'a> {
 	fn find_route(
 		&self, payer: &PublicKey, params: &RouteParameters, first_hops: Option<&[&ChannelDetails]>,
 		inflight_htlcs: InFlightHtlcs,
-	) -> Result<Route, msgs::LightningError> {
+	) -> Result<Route, &'static str> {
 		let route_res;
 		let next_route_opt = self.next_routes.lock().unwrap().pop_front();
 		if let Some((find_route_query, find_route_res)) = next_route_opt {