Consider allowing for BOLT12 payments over unannounced channels

[tnull]

Currently, we only allow for BOLT12 payments if the introduction point is an announced node both payer and payee are connected to. It is my understanding that this is at least partially intentional to discourage leaking the payer's IP to the payee.

However, this makes for an unexpected UX where users could send payments via an intermediate node, but not via a directly-connected private channel.

It would be nice if we could reconsider if we could relax some of the checks along the lines of tnull@8138bec, or, at least allow the user to opt into private-channel BOLT12 payments via a config option.

[TheBlueMatt]

The proposed patch would break the general case - we'd generate blinded paths starting with a private node and give those out publicly (which most nodes wouldn't be able to pay). Indeed, if we have no peers with public channels (?????) we should maybe not spuriously fail, but I don't think that would quite address the issue you're talking about here (where we have public channels, but also a private one with another node).

I'm not sure how we could handle this case properly. If the peer gives us a one-hop reply path we could detect it explicitly and give them a one-hop payment path, but that assumes they give us a one-hop reply path, and I'm not sure we even want to - a user receiving a BOLT12 payment may have some expectation for marginal privacy, so we'd be breaking it for some subset of senders, but that's fairly surprising.

[tnull]

The proposed patch would break the general case - we'd generate blinded paths starting with a private node and give those out publicly (which most nodes wouldn't be able to pay). Indeed, if we have no peers with public channels (?????) we should maybe not spuriously fail, but I don't think that would quite address the issue you're talking about here (where we have public channels, but also a private one with another node).

Yes, the linked patch was just a quick fix I applied to the experimental branch used for the recent LDK Node hackathon to easily enable BOLT12 sending in ~local regtest/signet setups. Always requiring the introduction point to be announced is pretty confusing in these kind of scenarios, especially if we have a direct (possibly even public) channel open but fail the payment due to not having received the node announcement yet. And of course, if the node was configured to use RGS, it would always ignore that node announcement, so even if the introduction point is announced it first has to be picked up and included in the next RGS snapshot.

I'm not sure how we could handle this case properly. If the peer gives us a one-hop reply path we could detect it explicitly and give them a one-hop payment path, but that assumes they give us a one-hop reply path, and I'm not sure we even want to - a user receiving a BOLT12 payment may have some expectation for marginal privacy, so we'd be breaking it for some subset of senders, but that's fairly surprising.

Right, I'm also not entirely sure how to solve this, but I think we need to do something, as there are just a lot of requirements and assumptions here that need to go the right way to allow a BOLT12 payment to be successfully sent, and in every other scenario we would fail the payment, often not returning an error at all (i.e., when it fails in the flow somewhere after the initial step constructing an invoice request).

[TheBlueMatt]

in ~local regtest/signet setups

For these we could simply relax the requirement that you have a public channel? Maybe that would suffice in many cases.

And of course, if the node was configured to use RGS, it would always ignore that node announcement, so even if the introduction point is announced it first has to be picked up and included in the next RGS snapshot.

I imagine in most real cases this won't be an issue? You are generally opening a channel with an LSP who has been around for a while. We should definitely improve the error messaging in this case, though, because we can control this top-to-bottom for offer/invoice_request generation (its all local, not messages going into a blackhole) we can provide a useful error message that describes some common cases.

For invoice generation we should also maybe consider some kind of error sentinel - eg an Event that we only generate once indicating we're failing to generate invoices and inbound payments will fail with a useful error message. We'd only want to do it once and not on every invoice_request we receive, though.

[tnull]

Still think we should find a solution here as I keep getting reports of BOLT12 early adopters that run into this while testing. (cc @jkczyz)

[TheBlueMatt]

Once we can build multiple paths, we can maybe detect this situation (node with only private channels and no known-public peers) and include both a one hop and two hop hint.

[TheBlueMatt]

This might end up actually mattering beyond test suites, too, as we'll end up in the same situation (as far as we know) if we're running without a network graph at all. I kinda wonder if we shouldn't just always pick a peer and announce with that as an intro node if we're private and we don't think we have a network graph (or its really small).

[jkczyz]

Once we can build multiple paths, we can maybe detect this situation (node with only private channels and no known-public peers) and include both a one hop and two hop hint.

Question is which peer if we have more than one? Presumably, in this situation we are paying a peer, so we aren't revealing anything to the peer by using a one-hop path. But with a two-hop path we are potentially revealing another peer.

This might end up actually mattering beyond test suites, too, as we'll end up in the same situation (as far as we know) if we're running without a network graph at all. I kinda wonder if we shouldn't just always pick a peer and announce with that as an intro node if we're private and we don't think we have a network graph (or its really small).

Is this essentially the case where our only peers are LSPs? Maybe we consider all peers when we are unannounced, still preferring those with more channels. Then if we have an empty NetworkGraph, those peers still can be chosen.

diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs
index 1f3f5a1fa..369a12243 100644
--- a/lightning/src/blinded_path/message.rs
+++ b/lightning/src/blinded_path/message.rs
@@ -28,11 +28,11 @@ use crate::util::ser::{FixedLengthReader, LengthReadableArgs, Writeable, Writer}
 
 use core::mem;
 use core::ops::Deref;
 
 /// An intermediate node, and possibly a short channel id leading to the next node.
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
 pub struct ForwardNode {
        /// This node's pubkey.
        pub node_id: PublicKey,
        /// The channel between `node_id` and the next hop. If set, the constructed [`BlindedHop`]'s
        /// `encrypted_payload` will use this instead of the next [`ForwardNode::node_id`] for a more
diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs
index ee49d00e9..702757242 100644
--- a/lightning/src/onion_message/messenger.rs
+++ b/lightning/src/onion_message/messenger.rs
@@ -502,12 +502,13 @@ where
                let mut peer_info = peers
                        // Limit to peers with announced channels
                        .filter_map(|peer|
                                network_graph
                                        .node(&NodeId::from_pubkey(&peer.node_id))
-                                       .filter(|info| info.channels.len() >= MIN_PEER_CHANNELS)
+                                       .filter(|info| !is_recipient_announced || info.channels.len() >= MIN_PEER_CHANNELS)
                                        .map(|info| (peer, info.is_tor_only(), info.channels.len()))
+                                       .or_else(|| (!is_recipient_announced).then(|| (peer, false, 0)))
                        )
                        // Exclude Tor-only nodes when the recipient is announced.
                        .filter(|(_, is_tor_only, _)| !(*is_tor_only && is_recipient_announced))
                        .collect::<Vec<_>>();

[tnull]

Question is which peer if we have more than one?

Yes, I also wonder if we expect this scenario to happen often in conjunction with QR code usage. That is to say that we may consider relaxing some of the size-constraints and even include more than one introduction node, which at least from a reliability perspective might improve things considerably.

Presumably, in this situation we are paying a peer, so we aren't revealing anything to the peer by using a one-hop path. But with a two-hop path we are potentially revealing another peer.

I'm confused - do you mean 'we're getting paid by a peer', i.e., wouldn't the payee leak its peers if they include any directly-connected peers (or even themselves) as their introduction point?

Or, if you indeed meant 'we are paying a peer', why would paying via two-hop blinded path leak another peer and to whom?

[jkczyz]

Ah, sorry I meant "we're getting paid by a peer". Including a one-hop path leaks that payee's node id, which the payer already has. So they learn they are paying their own peer (if they didn't already know that). With both a one-hop and a two-hop path, the payee is leaking that and potentially another peer unless they happen to choose the payer from their set of peers.

[TheBlueMatt]

Is this essentially the case where our only peers are LSPs? Maybe we consider all peers when we are unannounced, still preferring those with more channels. Then if we have an empty NetworkGraph, those peers still can be chosen.

Yea, that's what I was thinking. One peer (our LSP) and no network graph so we don't know that our peer is public. I agree we should maybe just consider all peers in that case.

[jkczyz]

Another issue that we'll need to solve is paying through a BlindedPath where we are the introduction node. For onion messages, we'll advance the path by one:

rust-lightning/lightning/src/onion_message/messenger.rs

Lines 850 to 853 in b8cdde8

	if introduction_node_id == our_node_id {
	advance_path_by_one(blinded_path, node_signer, node_id_lookup, &secp_ctx)
	.map_err(|()| SendError::BlindedPathAdvanceFailed)?;
	}

But for payment paths, we don't yet have the equivalent.

rust-lightning/lightning/src/routing/router.rs

Lines 1936 to 1937 in b8cdde8

	} else if *introduction_node_id == Some(&our_node_id) {
	log_info!(logger, "Got blinded path with ourselves as the introduction node, ignoring");

Ran across this when trying to write a test for the two unannounced nodes case.

[jkczyz]

Yea, that's what I was thinking. One peer (our LSP) and no network graph so we don't know that our peer is public. I agree we should maybe just consider all peers in that case.

The patch that I posted above will take any peer as the introduction node if we don't have a NetworkGraph.

.or_else(|| (!is_recipient_announced).then(|| (peer, false, 0)))

But if we have more than one unannounced peer -- unknowingly because we have an empty NetworkGraph-- then this may leak an unannounced peer and may not help with the testing case if the path for the wrong peer is selected for the offer / reply path. So instead this probably should be:

.or_else(|| (!is_recipient_announced && peer_count == 1).then(|| (peer, false, 0)))

So when we are unannounced, this would work for the testing case so long as we have only one peer. Similarly for the LSP case. Although there, if (in addition) the NetworkGraph contained just the LSP(s), then it would work in cases with multiple LSPs / other peers, too, because of the change relaxing the number of peer channels:

.filter(|info| !is_recipient_announced || info.channels.len() >= MIN_PEER_CHANNELS)

We may need to prioritized known announced peers over others, though.

Does this seem sufficient? Otherwise, we may need to pass more data to MessageRouter, but OnionMessenger doesn't have any more useful information about our peers.

[TheBlueMatt]

Makes sense to me, I think.