Update DefaultMessageRouter to always create compact blinded paths

Author: shaavan

lightning-dns-resolver/src/lib.rs

@@ -369,7 +369,7 @@ mod test {
 	#[tokio::test]
 	async fn end_to_end_test() {
 		let chanmon_cfgs = create_chanmon_cfgs(2);
-		let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+		let node_cfgs = create_node_cfgs_with_node_id_message_router(2, &chanmon_cfgs);
 		let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 		let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 

lightning/src/ln/functional_test_utils.rs

@@ -32,7 +32,7 @@ use crate::util::logger::Logger;
 use crate::util::scid_utils;
 use crate::util::test_channel_signer::TestChannelSigner;
 use crate::util::test_channel_signer::SignerOp;
-use crate::util::test_utils;
+use crate::util::test_utils::{self, TestLogger, TestMessageRouter};
 use crate::util::test_utils::{TestChainMonitor, TestScorer, TestKeysInterface};
 use crate::util::ser::{ReadableArgs, Writeable};
 
@@ -3293,24 +3293,35 @@ pub fn create_chanmon_cfgs_with_keys(node_count: usize, predefined_keys_ids: Opt
 	chan_mon_cfgs
 }
 
-pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMonCfg>) -> Vec<NodeCfg<'a>> {
-	create_node_cfgs_with_persisters(node_count, chanmon_cfgs, chanmon_cfgs.iter().map(|c| &c.persister).collect())
-}
-
-pub fn create_node_cfgs_with_persisters<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMonCfg>, persisters: Vec<&'a impl Persist<TestChannelSigner>>) -> Vec<NodeCfg<'a>> {
+fn create_node_cfgs_internal<'a, F>(
+	node_count: usize,
+	chanmon_cfgs: &'a Vec<TestChanMonCfg>,
+	persisters: Vec<&'a impl Persist<TestChannelSigner>>,
+	message_router_constructor: F,
+) -> Vec<NodeCfg<'a>>
+where
+	F: Fn(Arc<NetworkGraph<&'a TestLogger>>, &'a TestKeysInterface) -> TestMessageRouter<'a>,
+{
 	let mut nodes = Vec::new();
 
 	for i in 0..node_count {
-		let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[i].chain_source), &chanmon_cfgs[i].tx_broadcaster, &chanmon_cfgs[i].logger, &chanmon_cfgs[i].fee_estimator, persisters[i], &chanmon_cfgs[i].keys_manager);
 		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[i].logger));
+		let chain_monitor = test_utils::TestChainMonitor::new(
+			Some(&chanmon_cfgs[i].chain_source),
+			&chanmon_cfgs[i].tx_broadcaster,
+			&chanmon_cfgs[i].logger,
+			&chanmon_cfgs[i].fee_estimator,
+			persisters[i],
+			&chanmon_cfgs[i].keys_manager,
+		);
 		let seed = [i as u8; 32];
 		nodes.push(NodeCfg {
 			chain_source: &chanmon_cfgs[i].chain_source,
 			logger: &chanmon_cfgs[i].logger,
 			tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster,
 			fee_estimator: &chanmon_cfgs[i].fee_estimator,
 			router: test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[i].logger, &chanmon_cfgs[i].scorer),
-			message_router: test_utils::TestMessageRouter::new_default(network_graph.clone(), &chanmon_cfgs[i].keys_manager),
+			message_router: message_router_constructor(network_graph.clone(), &chanmon_cfgs[i].keys_manager),
 			chain_monitor,
 			keys_manager: &chanmon_cfgs[i].keys_manager,
 			node_seed: seed,
@@ -3322,33 +3333,29 @@ pub fn create_node_cfgs_with_persisters<'a>(node_count: usize, chanmon_cfgs: &'a
 	nodes
 }
 
