Multi-hop `BlindedPath` creation interface #2781

jkczyz · 2023-12-08T22:50:51Z

Expands Router and MessageRouter traits with methods for constructing BlindedPaths for payment and onion messaging, respectively. Also makes MessageRouter a super trait of Router so that ChannelManager doesn't need another parameter.

ChannelManager uses the new interface in its Offers flow to construct:

paths for Offers and Refunds,
reply paths for InvoiceRequest and Bolt12Invoice, and
payment paths for Bolt12Invoice.

DefaultMessageRouter and DefaultRouter implement the new methods by constructing three-hop blinded paths (i.e., where the counterparty of the peer is the introduction node).

Resolves #2690

jkczyz · 2023-12-08T22:51:09Z

@TheBlueMatt @valentinewallace Looking for high-level feedback on the new interface and how it is used by ChannelManager. I didn't implement the new methods for DefaultRouter or DefaultMessageRouter just yet. Instead, I simply provided trait-level defaults returning the naive two-hop blinded paths.

codecov-commenter · 2023-12-09T00:57:34Z

Codecov Report

Attention: 231 lines in your changes are missing coverage. Please review.

Comparison is base (c92db69) 88.59% compared to head (37319a6) 88.46%.

Files	Patch %	Lines
lightning/src/ln/channelmanager.rs	0.00%	80 Missing ⚠️
lightning/src/routing/router.rs	2.46%	79 Missing ⚠️
lightning/src/onion_message/messenger.rs	0.00%	34 Missing ⚠️
lightning/src/util/test_utils.rs	4.54%	21 Missing ⚠️
lightning/src/onion_message/functional_tests.rs	0.00%	8 Missing ⚠️
lightning/src/blinded_path/payment.rs	0.00%	6 Missing ⚠️
lightning/src/ln/features.rs	57.14%	3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2781      +/-   ##
==========================================
- Coverage   88.59%   88.46%   -0.14%     
==========================================
  Files         114      114              
  Lines       91561    91718     +157     
  Branches    91561    91718     +157     
==========================================
+ Hits        81120    81138      +18     
- Misses       7962     8103     +141     
+ Partials     2479     2477       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

TheBlueMatt

The API is fine, I think. A few minor comments.

TheBlueMatt · 2023-12-11T18:41:46Z

lightning/src/ln/features.rs

