Make DefaultMessageRouter use the context to pad/compact paths

After much discussion in #3246 we mostly decided to allow
downstream developers to override whatever decisions the
`DefaultMessageRouter` makes regarding blinded path selection by
providing easy overrides for the selected `OnionMessageRouter`. We
did not, however, actually select good defaults for
`DefaultMessageRouter`.

Here we add those defaults, taking advantage of the
`MessageContext` we're given to detect why we're building a blinded
path and selecting blinding and compaction parameters based on it.

Specifically, if the blinded path is not being built for an offers
context, we always use a non-compact blinded path and always pad it
to four hops (including the recipient).

However, if the blinded path is being built for an `Offers` context
which implies it might need to fit in a QR code (or, worse, a
payment onion), we reduce our padding and try to build a compact
blinded path if possible.

We retain the `NodeIdMessageRouter` to disable compact blinded path
creation but use the same path-padding heuristic as for
`DefaultMessageRouter`.

Author: TheBlueMatt

lightning/src/ln/offers_tests.rs

@@ -60,7 +60,7 @@ use crate::offers::invoice_error::InvoiceError;
 use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields, InvoiceRequestVerifiedFromOffer};
 use crate::offers::nonce::Nonce;
 use crate::offers::parse::Bolt12SemanticError;
-use crate::onion_message::messenger::{DefaultMessageRouter, Destination, MessageSendInstructions, NodeIdMessageRouter, NullMessageRouter, PeeledOnion, PADDED_PATH_LENGTH};
+use crate::onion_message::messenger::{DefaultMessageRouter, Destination, MessageSendInstructions, NodeIdMessageRouter, NullMessageRouter, PeeledOnion, PADDED_PATH_LENGTH, QR_CODED_PADDED_PATH_LENGTH};
 use crate::onion_message::offers::OffersMessage;
 use crate::routing::gossip::{NodeAlias, NodeId};
 use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig};
@@ -163,6 +163,17 @@ fn check_compact_path_introduction_node<'a, 'b, 'c>(
 		&& matches!(path.introduction_node(), IntroductionNode::DirectedShortChannelId(..))
 }
 