-pub fn create_node_cfgs_with_node_id_message_router<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMonCfg>) -> Vec<NodeCfg<'a>> {
-	let persisters: Vec<_> = chanmon_cfgs.iter().map(|c| &c.persister).collect();
-
-	let mut nodes = Vec::new();
-
-	for i in 0..node_count {
-		let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[i].chain_source), &chanmon_cfgs[i].tx_broadcaster, &chanmon_cfgs[i].logger, &chanmon_cfgs[i].fee_estimator, persisters[i], &chanmon_cfgs[i].keys_manager);
-		let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[i].logger));
-		let seed = [i as u8; 32];
-		nodes.push(NodeCfg {
-			chain_source: &chanmon_cfgs[i].chain_source,
-			logger: &chanmon_cfgs[i].logger,
-			tx_broadcaster: &chanmon_cfgs[i].tx_broadcaster,
-			fee_estimator: &chanmon_cfgs[i].fee_estimator,
-			router: test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[i].logger, &chanmon_cfgs[i].scorer),
-			message_router: test_utils::TestMessageRouter::new_node_id_router(network_graph.clone(), &chanmon_cfgs[i].keys_manager),
-			chain_monitor,
-			keys_manager: &chanmon_cfgs[i].keys_manager,
-			node_seed: seed,
-			network_graph,
-			override_init_features: Rc::new(RefCell::new(None)),
-		});
-	}
+pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec<TestChanMonCfg>) -> Vec<NodeCfg<'a>> {
+	create_node_cfgs_with_persisters(
+		node_count,
+		chanmon_cfgs,
+		chanmon_cfgs.iter().map(|c| &c.persister).collect(),
+	)
+}
 
-	nodes
+pub fn create_node_cfgs_with_persisters<'a>(
+	node_count: usize,
+	chanmon_cfgs: &'a Vec<TestChanMonCfg>,
+	persisters: Vec<&'a impl Persist<TestChannelSigner>>,
+) -> Vec<NodeCfg<'a>> {
+	create_node_cfgs_internal(node_count, chanmon_cfgs, persisters, test_utils::TestMessageRouter::new_default)
 }
 
+pub fn create_node_cfgs_with_node_id_message_router<'a>(
+	node_count: usize,
+	chanmon_cfgs: &'a Vec<TestChanMonCfg>,
+) -> Vec<NodeCfg<'a>> {
+	let persisters: Vec<_> = chanmon_cfgs.iter().map(|c| &c.persister).collect();
+	create_node_cfgs_internal(node_count, chanmon_cfgs, persisters, test_utils::TestMessageRouter::new_node_id_router)
+}
 
 pub fn test_default_channel_config() -> UserConfig {
 	let mut default_config = UserConfig::default();

lightning/src/ln/offers_tests.rs

@@ -148,6 +148,16 @@ fn resolve_introduction_node<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, path: &Blinded
 		.unwrap()
 }
 