@@ -1068,7 +1068,7 @@ mod tests {
 			// Check that the flags are as expected:
 			// - option_data_loss_protect (req)
 			// - var_onion_optin (req) | static_remote_key (req) | payment_secret(req)
-			// - basic_mpp | wumbo | anchors_zero_fee_htlc_tx
+			// - basic_mpp | wumbo | option_anchors_zero_fee_htlc_tx


nit: drop the the^H^H^Hoption, its cleaner.

Mostly was aiming for internal consistency here and with BOLT 9, though I get why we don't use option in the identifier names.

TheBlueMatt · 2023-12-11T19:00:29Z

lightning/src/onion_message/messenger.rs

+	fn create_blinded_paths<
+		ES: EntropySource + ?Sized, T: secp256k1::Signing + secp256k1::Verification
+	>(
+		&self, recipient: PublicKey, peers: Vec<PublicKey>, count: usize, entropy_source: &ES,


Does the count need to be an argument here, or can the selected hop count just be up to the implementation?

Ah, I didn't spell it out but count is the number of BlindedPaths, not the number of hops which is left up to the implementation. Currently, we only use one path, though someone may want to add more paths to an Offer or Refund. We also use the same reply path when sending multiple InvoiceRequests, but could ask for more and cycle through whatever the MessageRouter returns.

Oh, sorry, I wrote that comment when I was confused, but figured it out later in the patchset. Still, I'm not sure we need count specified in either case - shouldn't we leave it up to the trait impl how many paths to return? We'll need to give them enough info to figure out what we're using it for, but I think that's true already just looking at it.

Router::create_blinded_payment_paths is used for Invoice paths, so presumably the implementation could decide, yeah.

Message::create_blinded_paths as of now just needs one as mentioned above. If we allow implementations to decide, are you saying if an implementation returns more than one then we should set all of them on an Offer or Refund (where QR code size may be a factor)? For reply paths, would we simply use an arbitrary (first?) one? Or even cycle through them in case of sending multiple InvoiceRequests?

Yea, I mean I don't have a concrete answer, it just seems like if we have an interface for it it'd be nice if we let the implementor decide (cause sometimes they're not going in a QR code, even offers). Maybe we set some enum/flag to say what its for/if a qr code may matter?

lightning/src/onion_message/messenger.rs

TheBlueMatt · 2023-12-11T19:05:08Z

lightning/src/routing/router.rs

+		&self, recipient: PublicKey, first_hops: Vec<ChannelDetails>, tlvs: ReceiveTlvs,
+		amount_msats: u64, count: usize, entropy_source: &ES, secp_ctx: &Secp256k1<T>
+	) -> Result<Vec<(BlindedPayInfo, BlindedPath)>, ()> {
+		first_hops.into_iter()


Same here, we probably need to check if the peer has at least one (or maybe three) public channel(s) before we select it as an introduction point.

That would require a NetworkGraph, which means we can't provide a default (i.e., provided) implementation. That's fine, but would mean anyone with their own Rotuer implementation now needs to implement this method. Unless we just have the provided one return an empty Vec, allowing ChannelManager to fallback to one-hop paths? So I guess you'd still expose your node id -- and counterparties for anyone looking at the NetworkGraph.

Yea, I think that's okay to not have a default impl. DefaultRouter kinda is that.

Ok, just noting others will need to implement this if they use their own Router implementation, possibly just wrapping DefaultRouter assuming they have a NetworkGraph on hand.

TheBlueMatt · 2023-12-11T19:07:16Z

Tentatively giving this an 0.0.119 tag to indicate to reviewers to focus on it, but I kinda doubt we'll get it over the line in two days.

jkczyz

PTAL. I think expanding the number of hops by one shouldn't be too difficult.

jkczyz · 2023-12-12T02:32:40Z

lightning/src/routing/router.rs

+					&[forward_node], recipient, tlvs.clone(), u64::MAX, entropy_source, secp_ctx
+				)
+			})
+			.take(MAX_PAYMENT_PATHS)


Chose to limit to three paths in the implementation for DefaultRouter. I think we want some limit to avoid computing too many BlindedPaths.

Wondering if should increase this substantially to avoid failed payments. Though, maybe the best strategy is to sort ascending all possible paths by scorer penalty and take some N.

lightning/src/onion_message/messenger.rs

jkczyz · 2023-12-12T02:34:39Z

lightning/src/ln/features.rs

@@ -1068,7 +1068,7 @@ mod tests {
 			// Check that the flags are as expected:
 			// - option_data_loss_protect (req)
 			// - var_onion_optin (req) | static_remote_key (req) | payment_secret(req)
-			// - basic_mpp | wumbo | anchors_zero_fee_htlc_tx
+			// - basic_mpp | wumbo | option_anchors_zero_fee_htlc_tx


Mostly was aiming for internal consistency here and with BOLT 9, though I get why we don't use option in the identifier names.

lightning/src/routing/router.rs

valentinewallace · 2023-12-12T18:05:33Z

lightning/src/routing/router.rs

+			.filter(|details| amount_msats >= details.inbound_htlc_minimum_msat.unwrap_or(0))
+			.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(0))
+			.map(|details| {
+				let short_channel_id = details.get_inbound_payment_scid().unwrap();


I don't think we should unwrap here and below.

IIUC, a usable channel will always have these set. I could add debug assertions or additional filters to be safe, if you prefer.

Yeah, while unlikely this panic is reachable from the public interface so I think we should filter.

lightning/src/ln/channelmanager.rs

valentinewallace · 2023-12-12T18:47:20Z

lightning/src/routing/router.rs

@@ -78,10 +83,80 @@ impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Sco
 			&random_seed_bytes
 		)
 	}
+
+	fn create_blinded_payment_paths<


Given we have the amount, we probably want to use something more like lightning-invoice's sort_and_filter_channels for this method, at least eventually. Currently the sum of inbound capacity of the paths we're returning may not be sufficient, IIUC.