+fn check_padded_path_length<'a, 'b, 'c>(
+	path: &BlindedMessagePath,
+	lookup_node: &Node<'a, 'b, 'c>,
+	expected_introduction_node: PublicKey,
+	expected_path_length: usize,
+) -> bool {
+	let introduction_node_id = resolve_introduction_node(lookup_node, path);
+	introduction_node_id == expected_introduction_node
+		&& path.blinded_hops().len() == expected_path_length
+}
+
 fn route_bolt12_payment<'a, 'b, 'c>(
 	node: &Node<'a, 'b, 'c>, path: &[&Node<'a, 'b, 'c>], invoice: &Bolt12Invoice
 ) {
@@ -455,7 +466,7 @@ fn check_dummy_hop_pattern_in_offer() {
 	let bob_id = bob.node.get_our_node_id();
 
 	// Case 1: DefaultMessageRouter → uses compact blinded paths (via SCIDs)
-	// Expected: No dummy hops; each path contains only the recipient.
+	// Expected: Padded to QR_CODED_PADDED_PATH_LENGTH for QR code size optimization
 	let default_router = DefaultMessageRouter::new(alice.network_graph, alice.keys_manager);
 
 	let compact_offer = alice.node
@@ -467,8 +478,8 @@ fn check_dummy_hop_pattern_in_offer() {
 
 	for path in compact_offer.paths() {
 		assert_eq!(
-			path.blinded_hops().len(), 1,
-			"Compact paths must include only the recipient"
+			path.blinded_hops().len(), QR_CODED_PADDED_PATH_LENGTH,
+			"Compact offer paths are padded to QR_CODED_PADDED_PATH_LENGTH"
 		);
 	}
 
@@ -480,10 +491,10 @@ fn check_dummy_hop_pattern_in_offer() {
 
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
-	assert!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
+	assert!(check_padded_path_length(&reply_path, alice, bob_id, PADDED_PATH_LENGTH));
 
 	// Case 2: NodeIdMessageRouter → uses node ID-based blinded paths
-	// Expected: 0 to MAX_DUMMY_HOPS_COUNT dummy hops, followed by recipient.
+	// Expected: Also padded to QR_CODED_PADDED_PATH_LENGTH for QR code size optimization
 	let node_id_router = NodeIdMessageRouter::new(alice.network_graph, alice.keys_manager);
 
 	let padded_offer = alice.node
@@ -492,7 +503,7 @@ fn check_dummy_hop_pattern_in_offer() {
 		.build().unwrap();
 
 	assert!(!padded_offer.paths().is_empty());
-	assert!(padded_offer.paths().iter().all(|path| path.blinded_hops().len() == PADDED_PATH_LENGTH));
+	assert!(padded_offer.paths().iter().all(|path| path.blinded_hops().len() == QR_CODED_PADDED_PATH_LENGTH));
 
 	let payment_id = PaymentId([2; 32]);
 	bob.node.pay_for_offer(&padded_offer, None, payment_id, Default::default()).unwrap();
@@ -502,7 +513,7 @@ fn check_dummy_hop_pattern_in_offer() {
 
 	assert_eq!(invoice_request.amount_msats(), Some(10_000_000));
 	assert_ne!(invoice_request.payer_signing_pubkey(), bob_id);
-	assert!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
+	assert!(check_padded_path_length(&reply_path, alice, bob_id, PADDED_PATH_LENGTH));
 }
 
 /// Checks that blinded paths are compact for short-lived offers.
@@ -687,7 +698,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!(check_compact_path_introduction_node(&reply_path, bob, charlie_id));
+	assert!(check_padded_path_length(&reply_path, bob, charlie_id, PADDED_PATH_LENGTH));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
 	charlie.onion_messenger.handle_onion_message(alice_id, &onion_message);
@@ -706,8 +717,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!(
-		check_compact_path_introduction_node(&reply_path, david, bob_id) ||
-		check_compact_path_introduction_node(&reply_path, david, charlie_id)
+		check_padded_path_length(&reply_path, david, bob_id, PADDED_PATH_LENGTH) ||
+		check_padded_path_length(&reply_path, david, charlie_id, PADDED_PATH_LENGTH)
 	);
 
 	route_bolt12_payment(david, &[charlie, bob, alice], &invoice);
@@ -790,7 +801,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!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
+	assert!(check_padded_path_length(&reply_path, alice, bob_id, PADDED_PATH_LENGTH));
 
 	route_bolt12_payment(david, &[charlie, bob, alice], &invoice);
 	expect_recent_payment!(david, RecentPaymentDetails::Pending, payment_id);
@@ -845,7 +856,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!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
+	assert!(check_padded_path_length(&reply_path, alice, bob_id, PADDED_PATH_LENGTH));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
@@ -857,7 +868,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!(check_compact_path_introduction_node(&reply_path, bob, alice_id));
+	assert!(check_padded_path_length(&reply_path, bob, alice_id, PADDED_PATH_LENGTH));
 
 	route_bolt12_payment(bob, &[alice], &invoice);
 	expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id);
@@ -913,7 +924,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!(check_compact_path_introduction_node(&reply_path, bob, alice_id));
+	assert!(check_padded_path_length(&reply_path, bob, alice_id, PADDED_PATH_LENGTH));
 
 	route_bolt12_payment(bob, &[alice], &invoice);
 	expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id);
@@ -1059,6 +1070,7 @@ fn send_invoice_requests_with_distinct_reply_path() {
 	let bob_id = bob.node.get_our_node_id();
 	let charlie_id = charlie.node.get_our_node_id();
 	let david_id = david.node.get_our_node_id();
+	let frank_id = nodes[6].node.get_our_node_id();
 
 	disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5], &nodes[6]]);
 	disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]);
@@ -1089,7 +1101,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!(check_compact_path_introduction_node(&reply_path, alice, charlie_id));
+	assert!(check_padded_path_length(&reply_path, alice, charlie_id, PADDED_PATH_LENGTH));
 
 	// Send, extract and verify the second Invoice Request message
 	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
@@ -1099,7 +1111,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!(check_compact_path_introduction_node(&reply_path, alice, nodes[6].node.get_our_node_id()));
+	assert!(check_padded_path_length(&reply_path, alice, frank_id, PADDED_PATH_LENGTH));
 }
 
 /// This test checks that when multiple potential introduction nodes are available for the payee,
@@ -1170,7 +1182,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!(check_compact_path_introduction_node(&reply_path, alice, charlie_id));
+	assert!(check_padded_path_length(&reply_path, alice, charlie_id, PADDED_PATH_LENGTH));
 
 	// Send, extract and verify the second Invoice Request message
 	let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
@@ -1179,7 +1191,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!(check_compact_path_introduction_node(&reply_path, alice, nodes[6].node.get_our_node_id()));
+	assert!(check_padded_path_length(&reply_path, alice, nodes[6].node.get_our_node_id(), PADDED_PATH_LENGTH));
 }
 
 /// Verifies that the invoice request message can be retried if it fails to reach the