+fn check_compact_path_introduction_node<'a, 'b, 'c>(
+    path: &BlindedMessagePath,
+    lookup_node: &Node<'a, 'b, 'c>,
+    expected_introduction_node: PublicKey,
+) -> bool {
+    let introduction_node_id = resolve_introduction_node(lookup_node, path);
+    introduction_node_id == expected_introduction_node
+        && matches!(path.introduction_node(), IntroductionNode::DirectedShortChannelId(..))
+}
+
 fn route_bolt12_payment<'a, 'b, 'c>(
 	node: &Node<'a, 'b, 'c>, path: &[&Node<'a, 'b, 'c>], invoice: &Bolt12Invoice
 ) {
@@ -406,7 +416,7 @@ fn creates_short_lived_offer() {
 #[test]
 fn creates_long_lived_offer() {
 	let chanmon_cfgs = create_chanmon_cfgs(2);
-	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_cfgs = create_node_cfgs_with_node_id_message_router(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
@@ -470,7 +480,7 @@ fn creates_short_lived_refund() {
 #[test]
 fn creates_long_lived_refund() {
 	let chanmon_cfgs = create_chanmon_cfgs(2);
-	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_cfgs = create_node_cfgs_with_node_id_message_router(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
@@ -539,7 +549,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 
 	let payment_id = PaymentId([1; 32]);
@@ -569,7 +579,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
 	});
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, bob, charlie_id));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
 	charlie.onion_messenger.handle_onion_message(alice_id, &onion_message);
@@ -588,10 +598,8 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() {
 	// to Alice when she's handling the message. Therefore, either Bob or Charlie could
 	// serve as the introduction node for the reply path back to Alice.
 	assert!(
-		matches!(
-			reply_path.introduction_node(),
-			&IntroductionNode::NodeId(node_id) if node_id == bob_id || node_id == charlie_id,
-		)
+		check_compact_path_introduction_node(&reply_path, david, bob_id) ||
+		check_compact_path_introduction_node(&reply_path, david, charlie_id)
 	);
 
 	route_bolt12_payment(david, &[charlie, bob, alice], &invoice);
@@ -650,7 +658,7 @@ fn creates_and_pays_for_refund_using_two_hop_blinded_path() {
 	assert_ne!(refund.payer_signing_pubkey(), david_id);
 	assert!(!refund.paths().is_empty());
 	for path in refund.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+		assert!(check_compact_path_introduction_node(&path, david, charlie_id));
 	}
 	expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id);
 
@@ -674,8 +682,7 @@ fn creates_and_pays_for_refund_using_two_hop_blinded_path() {
 	for path in invoice.payment_paths() {
 		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
 	}
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
-
+	assert!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
 
 	route_bolt12_payment(david, &[charlie, bob, alice], &invoice);
 	expect_recent_payment!(david, RecentPaymentDetails::Pending, payment_id);
@@ -708,7 +715,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(alice_id));
+		assert!(check_compact_path_introduction_node(&path, bob, alice_id));
 	}
 
 	let payment_id = PaymentId([1; 32]);
@@ -730,7 +737,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
 	});
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
@@ -742,7 +749,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() {
 	for path in invoice.payment_paths() {
 		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(alice_id));
 	}
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(alice_id));
+	assert!(check_compact_path_introduction_node(&reply_path, bob, alice_id));
 
 	route_bolt12_payment(bob, &[alice], &invoice);
 	expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id);
@@ -779,7 +786,7 @@ fn creates_and_pays_for_refund_using_one_hop_blinded_path() {
 	assert_ne!(refund.payer_signing_pubkey(), bob_id);
 	assert!(!refund.paths().is_empty());
 	for path in refund.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 	expect_recent_payment!(bob, RecentPaymentDetails::AwaitingInvoice, payment_id);
 
@@ -798,7 +805,7 @@ fn creates_and_pays_for_refund_using_one_hop_blinded_path() {
 	for path in invoice.payment_paths() {
 		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(alice_id));
 	}
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(alice_id));
+	assert!(check_compact_path_introduction_node(&reply_path, bob, alice_id));
 
 	route_bolt12_payment(bob, &[alice], &invoice);
 	expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id);
@@ -956,7 +963,7 @@ fn send_invoice_requests_with_distinct_reply_path() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 
 	let payment_id = PaymentId([1; 32]);
@@ -975,7 +982,7 @@ fn send_invoice_requests_with_distinct_reply_path() {
 	alice.onion_messenger.handle_onion_message(bob_id, &onion_message);
 
 	let (_, reply_path) = extract_invoice_request(alice, &onion_message);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, charlie_id));
 
 	// Send, extract and verify the second Invoice Request message
 	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
@@ -985,7 +992,7 @@ fn send_invoice_requests_with_distinct_reply_path() {
 	alice.onion_messenger.handle_onion_message(bob_id, &onion_message);
 
 	let (_, reply_path) = extract_invoice_request(alice, &onion_message);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(nodes[6].node.get_our_node_id()));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, nodes[6].node.get_our_node_id()));
 }
 
 /// This test checks that when multiple potential introduction nodes are available for the payee,