Given we have the amount, we probably want to use something more like lightning-invoice's sort_and_filter_channels for this method, at least eventually.

Hmm... perhaps eventually. Unlike in lightning-invoice, these are meant to be used immediately, so the channels should be live at very least when going to pay. We may want to prefer channels based on their available liquidity, which would require a collect and sort. I was trying to avoid this given this action can be initiated by anyone sending an onion message.

Currently the sum of inbound capacity of the paths we're returning may not be sufficient, IIUC.

Added the check against inbound capacity for now as that was indeed missing. Currently, requiring that every channel has enough. I suppose if this constraint cannot be met we could relax it to a fraction of the amount based on MAX_PAYMENT_PATHS?

I was trying to avoid this given this action can be initiated by anyone sending an onion message.

The action can be initiated by anyone only if this node has a public offer, is that right?

Another thing to note is that if we include one of our channels that is public but doesn't have enough confirmations to be in the public network graph, we'll be revealing to our counterparty that we are the likely destination for the payment. This is something that sort_and_filter_channels takes care of, and based on the docs for the method it also takes other considerations into account that seem relevant here. This may not be a big concern for v1, though.

The action can be initiated by anyone only if this node has a public offer, is that right?

Yeah, true.

Another thing to note is that if we include one of our channels that is public but doesn't have enough confirmations to be in the public network graph, we'll be revealing to our counterparty that we are the likely destination for the payment.

Does list_usable_channels mostly avoid this? I believe sort_and_filter_channels doesn't assume all channels passed are usable. Wouldn't our counterparty know the payment is likely for us if the channel is unannounced as well?

This is something that sort_and_filter_channels takes care of, and based on the docs for the method it also takes other considerations into account that seem relevant here. This may not be a big concern for v1, though.

Hmm... yeah there is some overlap there, but I think the scenarios are also different since the introduction nodes aren't our counterparties here whereas for sort_and_filter_channels the hints are. So the filtering and selection criteria aren't necessarily going to be the same as there.

Also, as mentioned elsewhere, we probably need to do some sorting after all so the payer knows which paths to prefer (e.g., ones where we have higher confidence that the hop from the introduction node has enough liquidity).

Does list_usable_channels mostly avoid this? I believe sort_and_filter_channels doesn't assume all channels passed are usable.

I believe list_usable_channels just checks that the channel is funded, not how many confirmations it has.

Wouldn't our counterparty know the payment is likely for us if the channel is unannounced as well?

Yeah, so if we have public and private channels we should prefer the public ones in blinded payment paths we return, IIUC.

So the filtering and selection criteria aren't necessarily going to be the same as there.

With you there, just saying we'll probably want to see which of its checks are relevant for us when we add sorting.

I believe list_usable_channels just checks that the channel is funded, not how many confirmations it has.

AFAICT, it goes it is consider usable when both parties sent channel_ready, which happens once the funding transaction has enough confirmations.

Yeah, so if we have public and private channels we should prefer the public ones in blinded payment paths we return, IIUC.

I'm not sure I understand why. They are blinded from the payer, so the concern we have with invoice hints doesn't apply. Maybe our counterparty learns something if we have both public and private channels with them and the payer uses MPP over both? Let me know if I'm missing something.

Maybe our counterparty learns something if we have both public and private channels with them and the payer uses MPP over both? Let me know if I'm missing something.

I was thinking that the counterparty can be fairly confident that we're the recipient if a private channel is used, but if public channels are used then we have more deniability. Privacy concerns aren't too high priority rn though, as discussed offline.

jkczyz

Added another hop to the blinded payment paths.

jkczyz · 2023-12-12T21:09:18Z

lightning/src/routing/router.rs

@@ -78,10 +83,80 @@ impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Sco
 			&random_seed_bytes
 		)
 	}
+
+	fn create_blinded_payment_paths<


Given we have the amount, we probably want to use something more like lightning-invoice's sort_and_filter_channels for this method, at least eventually.