@@ -1233,7 +1245,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!(check_compact_path_introduction_node(&reply_path, alice, bob_id));
+	assert!(check_padded_path_length(&reply_path, alice, bob_id, PADDED_PATH_LENGTH));
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap();
 	bob.onion_messenger.handle_onion_message(alice_id, &onion_message);
 
@@ -1534,7 +1546,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!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
+	assert!(check_padded_path_length(&reply_path, david, charlie_id, PADDED_PATH_LENGTH));
 
 	assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None);
 
@@ -1563,7 +1575,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!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
+	assert!(check_padded_path_length(&reply_path, david, charlie_id, PADDED_PATH_LENGTH));
 
 	assert_eq!(alice.onion_messenger.next_onion_message_for_peer(charlie_id), None);
 }
@@ -1663,7 +1675,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!(check_compact_path_introduction_node(&reply_path, david, charlie_id));
+	assert!(check_padded_path_length(&reply_path, david, charlie_id, PADDED_PATH_LENGTH));
 
 	let onion_message = alice.onion_messenger.next_onion_message_for_peer(charlie_id).unwrap();
 	charlie.onion_messenger.handle_onion_message(alice_id, &onion_message);

lightning/src/onion_message/messenger.rs

@@ -524,9 +524,11 @@ pub trait MessageRouter {
 
 /// A [`MessageRouter`] that can only route to a directly connected [`Destination`].
 ///
-/// [`DefaultMessageRouter`] constructs compact [`BlindedMessagePath`]s on a best-effort basis.
-/// That is, if appropriate SCID information is available for the intermediate peers, it will
-/// default to creating compact paths.
+/// [`DefaultMessageRouter`] tries to construct compact and padded [`BlindedMessagePath`]s based on
+/// the [`MessageContext`] given to [`MessageRouter::create_blinded_paths`]. That is, if the
+/// provided context implies the path may be used in a BOLT 12 object which might appear in a QR
+/// code, it reduces the amount of padding and prefers building compact paths when short channel
+/// IDs (SCIDs) are available for intermediate peers.
 ///
 /// # Compact Blinded Paths
 ///
@@ -545,7 +547,8 @@ pub trait MessageRouter {
 /// Creating [`BlindedMessagePath`]s may affect privacy since, if a suitable path cannot be found,
 /// it will create a one-hop path using the recipient as the introduction node if it is an announced
 /// node. Otherwise, there is no way to find a path to the introduction node in order to send a
-/// message, and thus an `Err` is returned.
+/// message, and thus an `Err` is returned. The impact of this may be somewhat muted when
+/// additional padding is added to the blinded path, but this protection is not complete.
 pub struct DefaultMessageRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref>
 where
 	L::Target: Logger,
@@ -555,14 +558,17 @@ where
 	entropy_source: ES,
 }
 
-// Target total length (in hops) for non-compact blinded paths.
-// We pad with dummy hops until the path reaches this length,
-// obscuring the recipient's true position.
+// Target total length (in hops) for blinded paths used outside of QR codes.
 //
-// Compact paths are optimized for minimal size, so we avoid
-// adding dummy hops to them.
+// We pad with dummy hops until the path reaches this length (including the recipient).
 pub(crate) const PADDED_PATH_LENGTH: usize = 4;
 
+// Target total length (in hops) for blinded paths which are included in objects which may appear
+// in a QR code.
+//
+// We pad with dummy hops until the path reaches this length (including the recipient).
+pub(crate) const QR_CODED_PADDED_PATH_LENGTH: usize = 2;
+
 impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref> DefaultMessageRouter<G, L, ES>
 where
 	L::Target: Logger,
@@ -574,12 +580,12 @@ where
 	}
 
 	pub(crate) fn create_blinded_paths_from_iter<
