From 532617188577150cbd974401b10d2b4e0c2e0809 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 22 May 2024 15:32:32 -0500 Subject: [PATCH] Refactor MessageRouter::create_blinded_paths Using compact blinded paths isn't always necessary or desirable. For instance, reply paths are communicated via onion messages where space isn't a premium unlike in QR codes. Additionally, long-lived paths could become invalid if the channel associated with the SCID is closed. Refactor MessageRouter::create_blinded_paths into two methods: one for compact blinded paths and one for normal blinded paths. --- fuzz/src/chanmon_consistency.rs | 3 +- fuzz/src/full_stack.rs | 3 +- fuzz/src/onion_message.rs | 3 +- lightning/src/ln/channelmanager.rs | 12 ++--- lightning/src/onion_message/messenger.rs | 56 +++++++++++++++++++----- lightning/src/routing/router.rs | 10 ++++- lightning/src/util/test_utils.rs | 18 +++++++- 7 files changed, 80 insertions(+), 25 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 1bcba1fbf..1cdf617fa 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -34,7 +34,6 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash; use bitcoin::hash_types::BlockHash; use lightning::blinded_path::BlindedPath; -use lightning::blinded_path::message::ForwardNode; use lightning::blinded_path::payment::ReceiveTlvs; use lightning::chain; use lightning::chain::{BestBlock, ChannelMonitorUpdateStatus, chainmonitor, channelmonitor, Confirm, Watch}; @@ -124,7 +123,7 @@ impl MessageRouter for FuzzRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 2ae5ba222..bdd29be91 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -31,7 +31,6 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash; use bitcoin::hash_types::{Txid, BlockHash}; use lightning::blinded_path::BlindedPath; -use lightning::blinded_path::message::ForwardNode; use lightning::blinded_path::payment::ReceiveTlvs; use lightning::chain; use lightning::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen}; @@ -162,7 +161,7 @@ impl MessageRouter for FuzzRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 4c1c5ac11..371a9421f 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -7,7 +7,6 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature; use bitcoin::secp256k1::schnorr; use lightning::blinded_path::{BlindedPath, EmptyNodeIdLookUp}; -use lightning::blinded_path::message::ForwardNode; use lightning::ln::features::InitFeatures; use lightning::ln::msgs::{self, DecodeError, OnionMessageHandler}; use lightning::ln::script::ShutdownScript; @@ -89,7 +88,7 @@ impl MessageRouter for TestMessageRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, + &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e3f7243cd..691391941 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8245,8 +8245,8 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { /// /// # Privacy /// - /// Uses [`MessageRouter::create_blinded_paths`] to construct a [`BlindedPath`] for the offer. - /// However, if one is not found, uses a one-hop [`BlindedPath`] with + /// Uses [`MessageRouter::create_compact_blinded_paths`] to construct a [`BlindedPath`] for the + /// offer. However, if one is not found, uses a one-hop [`BlindedPath`] with /// [`ChannelManager::get_our_node_id`] as the introduction node instead. In the latter case, /// the node must be announced, otherwise, there is no way to find a path to the introduction in /// order to send the [`InvoiceRequest`]. @@ -8304,8 +8304,8 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { /// /// # Privacy /// - /// Uses [`MessageRouter::create_blinded_paths`] to construct a [`BlindedPath`] for the refund. - /// However, if one is not found, uses a one-hop [`BlindedPath`] with + /// Uses [`MessageRouter::create_compact_blinded_paths`] to construct a [`BlindedPath`] for the + /// refund. However, if one is not found, uses a one-hop [`BlindedPath`] with /// [`ChannelManager::get_our_node_id`] as the introduction node instead. In the latter case, /// the node must be announced, otherwise, there is no way to find a path to the introduction in /// order to send the [`Bolt12Invoice`]. @@ -8686,7 +8686,7 @@ where inbound_payment::get_payment_preimage(payment_hash, payment_secret, &self.inbound_payment_key) } - /// Creates a blinded path by delegating to [`MessageRouter::create_blinded_paths`]. + /// Creates a blinded path by delegating to [`MessageRouter::create_compact_blinded_paths`]. /// /// Errors if the `MessageRouter` errors or returns an empty `Vec`. fn create_blinded_path(&self) -> Result { @@ -8708,7 +8708,7 @@ where .collect::>(); self.router - .create_blinded_paths(recipient, peers, secp_ctx) + .create_compact_blinded_paths(recipient, peers, secp_ctx) .and_then(|paths| paths.into_iter().next().ok_or(())) } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 500498663..5ceb0683c 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -162,7 +162,7 @@ for OnionMessenger where /// # }) /// # } /// # fn create_blinded_paths( -/// # &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1 +/// # &self, _recipient: PublicKey, _peers: Vec, _secp_ctx: &Secp256k1 /// # ) -> Result, ()> { /// # unreachable!() /// # } @@ -426,8 +426,33 @@ pub trait MessageRouter { fn create_blinded_paths< T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()>; + + /// Creates compact [`BlindedPath`]s to the `recipient` node. The nodes in `peers` are assumed + /// to be direct peers with the `recipient`. + /// + /// Compact blinded paths use short channel ids instead of pubkeys for a smaller serialization, + /// which is beneficial when a QR code is used to transport the data. The SCID is passed using a + /// [`ForwardNode`] but may be `None` for graceful degradation. + /// + /// Implementations using additional intermediate nodes are responsible for using a + /// [`ForwardNode`] with `Some` short channel id, if possible. Similarly, implementations should + /// call [`BlindedPath::use_compact_introduction_node`]. + /// + /// The provided implementation simply delegates to [`MessageRouter::create_blinded_paths`], + /// ignoring the short channel ids. + fn create_compact_blinded_paths< + T: secp256k1::Signing + secp256k1::Verification + >( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + let peers = peers + .into_iter() + .map(|ForwardNode { node_id, short_channel_id: _ }| node_id) + .collect(); + self.create_blinded_paths(recipient, peers, secp_ctx) + } } /// A [`MessageRouter`] that can only route to a directly connected [`Destination`]. @@ -454,7 +479,7 @@ where I: Iterator, T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: I, secp_ctx: &Secp256k1, + &self, recipient: PublicKey, peers: I, secp_ctx: &Secp256k1, compact_paths: bool ) -> Result, ()> { // Limit the number of blinded paths that are computed. const MAX_PATHS: usize = 3; @@ -502,8 +527,11 @@ where } }, }?; - for path in &mut paths { - path.use_compact_introduction_node(&network_graph); + + if compact_paths { + for path in &mut paths { + path.use_compact_introduction_node(&network_graph); + } } Ok(paths) @@ -550,10 +578,21 @@ where fn create_blinded_paths< T: secp256k1::Signing + secp256k1::Verification + >( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + let peers = peers + .into_iter() + .map(|node_id| ForwardNode { node_id, short_channel_id: None }); + self.create_blinded_paths_from_iter(recipient, peers, secp_ctx, false) + } + + fn create_compact_blinded_paths< + T: secp256k1::Signing + secp256k1::Verification >( &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.create_blinded_paths_from_iter(recipient, peers.into_iter(), secp_ctx) + self.create_blinded_paths_from_iter(recipient, peers.into_iter(), secp_ctx, true) } } @@ -1090,10 +1129,7 @@ where let peers = self.message_recipients.lock().unwrap() .iter() .filter(|(_, peer)| matches!(peer, OnionMessageRecipient::ConnectedPeer(_))) - .map(|(node_id, _ )| ForwardNode { - node_id: *node_id, - short_channel_id: None, - }) + .map(|(node_id, _ )| *node_id) .collect::>(); self.message_router diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index e1b5b6557..c105e9146 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -173,10 +173,18 @@ impl< G: Deref> + Clone, L: Deref, ES: Deref, S: Deref, fn create_blinded_paths< T: secp256k1::Signing + secp256k1::Verification > ( - &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { self.message_router.create_blinded_paths(recipient, peers, secp_ctx) } + + fn create_compact_blinded_paths< + T: secp256k1::Signing + secp256k1::Verification + > ( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + self.message_router.create_compact_blinded_paths(recipient, peers, secp_ctx) + } } /// A trait defining behavior for routing a payment. diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index a5363d32c..f6616a8e5 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -250,10 +250,18 @@ impl<'a> MessageRouter for TestRouter<'a> { fn create_blinded_paths< T: secp256k1::Signing + secp256k1::Verification >( - &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { self.router.create_blinded_paths(recipient, peers, secp_ctx) } + + fn create_compact_blinded_paths< + T: secp256k1::Signing + secp256k1::Verification + >( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + self.router.create_compact_blinded_paths(recipient, peers, secp_ctx) + } } impl<'a> Drop for TestRouter<'a> { @@ -285,10 +293,16 @@ impl<'a> MessageRouter for TestMessageRouter<'a> { } fn create_blinded_paths( - &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { self.inner.create_blinded_paths(recipient, peers, secp_ctx) } + + fn create_compact_blinded_paths( + &self, recipient: PublicKey, peers: Vec, secp_ctx: &Secp256k1, + ) -> Result, ()> { + self.inner.create_compact_blinded_paths(recipient, peers, secp_ctx) + } } pub struct OnlyReadsKeysInterface {} -- 2.39.5