Hmm... perhaps eventually. Unlike in lightning-invoice, these are meant to be used immediately, so the channels should be live at very least when going to pay. We may want to prefer channels based on their available liquidity, which would require a collect and sort. I was trying to avoid this given this action can be initiated by anyone sending an onion message.

Currently the sum of inbound capacity of the paths we're returning may not be sufficient, IIUC.

Added the check against inbound capacity for now as that was indeed missing. Currently, requiring that every channel has enough. I suppose if this constraint cannot be met we could relax it to a fraction of the amount based on MAX_PAYMENT_PATHS?

jkczyz · 2023-12-12T21:13:56Z

lightning/src/routing/router.rs

+			.filter(|details| amount_msats >= details.inbound_htlc_minimum_msat.unwrap_or(0))
+			.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(0))
+			.map(|details| {
+				let short_channel_id = details.get_inbound_payment_scid().unwrap();


IIUC, a usable channel will always have these set. I could add debug assertions or additional filters to be safe, if you prefer.

valentinewallace · 2023-12-13T17:33:12Z

lightning/src/routing/router.rs

+	>(
+		&self, recipient: PublicKey, first_hops: Vec<ChannelDetails>, tlvs: ReceiveTlvs,
+		amount_msats: u64, entropy_source: &ES, secp_ctx: &Secp256k1<T>
+	) -> Result<Vec<(BlindedPayInfo, BlindedPath)>, ()> {


Are you planning to add some testing for this method/others? I guess e2e tests are also an option now that #2688 landed.

Yeah, I need to do that but haven't decided on exactly how given the returned paths are blinded. Would e2e test be possible if we can't forward on the blinded path? Otherwise, might be better to refactor out the filtering / sorting logic and test that independently.

e2e would be possible with 2-hop blinded payment paths but not 3 :/

valentinewallace · 2023-12-13T20:02:00Z

lightning/src/routing/router.rs

+					}
+				};
+				BlindedPath::new_for_payment(
+					&[introduction_forward_node, counterparty_forward_node], recipient,


It seems potentially likely for users with some LDKServer<>LDKClient architecture that the counterparty will support route blinding, but none of the counterparty's public counterparties do. I don't think we should fail to construct a path in that situation?

How does the architecture play a role here? The same could be said for a standard one, no? In that case, we fallback to the one-hop path. Could do two-hop fallback before that?

How does the architecture play a role here?

The architecture/situation outlined seems like a somewhat common LDK use case where 3-hop wouldn't work, and 1-hop also wouldn't work (due to the client likely being private).

To me it would make more sense if we always tried for 2-hop in this method, and fall back to 1-hop if the recipient is a public node, otherwise error. Then we wouldn't need to have 1-hop fallback logic in channelmanager as well. I may be missing something here, though.

How does the architecture play a role here?

The architecture/situation outlined seems like a somewhat common LDK use case where 3-hop wouldn't work, and 1-hop also wouldn't work (due to the client likely being private).

Still a little confused about this. I thought you were talking about splitting an LDK node. But are you saying LDKServer is an LSP node and LSPClient is a separate node with a private channel with the LSP?

To me it would make more sense if we always tried for 2-hop in this method, and fall back to 1-hop if the recipient is a public node, otherwise error. Then we wouldn't need to have 1-hop fallback logic in channelmanager as well. I may be missing something here, though.

When would we try 3-hops? Are you thinking something like this?

let paths = if has_public_channels { create_three_hop_paths() } else { create_two_hop_paths() }; if !paths.is_empty() { paths } else if has_public_channels { create_one_hop_path() } else { Err(()) }

Still a little confused about this. I thought you were talking about splitting an LDK node. But are you saying LDKServer is an LSP node and LSPClient is a separate node with a private channel with the LSP?

That was my understanding - an LSP and a client where the LSP has no route-blinding-supporting peers, but a client that wants a blinded path (through the LSP, cause they have no public channels).

We should still presumably try 3 hop in that case, but if we fail (cause the LSP has no blinding-supporting peers) we should build a 2 hop.

See #2781 (comment)

valentinewallace · 2023-12-13T20:08:19Z

lightning/src/routing/router.rs

@@ -78,10 +83,80 @@ impl< G: Deref<Target = NetworkGraph<L>>, L: Deref, S: Deref, SP: Sized, Sc: Sco
 			&random_seed_bytes
 		)
 	}