-		I: ExactSizeIterator<Item = MessageForwardNode>,
+		I: ExactSizeIterator<Item = MessageForwardNode> + Clone,
 		T: secp256k1::Signing + secp256k1::Verification,
 	>(
 		network_graph: &G, recipient: PublicKey, local_node_receive_key: ReceiveAuthKey,
 		context: MessageContext, peers: I, entropy_source: &ES, secp_ctx: &Secp256k1<T>,
-		compact_paths: bool,
+		never_compact_path: bool,
 	) -> Result<Vec<BlindedMessagePath>, ()> {
 		// Limit the number of blinded paths that are computed.
 		const MAX_PATHS: usize = 3;
@@ -592,6 +598,33 @@ where
 		let is_recipient_announced =
 			network_graph.nodes().contains_key(&NodeId::from_pubkey(&recipient));
 
+		let (mut compact_paths, padded_path_len) = match &context {
+			MessageContext::Offers(OffersContext::InvoiceRequest { .. })
+			|MessageContext::Offers(OffersContext::OutboundPaymentInRefund { .. }) => {
+				// When embeding blinded paths within BOLT 12 objects which are generally embedded
+				// in QR codes, we sadly need to be conservative about size, especially if the QR
+				// code ultimately also includes an on-chain address.
+				(true, QR_CODED_PADDED_PATH_LENGTH)
+			},
+			MessageContext::Offers(OffersContext::StaticInvoiceRequested { .. }) => {
+				// Async Payments aggressively embeds the entire `InvoiceRequest` in the payment
+				// onion. In a future version it should likely move to embedding only the
+				// `InvoiceRequest`-specific fields instead, but until then we have to be
+				// incredibly strict in the size of the blinded path we include in a static payment
+				// `Offer`.
+				(true, 0)
+			},
+			_ => {
+				// If there's no need to be small, pad the path more and never use SCID-based
+				// next-hops as they carry additional expiry risk.
+				(false, PADDED_PATH_LENGTH)
+			},
+		};
+
+		if never_compact_path {
+			compact_paths = false;
+		}
+
 		let has_one_peer = peers.len() == 1;
 		let mut peer_info = peers
 			.map(|peer| MessageForwardNode {
@@ -619,12 +652,8 @@ where
 		});
 
 		let build_path = |intermediate_hops: &[MessageForwardNode]| {
-			let dummy_hops_count = if compact_paths {
-				0
-			} else {
-				// Add one for the final recipient TLV
-				PADDED_PATH_LENGTH.saturating_sub(intermediate_hops.len() + 1)
-			};
+			// Calculate the dummy hops given the total hop count target (including the recipient).
+			let dummy_hops_count = padded_path_len.saturating_sub(intermediate_hops.len() + 1);
 
 			BlindedMessagePath::new_with_dummy_hops(
 				intermediate_hops,
@@ -651,12 +680,6 @@ where
 			}
 		}
 
-		// Sanity check: Ones the paths are created for the non-compact case, ensure
-		// each of them are of the length `PADDED_PATH_LENGTH`.
-		if !compact_paths {
-			debug_assert!(paths.iter().all(|path| path.blinded_hops().len() == PADDED_PATH_LENGTH));
-		}
-
 		if compact_paths {
 			for path in &mut paths {
 				path.use_compact_introduction_node(&network_graph);
@@ -740,13 +763,15 @@ where
 			peers.into_iter(),
 			&self.entropy_source,
 			secp_ctx,
-			true,
+			false,
 		)
 	}
 }
 
 /// This message router is similar to [`DefaultMessageRouter`], but it always creates
-/// full-length blinded paths, using the peer's [`NodeId`].
+/// non-compact blinded paths, using the peer's [`NodeId`]. It uses the same heuristics as
+/// [`DefaultMessageRouter`] for deciding when to pad the generated blinded paths with additional
+/// hops.
 ///
 /// This message router can only route to a directly connected [`Destination`].
 ///
@@ -755,7 +780,8 @@ where
 /// Creating [`BlindedMessagePath`]s may affect privacy since, if a suitable path cannot be found,
 /// it will create a one-hop path using the recipient as the introduction node if it is an announced
 /// node. Otherwise, there is no way to find a path to the introduction node in order to send a
-/// message, and thus an `Err` is returned.
+/// message, and thus an `Err` is returned. The impact of this may be somewhat muted when
+/// additional padding is added to the blinded path, but this protection is not complete.
 pub struct NodeIdMessageRouter<G: Deref<Target = NetworkGraph<L>>, L: Deref, ES: Deref>
 where
 	L::Target: Logger,
@@ -790,8 +816,11 @@ where
 
 	fn create_blinded_paths<T: secp256k1::Signing + secp256k1::Verification>(
 		&self, recipient: PublicKey, local_node_receive_key: ReceiveAuthKey,
-		context: MessageContext, peers: Vec<MessageForwardNode>, secp_ctx: &Secp256k1<T>,
+		context: MessageContext, mut peers: Vec<MessageForwardNode>, secp_ctx: &Secp256k1<T>,
 	) -> Result<Vec<BlindedMessagePath>, ()> {
+		for peer in peers.iter_mut() {
+			peer.short_channel_id = None;
+		}
 		DefaultMessageRouter::create_blinded_paths_from_iter(
 			&self.network_graph,
 			recipient,
@@ -800,7 +829,7 @@ where
 			peers.into_iter(),
 			&self.entropy_source,
 			secp_ctx,
-			false,
+			true,
 		)
 	}
 }