@@ -1040,7 +1047,7 @@ fn send_invoice_for_refund_with_distinct_reply_path() {
 		.build().unwrap();
 	assert_ne!(refund.payer_signing_pubkey(), alice_id);
 	for path in refund.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 	expect_recent_payment!(alice, RecentPaymentDetails::AwaitingInvoice, payment_id);
 
@@ -1056,7 +1063,7 @@ fn send_invoice_for_refund_with_distinct_reply_path() {
 	let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
 
 	let (_, reply_path) = extract_invoice(alice, &onion_message);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, charlie_id));
 
 	// Send, extract and verify the second Invoice Request message
 	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
@@ -1065,7 +1072,7 @@ fn send_invoice_for_refund_with_distinct_reply_path() {
 	let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap();
 
 	let (_, reply_path) = extract_invoice(alice, &onion_message);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(nodes[6].node.get_our_node_id()));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, nodes[6].node.get_our_node_id()));
 }
 
 /// Verifies that the invoice request message can be retried if it fails to reach the
@@ -1091,7 +1098,7 @@ fn creates_and_pays_for_offer_with_retry() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(alice_id));
+		assert!(check_compact_path_introduction_node(&path, bob, alice_id));
 	}
 	let payment_id = PaymentId([1; 32]);
 	bob.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), RouteParametersConfig::default()).unwrap();
@@ -1119,7 +1126,7 @@ fn creates_and_pays_for_offer_with_retry() {
 	});
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+	assert!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
 
@@ -1384,7 +1391,7 @@ fn fails_authentication_when_handling_invoice_request() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 
 	let invalid_path = alice.node
@@ -1393,7 +1400,7 @@ fn fails_authentication_when_handling_invoice_request() {
 		.build().unwrap()
 		.paths().first().unwrap()
 		.clone();
-	assert_eq!(invalid_path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+	assert!(check_compact_path_introduction_node(&invalid_path, alice, bob_id));
 
 	// Send the invoice request directly to Alice instead of using a blinded path.
 	let payment_id = PaymentId([1; 32]);
@@ -1414,7 +1421,7 @@ fn fails_authentication_when_handling_invoice_request() {
 	let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
 
 	assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None);
 
@@ -1444,7 +1451,7 @@ fn fails_authentication_when_handling_invoice_request() {
 	let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
 
 	assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None);
 }
@@ -1495,7 +1502,7 @@ fn fails_authentication_when_handling_invoice_for_offer() {
 	assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id));
 	assert!(!offer.paths().is_empty());
 	for path in offer.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(bob_id));
+		assert!(check_compact_path_introduction_node(&path, alice, bob_id));
 	}
 
 	// Initiate an invoice request, but abandon tracking it.
@@ -1546,7 +1553,7 @@ fn fails_authentication_when_handling_invoice_for_offer() {
 	let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message);
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), david_id);
-	assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+	assert!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
 	charlie.onion_messenger.handle_onion_message(alice_id, &onion_message);
@@ -1603,7 +1610,7 @@ fn fails_authentication_when_handling_invoice_for_refund() {
 	assert_ne!(refund.payer_signing_pubkey(), david_id);
 	assert!(!refund.paths().is_empty());
 	for path in refund.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+		assert!(check_compact_path_introduction_node(&path, david, charlie_id));
 	}
 	expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id);
 
@@ -1637,7 +1644,7 @@ fn fails_authentication_when_handling_invoice_for_refund() {
 	assert_ne!(refund.payer_signing_pubkey(), david_id);
 	assert!(!refund.paths().is_empty());
 	for path in refund.paths() {
-		assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(charlie_id));
+		assert!(check_compact_path_introduction_node(&path, david, charlie_id));
 	}
 
 	let expected_invoice = alice.node.request_refund_payment(&refund).unwrap();

lightning/src/onion_message/messenger.rs

@@ -684,7 +684,7 @@ where
 			peers.into_iter(),
 			&self.entropy_source,
 			secp_ctx,
-			false,
+			true,
 		)
 	}