+
+	fn create_blinded_payment_paths<


Does list_usable_channels mostly avoid this? I believe sort_and_filter_channels doesn't assume all channels passed are usable.

I believe list_usable_channels just checks that the channel is funded, not how many confirmations it has.

Wouldn't our counterparty know the payment is likely for us if the channel is unannounced as well?

Yeah, so if we have public and private channels we should prefer the public ones in blinded payment paths we return, IIUC.

So the filtering and selection criteria aren't necessarily going to be the same as there.

With you there, just saying we'll probably want to see which of its checks are relevant for us when we add sorting.

valentinewallace · 2023-12-13T21:44:42Z

lightning/src/routing/router.rs

+				)
+			})
+			.take(MAX_PAYMENT_PATHS)
+			.collect()


Seems like this should error if no paths are found?

We treat these the same in ChannelManager by falling back to 1-hop paths. Not sure if we should move that code here, but failing when we don't have public channels means we can't send an invoice, FWIW.

I guess to me it seems like a feature that we'll fail to send an invoice if we're a private node and none of our channel peers support route blinding, because the payer would most likely have no way to find a route to our 1-hop blinded path anyway. Also nice to keep the fallback code out of channelmanager when it seems more like the domain of the router. Not sure it's worth holding up the PR over.

Moved fallback code to the respective default implementations, FYI.

TheBlueMatt · 2023-12-14T00:48:12Z

i kinda wonder if we shouldn't max out at 2 hops for now. Its not great for privacy, but until we use scoring data in our blinding selection logic i fear we're just gonna always fail to receive, which is just gonna kill B12 adoption until we fix it.

jkczyz · 2023-12-14T03:52:17Z

i kinda wonder if we shouldn't max out at 2 hops for now. Its not great for privacy, but until we use scoring data in our blinding selection logic i fear we're just gonna always fail to receive, which is just gonna kill B12 adoption until we fix it.

Discussed offline. We'll use 2-hop paths for now and do 3-hop paths in another PR. Moved relevant commits to #2793 and squashed.

TheBlueMatt · 2023-12-14T21:22:50Z

lightning/src/ln/channelmanager.rs

+	}
+
+	/// Creates a one-hop blinded payment path with [`ChannelManager::get_our_node_id`] as the
+	/// introduction node.
 	fn create_one_hop_blinded_payment_path(


We currently dont fail create_offer_builder if we have no public channels and fail to build a multi-hop path, but ISTM this (and create_one_hop_blinded_path) should fail if we don't have any public channels.

So now that this is moved to Router (and MessageRouter, respectively), we can check there but only in Router. In MessageRouter, we don't have channel information.

Oh, actually, we can just use the NetworkGraph to see if our node was announced.

lightning/src/routing/router.rs

TheBlueMatt · 2023-12-14T21:29:22Z

lightning/src/routing/router.rs

+				let payment_relay: PaymentRelay = details.counterparty.forwarding_info.unwrap().into();
+				let payment_constraints = PaymentConstraints {
+					max_cltv_expiry: tlvs.payment_constraints.max_cltv_expiry
+						+ payment_relay.cltv_expiry_delta as u32,


We need to use some kind of max here to not expose esoteric CLTV expiry deltas.

Done. Let me know if that is what you were thinking.

TheBlueMatt

LGTM, with one nit about needing docs.

lightning/src/ln/channelmanager.rs

TheBlueMatt · 2023-12-15T05:25:33Z

lightning/src/routing/router.rs

+					None => return None,
+				};
+				let payment_relay: PaymentRelay = match details.counterparty.forwarding_info {
+					Some(forwarding_info) => forwarding_info.into(),


Oops, also the fees here and the CLTV here need normalizing. Maybe this doesn't belong in the router but in new_for_payment? I'm fine leaving this for a followup in 120 if you prefer.

Will leave to a follow-up.

jkczyz · 2023-12-15T15:10:00Z

lightning/src/routing/router.rs

@@ -99,7 +99,6 @@ impl<G: Deref<Target = NetworkGraph<L>> + Clone, L: Deref, S: Deref, SP: Sized,

 		let network_graph = self.network_graph.deref().read_only();
 		let paths = first_hops.into_iter()
-			.filter(|details| details.is_public)


Needed to drop this since we want two-hop paths that contain an unannounced channel between the intro node and us.

jkczyz · 2023-12-15T15:18:48Z

I wrote a test in #2697, but I'm getting the following failure when making the payment (debugging added):

[lightning/src/ln/onion_payment.rs:46] inbound_amt_msat = 10001000
[lightning/src/ln/onion_payment.rs:46] payment_constraints.htlc_minimum_msat = 1000
[lightning/src/ln/onion_payment.rs:46] dbg!(inbound_amt_msat) < dbg!(payment_constraints.htlc_minimum_msat) = false
[lightning/src/ln/onion_payment.rs:47] outgoing_cltv_value = 259
[lightning/src/ln/onion_payment.rs:47] payment_constraints.max_cltv_expiry = 93
[lightning/src/ln/onion_payment.rs:47] dbg!(outgoing_cltv_value) > dbg!(payment_constraints.max_cltv_expiry) = true
node 1 INFO [lightning::ln::onion_payment:393]         Failed to accept/forward incoming HTLC: Underflow calculating outbound amount or cltv value for blinded forward

So the CLTV check is causing the failure: https://github.com/jkczyz/rust-lightning/blob/181e621d26f9454c1e4726b7a923b9c8e26addf2/lightning/src/ln/onion_payment.rs#L47

Can be reproduced on that PR using:

cargo test -p lightning -- creates_and_pays_for_offer_using_two_hop_blinded_path

@valentinewallace Do you see anything wrong in the test setup or how I'm calculating the payment constraints? Note that I haven't rebased on your current work yet, so maybe that is it?

valentinewallace · 2023-12-15T16:01:35Z

lightning/src/ln/channelmanager.rs

 		let entropy_source = self.entropy_source.deref();
 		let secp_ctx = &self.secp_ctx;

+		let first_hops = self.list_usable_channels();
 		let payee_node_id = self.get_our_node_id();
 		let max_cltv_expiry = self.best_block.read().unwrap().height() + LATENCY_GRACE_PERIOD_BLOCKS;


Looks like this doesn't allow for the router adding the shadow offset to the CLTV during route construction. Will probably want to add MAX_SHADOW_CLTV_EXPIRY_DELTA_OFFSET (from router.rs) to this value.

Hmm... but the BlindedPath is created by the recipient, and the Route is created by the payer. Is that value common across implementations? Or just something we expect to be good enough?

True. I think it will be good enough, but this does mean that our router should be taking max_cltv_expiry into account and currently isn't :/

Looks like this doesn't allow for the router adding the shadow offset to the CLTV during route construction. Will probably want to add MAX_SHADOW_CLTV_EXPIRY_DELTA_OFFSET (from router.rs) to this value.

Thinking more, currently the way we set max_cltv_expiry here doesn't allow for any CLTV delta along the route, not just the shadow offset. So I think the value we set here should increase even further, actually.
Never mind, nodes subtract their delta at each payment hop...

True. I think it will be good enough, but this does mean that our router should be taking max_cltv_expiry into account and currently isn't :/

Ok took me a minute to reload all the context here. max_cltv_expiry isn't part of BlindedPayInfo so there's no way to take it into account when pathfinding. This confused me originally so I opened lightning/bolts#1069 to clarify it. But I don't think we should stop using the shadow offset in pathfinding just because invoice expiries tend to be short, so I'm not exactly sure what the resolution is here.

Discussed offline to add CLTV_FAR_FAR_AWAY.

lightning/src/routing/router.rs

Move AnchorsZeroFeeHtlcTx after Wumbo to keep order by feature bit. Also, update setting order and comment in tests.

The RouteBlinding feature flag is signals support for relaying payments over blinded paths. It is used for paying BOLT 12 invoices, which are required to included at least one blinded path.

The MessageRouter trait is used to find an OnionMessagePath to a Destination (e.g., to a BlindedPath). Expand the interface with a create_blinded_paths method for creating such paths to a recipient. Provide a default implementation creating two-hop blinded paths where the recipient's peers serve as introduction nodes.

ChannelManager is parameterized by a Router in order to find routes when sending and retrying payments. For the offers flow, it needs to be able to construct blinded paths (e.g., in the offer and in reply paths). Instead of adding yet another parameter to ChannelManager, require that any Router also implements MessageRouter. Implement this for DefaultRouter by delegating to a DefaultMessageRouter.

valentinewallace · 2023-12-15T21:14:12Z

lightning/src/ln/channelmanager.rs

 		let entropy_source = self.entropy_source.deref();
 		let secp_ctx = &self.secp_ctx;

+		let first_hops = self.list_usable_channels();
 		let payee_node_id = self.get_our_node_id();
 		let max_cltv_expiry = self.best_block.read().unwrap().height() + LATENCY_GRACE_PERIOD_BLOCKS;


This should also account for the invoice expiry, IIUC. We default to 60 minutes right now, so I think we should add 6 blocks on for that (assuming 10 minute blocks), per lightning/bolts#1069.

Discussed offline to add CLTV_FAR_FAR_AWAY.

When finding a route through a blinded path, a random CLTV offset may be added to the path in order to preserve privacy. This needs to be accounted for in the blinded path's PaymentConstraints. Add CLTV_FAR_FAR_AWAY to the max_cltv_expiry constraint to allow for such offsets.

When constructing blinded paths for Offer and Refund, delegate to MessageRouter::create_blinded_paths which may produce multi-hop blinded paths. Fallback to one-hop blinded paths if the MessageRouter fails or returns no paths. Likewise, do the same for InvoiceRequest and Bolt12Invoice reply paths.

The Router trait is used to find a Route for paying a node. Expand the interface with a create_blinded_payment paths method for creating such paths to a recipient node. Provide an implementation for DefaultRouter that creates two-hop blinded paths where the recipient's peers serve as the introduction nodes.

When constructing blinded payment paths for Bolt12Invoice, delegate to Router::create_blinded_payment_paths which may produce multi-hop blinded paths. Fallback to one-hop blinded paths if the Router fails or returns no paths.

To avoid exposing a node's identity in a blinded path, only create one-hop blinded paths if the node has been announced, and thus has public channels. Otherwise, there is no way to route a payment to the node, exposing its identity needlessly.

jkczyz · 2023-12-15T21:43:49Z

Rebased and squashed.

TheBlueMatt

Oops, two issues, but I can fix in #2798.

TheBlueMatt · 2023-12-15T22:26:49Z

lightning/src/routing/router.rs

+			.filter(|details| details.counterparty.features.supports_route_blinding())
+			.filter(|details| amount_msats <= details.inbound_capacity_msat)
+			.filter(|details| amount_msats >= details.inbound_htlc_minimum_msat.unwrap_or(0))
+			.filter(|details| amount_msats <= details.inbound_htlc_maximum_msat.unwrap_or(0))


Oops, this should read unwrap_or(u64::max_value()).

TheBlueMatt · 2023-12-15T22:27:48Z

lightning/src/routing/router.rs

+						features: BlindedHopFeatures::empty(),
+					},
+					node_id: details.counterparty.node_id,
+					htlc_maximum_msat: details.inbound_htlc_maximum_msat.unwrap_or(0),


jkczyz force-pushed the 2023-09-multihop-paths branch from a82bc73 to 5a9297b Compare December 9, 2023 00:28

TheBlueMatt reviewed Dec 11, 2023

View reviewed changes

TheBlueMatt added this to the 0.0.119 milestone Dec 11, 2023

jkczyz force-pushed the 2023-09-multihop-paths branch from 5a9297b to c5adf74 Compare December 12, 2023 02:28

jkczyz commented Dec 12, 2023

View reviewed changes

valentinewallace reviewed Dec 12, 2023

View reviewed changes

jkczyz force-pushed the 2023-09-multihop-paths branch from c5adf74 to ec160f7 Compare December 12, 2023 21:14

jkczyz commented Dec 12, 2023

View reviewed changes

jkczyz force-pushed the 2023-09-multihop-paths branch 2 times, most recently from 0e33921 to 2b8170b Compare December 13, 2023 02:57

jkczyz marked this pull request as ready for review December 13, 2023 02:57

valentinewallace reviewed Dec 13, 2023

View reviewed changes

jkczyz force-pushed the 2023-09-multihop-paths branch from 2b8170b to fad9dbd Compare December 14, 2023 03:47

jkczyz mentioned this pull request Dec 14, 2023

Three-hop blinded paths #2793

Draft

jkczyz mentioned this pull request Dec 14, 2023

Functional tests for BOLT 12 Offers payment flow #2697

Merged

TheBlueMatt reviewed Dec 14, 2023

View reviewed changes

jkczyz force-pushed the 2023-09-multihop-paths branch from fad9dbd to fcd8a11 Compare December 15, 2023 00:16

TheBlueMatt reviewed Dec 15, 2023

View reviewed changes

jkczyz force-pushed the 2023-09-multihop-paths branch from c4ba79f to cc798a8 Compare December 15, 2023 15:07

jkczyz commented Dec 15, 2023

View reviewed changes

valentinewallace reviewed Dec 15, 2023

View reviewed changes

lightning/src/routing/router.rs Show resolved Hide resolved

jkczyz added 5 commits December 15, 2023 14:52

Re-order AnchorsZeroFeeHtlcTx in module docs/tests

ee026d4

Move AnchorsZeroFeeHtlcTx after Wumbo to keep order by feature bit. Also, update setting order and comment in tests.

Add RouteBlinding feature flag

ffb0d83

The RouteBlinding feature flag is signals support for relaying payments over blinded paths. It is used for paying BOLT 12 invoices, which are required to included at least one blinded path.

Fix create_one_hop_blinded_payment_path docs

c558ccd

valentinewallace reviewed Dec 15, 2023

View reviewed changes

jkczyz added 9 commits December 15, 2023 15:37

Fix broken doc link in create_refund_builder

4da0862

CounterpartyForwardingInfo to PaymentRelay mapping

edb5892

Multi-hop blinded payment paths in ChannelManager

827833c

When constructing blinded payment paths for Bolt12Invoice, delegate to Router::create_blinded_payment_paths which may produce multi-hop blinded paths. Fallback to one-hop blinded paths if the Router fails or returns no paths.

DRY up OffersMessage::InvoiceRequest handling

16ee240

Fix build warning

37319a6

jkczyz force-pushed the 2023-09-multihop-paths branch from cc798a8 to 37319a6 Compare December 15, 2023 21:43

TheBlueMatt approved these changes Dec 15, 2023

View reviewed changes

valentinewallace approved these changes Dec 15, 2023

View reviewed changes

TheBlueMatt reviewed Dec 15, 2023

View reviewed changes

TheBlueMatt merged commit b9797eb into lightningdevkit:main Dec 15, 2023
12 of 15 checks passed

TheBlueMatt mentioned this pull request Dec 15, 2023

Small API cleanups pre-0.0.119 #2798

Merged

Multi-hop BlindedPath creation interface #2781

Multi-hop BlindedPath creation interface #2781

Conversation

jkczyz commented Dec 8, 2023 • edited

jkczyz commented Dec 8, 2023

codecov-commenter commented Dec 9, 2023 • edited

Codecov Report

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt Dec 11, 2023 • edited

Choose a reason for hiding this comment

jkczyz Dec 11, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt Dec 11, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Dec 11, 2023

jkczyz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Dec 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Dec 14, 2023

jkczyz commented Dec 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkczyz commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valentinewallace Dec 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Multi-hop `BlindedPath` creation interface #2781

Multi-hop `BlindedPath` creation interface #2781

jkczyz commented Dec 8, 2023 •

edited

codecov-commenter commented Dec 9, 2023 •

edited

TheBlueMatt Dec 11, 2023 •

edited

jkczyz Dec 11, 2023 •

edited

TheBlueMatt Dec 11, 2023 •

edited

valentinewallace Dec 13, 2023 •

edited

valentinewallace Dec 15, 2023 •

edited