From b00ec2f139d3b991f1a88f40e1b49beb9d290857 Mon Sep 17 00:00:00 2001 From: shaavan Date: Thu, 19 Jun 2025 23:50:09 +0530 Subject: [PATCH 1/9] Fix: Ignore SCIDs when creating full-length blinded paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the `compact_paths` flag was only used to determine whether to use a compact introduction node when creating compact blinded paths. With the upcoming change to accept `MessageForwardNode` in `create_blinded_paths`, there's a risk of SCIDs being passed (and used) even when the user intends to create a full-length blinded path. This patch updates the logic in `create_blinded_paths_from_iter` to ignore SCIDs unless `compact_paths` is explicitly true—preserving correct behavior for full-length blinded paths. --- lightning/src/onion_message/messenger.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index fd98f78350e..8285652995a 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -571,6 +571,10 @@ where let has_one_peer = peers.len() == 1; let mut peer_info = peers + .map(|peer| MessageForwardNode { + node_id: peer.node_id, + short_channel_id: if compact_paths { peer.short_channel_id } else { None }, + }) // Limit to peers with announced channels unless the recipient is unannounced. .filter_map(|peer| { network_graph From 71dcf214e7c13565ce0a50743153455074fc9732 Mon Sep 17 00:00:00 2001 From: shaavan Date: Thu, 27 Mar 2025 14:27:09 +0530 Subject: [PATCH 2/9] Update `create_blinded_paths` to accept `Vec` To prepare for supporting both standard and compact blinded paths, this commit updates the `create_blinded_paths` function to take a `Vec` as input. This change ensures the function has all the information it needs to handle both types of blinded path creation. This refactor that sets the stage for upcoming enhancements. --- fuzz/src/chanmon_consistency.rs | 4 +- fuzz/src/full_stack.rs | 4 +- fuzz/src/onion_message.rs | 4 +- lightning-dns-resolver/src/lib.rs | 6 ++- lightning/src/offers/flow.rs | 1 - lightning/src/onion_message/messenger.rs | 59 ++++++------------------ lightning/src/util/test_utils.rs | 2 +- 7 files changed, 25 insertions(+), 55 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 59612636a9e..a5721b3c78d 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -33,7 +33,7 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash; use bitcoin::hashes::Hash as TraitImport; use bitcoin::WPubkeyHash; -use lightning::blinded_path::message::{BlindedMessagePath, MessageContext}; +use lightning::blinded_path::message::{BlindedMessagePath, MessageContext, MessageForwardNode}; use lightning::blinded_path::payment::{BlindedPaymentPath, ReceiveTlvs}; use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; @@ -142,7 +142,7 @@ impl MessageRouter for FuzzRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, + &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index f6fa07199fa..8f5a1408fcd 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -30,7 +30,7 @@ use bitcoin::hashes::Hash as _; use bitcoin::hex::FromHex; use bitcoin::WPubkeyHash; -use lightning::blinded_path::message::{BlindedMessagePath, MessageContext}; +use lightning::blinded_path::message::{BlindedMessagePath, MessageContext, MessageForwardNode}; use lightning::blinded_path::payment::{BlindedPaymentPath, ReceiveTlvs}; use lightning::chain; use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; @@ -173,7 +173,7 @@ impl MessageRouter for FuzzRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, + &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index f2da1a316fc..f2e5b4464b7 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -6,7 +6,7 @@ use bitcoin::secp256k1::schnorr; use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1, SecretKey}; use lightning::blinded_path::message::{ - AsyncPaymentsContext, BlindedMessagePath, MessageContext, OffersContext, + AsyncPaymentsContext, BlindedMessagePath, MessageContext, MessageForwardNode, OffersContext, }; use lightning::blinded_path::EmptyNodeIdLookUp; use lightning::ln::inbound_payment::ExpandedKey; @@ -103,7 +103,7 @@ impl MessageRouter for TestMessageRouter { } fn create_blinded_paths( - &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, + &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, _secp_ctx: &Secp256k1, ) -> Result, ()> { unreachable!() diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs index 73dccdadd23..4310cbcd74e 100644 --- a/lightning-dns-resolver/src/lib.rs +++ b/lightning-dns-resolver/src/lib.rs @@ -159,7 +159,9 @@ mod test { use bitcoin::secp256k1::{self, PublicKey, Secp256k1}; use bitcoin::Block; - use lightning::blinded_path::message::{BlindedMessagePath, MessageContext}; + use lightning::blinded_path::message::{ + BlindedMessagePath, MessageContext, MessageForwardNode, + }; use lightning::blinded_path::NodeIdLookUp; use lightning::events::{Event, PaymentPurpose}; use lightning::ln::channelmanager::{PaymentId, Retry}; @@ -230,7 +232,7 @@ mod test { } fn create_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, _peers: Vec, + &self, recipient: PublicKey, context: MessageContext, _peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { let keys = KeysManager::new(&[0; 32], 42, 43); diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 38f674141b1..bac321f1679 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -229,7 +229,6 @@ where let recipient = self.get_our_node_id(); let secp_ctx = &self.secp_ctx; - let peers = peers.into_iter().map(|node| node.node_id).collect(); self.message_router .create_blinded_paths(recipient, context, peers, secp_ctx) .and_then(|paths| (!paths.is_empty()).then(|| paths).ok_or(())) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 8285652995a..0b7e6048e2c 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -217,7 +217,7 @@ where /// # }) /// # } /// # fn create_blinded_paths( -/// # &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, _secp_ctx: &Secp256k1 +/// # &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, _secp_ctx: &Secp256k1 /// # ) -> Result, ()> { /// # unreachable!() /// # } @@ -495,7 +495,7 @@ pub trait MessageRouter { /// Creates [`BlindedMessagePath`]s to the `recipient` node. The nodes in `peers` are assumed to /// be direct peers with the `recipient`. fn create_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, + &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()>; @@ -516,10 +516,6 @@ pub trait MessageRouter { &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - let peers = peers - .into_iter() - .map(|MessageForwardNode { node_id, short_channel_id: _ }| node_id) - .collect(); self.create_blinded_paths(recipient, context, peers, secp_ctx) } } @@ -551,7 +547,7 @@ where Self { network_graph, entropy_source } } - fn create_blinded_paths_from_iter< + pub(crate) fn create_blinded_paths_from_iter< I: ExactSizeIterator, T: secp256k1::Signing + secp256k1::Verification, >( @@ -665,38 +661,6 @@ where } } } - - pub(crate) fn create_blinded_paths( - network_graph: &G, recipient: PublicKey, context: MessageContext, peers: Vec, - entropy_source: &ES, secp_ctx: &Secp256k1, - ) -> Result, ()> { - let peers = - peers.into_iter().map(|node_id| MessageForwardNode { node_id, short_channel_id: None }); - Self::create_blinded_paths_from_iter( - network_graph, - recipient, - context, - peers.into_iter(), - entropy_source, - secp_ctx, - false, - ) - } - - pub(crate) fn create_compact_blinded_paths( - network_graph: &G, recipient: PublicKey, context: MessageContext, - peers: Vec, entropy_source: &ES, secp_ctx: &Secp256k1, - ) -> Result, ()> { - Self::create_blinded_paths_from_iter( - network_graph, - recipient, - context, - peers.into_iter(), - entropy_source, - secp_ctx, - true, - ) - } } impl>, L: Deref, ES: Deref> MessageRouter @@ -712,16 +676,17 @@ where } fn create_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, + &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - Self::create_blinded_paths( + Self::create_blinded_paths_from_iter( &self.network_graph, recipient, context, - peers, + peers.into_iter(), &self.entropy_source, secp_ctx, + false, ) } @@ -729,13 +694,14 @@ where &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - Self::create_compact_blinded_paths( + Self::create_blinded_paths_from_iter( &self.network_graph, recipient, context, - peers, + peers.into_iter(), &self.entropy_source, secp_ctx, + true, ) } } @@ -1424,7 +1390,10 @@ where message_recipients .iter() .filter(|(_, peer)| matches!(peer, OnionMessageRecipient::ConnectedPeer(_))) - .map(|(node_id, _)| *node_id) + .map(|(node_id, _)| MessageForwardNode { + node_id: *node_id, + short_channel_id: None, + }) .collect::>() }; diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index fecdb830fe0..597a99dcf93 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -335,7 +335,7 @@ impl<'a> MessageRouter for TestMessageRouter<'a> { } fn create_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, + &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { self.inner.create_blinded_paths(recipient, context, peers, secp_ctx) From 8401798562461dab88d07ec2d22278d823d3952e Mon Sep 17 00:00:00 2001 From: shaavan Date: Thu, 27 Mar 2025 14:40:00 +0530 Subject: [PATCH 3/9] Introduce `NodeIdMessageRouter` and `NullMessageRouter` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To make the purpose of each `MessageRouter` implementation unambiguous, this commit sets a direction where the type of `MessageRouter` used deterministically defines the kind of blinded paths created. As a step toward this goal, two new default routers are introduced: - `NodeIdMessageRouter` – creates full-length blinded paths using the peer's node ID. - `NullMessageRouter` – intentionally creates no blinded paths. --- lightning/src/onion_message/messenger.rs | 108 +++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 0b7e6048e2c..9ed759ecb5f 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -706,6 +706,114 @@ where } } +/// This message router is similar to [`DefaultMessageRouter`], but it always creates +/// full-length blinded paths, using the peer's [`NodeId`]. +/// +/// This message router can only route to a directly connected [`Destination`]. +/// +/// # Privacy +/// +/// 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 a 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. +pub struct NodeIdMessageRouter>, L: Deref, ES: Deref> +where + L::Target: Logger, + ES::Target: EntropySource, +{ + network_graph: G, + entropy_source: ES, +} + +impl>, L: Deref, ES: Deref> NodeIdMessageRouter +where + L::Target: Logger, + ES::Target: EntropySource, +{ + /// Creates a [`NodeIdMessageRouter`] using the given [`NetworkGraph`]. + pub fn new(network_graph: G, entropy_source: ES) -> Self { + Self { network_graph, entropy_source } + } +} + +impl>, L: Deref, ES: Deref> MessageRouter + for NodeIdMessageRouter +where + L::Target: Logger, + ES::Target: EntropySource, +{ + fn find_path( + &self, sender: PublicKey, peers: Vec, destination: Destination, + ) -> Result { + DefaultMessageRouter::::find_path(&self.network_graph, sender, peers, destination) + } + + fn create_blinded_paths( + &self, recipient: PublicKey, context: MessageContext, peers: Vec, + secp_ctx: &Secp256k1, + ) -> Result, ()> { + DefaultMessageRouter::create_blinded_paths_from_iter( + &self.network_graph, + recipient, + context, + peers.into_iter(), + &self.entropy_source, + secp_ctx, + false, + ) + } + + fn create_compact_blinded_paths( + &self, recipient: PublicKey, context: MessageContext, peers: Vec, + secp_ctx: &Secp256k1, + ) -> Result, ()> { + DefaultMessageRouter::create_blinded_paths_from_iter( + &self.network_graph, + recipient, + context, + peers.into_iter(), + &self.entropy_source, + secp_ctx, + false, + ) + } +} + +/// A special [`MessageRouter`] implementation that performs no routing. +/// +/// # Note +/// [`NullMessageRouter`] **must not** be used with [`ChannelManager`] as a parameter. +/// +/// # Reason +/// [`ChannelManager`] requires a functioning [`MessageRouter`] to create blinded paths, +/// which are necessary for constructing reply paths in onion message communication. +/// +/// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager +pub struct NullMessageRouter {} + +impl MessageRouter for NullMessageRouter { + fn find_path( + &self, _sender: PublicKey, _peers: Vec, _destination: Destination, + ) -> Result { + unreachable!() + } + + fn create_blinded_paths( + &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, + _secp_ctx: &Secp256k1, + ) -> Result, ()> { + Ok(vec![]) + } + + fn create_compact_blinded_paths( + &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, + _secp_ctx: &Secp256k1, + ) -> Result, ()> { + Ok(vec![]) + } +} + /// A path for sending an [`OnionMessage`]. #[derive(Clone)] pub struct OnionMessagePath { From ecfb791e883695dd01570b6ade9ae40aac82bc79 Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 18 Jun 2025 18:47:01 +0530 Subject: [PATCH 4/9] refactor: Update TestMessageRouter to enum To allow choosing different message router types for testing nodes, convert `TestMessageRouter` to an enum with variants `DefaultMessageRouter` and `NodeIdMessageRouter`. This provides better flexibility when testing various scenarios. --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/functional_test_utils.rs | 72 ++++++++++++++++++----- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/test_utils.rs | 57 ++++++++++++++---- 4 files changed, 103 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9d25c68c625..14af6a44d6a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -17077,7 +17077,7 @@ pub mod bench { let scorer = RwLock::new(test_utils::TestScorer::new()); let entropy = test_utils::TestKeysInterface::new(&[0u8; 32], network); let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &logger_a, &scorer); - let message_router = test_utils::TestMessageRouter::new(Arc::new(NetworkGraph::new(network, &logger_a)), &entropy); + let message_router = test_utils::TestMessageRouter::new_default(Arc::new(NetworkGraph::new(network, &logger_a)), &entropy); let mut config: UserConfig = Default::default(); config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 79219857b0e..ce55f907299 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -38,7 +38,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}; use crate::util::test_utils::{TestChainMonitor, TestScorer, TestKeysInterface}; use crate::util::ser::{ReadableArgs, Writeable}; @@ -728,7 +728,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { signer_provider: self.keys_manager, fee_estimator: &test_utils::TestFeeEstimator::new(253), router: &test_utils::TestRouter::new(Arc::clone(&network_graph), &self.logger, &scorer), - message_router: &test_utils::TestMessageRouter::new(network_graph, self.keys_manager), + message_router: &test_utils::TestMessageRouter::new_default(network_graph, self.keys_manager), chain_monitor: self.chain_monitor, tx_broadcaster: &broadcaster, logger: &self.logger, @@ -3345,26 +3345,39 @@ 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) -> Vec> { - 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, persisters: Vec<&'a impl test_utils::SyncPersist>) -> Vec> { +fn create_node_cfgs_internal<'a, F>( + node_count: usize, + chanmon_cfgs: &'a Vec, + persisters: Vec<&'a impl test_utils::SyncPersist>, + message_router_constructor: F, +) -> Vec> +where + F: Fn(Arc>, &'a TestKeysInterface) -> test_utils::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 cfg = &chanmon_cfgs[i]; + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &cfg.logger)); + let chain_monitor = test_utils::TestChainMonitor::new( + Some(&cfg.chain_source), + &cfg.tx_broadcaster, + &cfg.logger, + &cfg.fee_estimator, + persisters[i], + &cfg.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(Arc::clone(&network_graph), &chanmon_cfgs[i].logger, &chanmon_cfgs[i].scorer), - message_router: test_utils::TestMessageRouter::new(Arc::clone(&network_graph), &chanmon_cfgs[i].keys_manager), + chain_source: &cfg.chain_source, + logger: &cfg.logger, + tx_broadcaster: &cfg.tx_broadcaster, + fee_estimator: &cfg.fee_estimator, + router: test_utils::TestRouter::new(Arc::clone(&network_graph), &cfg.logger, &cfg.scorer), + message_router: message_router_constructor(Arc::clone(&network_graph), &cfg.keys_manager), chain_monitor, - keys_manager: &chanmon_cfgs[i].keys_manager, + keys_manager: &cfg.keys_manager, node_seed: seed, network_graph, override_init_features: Rc::new(RefCell::new(None)), @@ -3374,6 +3387,33 @@ pub fn create_node_cfgs_with_persisters<'a>(node_count: usize, chanmon_cfgs: &'a nodes } +pub fn create_node_cfgs<'a>(node_count: usize, chanmon_cfgs: &'a Vec) -> Vec> { + let persisters = chanmon_cfgs.iter().map(|c| &c.persister).collect(); + create_node_cfgs_internal(node_count, chanmon_cfgs, persisters, |graph, keys_manager| { + test_utils::TestMessageRouter::new_default(graph, keys_manager) + }) +} + +pub fn create_node_cfgs_with_persisters<'a>( + node_count: usize, + chanmon_cfgs: &'a Vec, + persisters: Vec<&'a impl test_utils::SyncPersist>, +) -> Vec> { + create_node_cfgs_internal(node_count, chanmon_cfgs, persisters, |graph, keys_manager| { + test_utils::TestMessageRouter::new_default(graph, keys_manager) + }) +} + +pub fn create_node_cfgs_with_node_id_message_router<'a>( + node_count: usize, + chanmon_cfgs: &'a Vec, +) -> Vec> { + let persisters = chanmon_cfgs.iter().map(|c| &c.persister).collect(); + create_node_cfgs_internal(node_count, chanmon_cfgs, persisters, |graph, keys_manager| { + test_utils::TestMessageRouter::new_node_id_router(graph, keys_manager) + }) +} + pub fn test_default_channel_config() -> UserConfig { let mut default_config = UserConfig::default(); // Set cltv_expiry_delta slightly lower to keep the final CLTV values inside one byte in our diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 60aa21bc44e..3ef8317bfa5 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -5175,7 +5175,7 @@ pub fn test_key_derivation_params() { let router = test_utils::TestRouter::new(Arc::clone(&network_graph), &chanmon_cfgs[0].logger, &scorer); let message_router = - test_utils::TestMessageRouter::new(Arc::clone(&network_graph), &keys_manager); + test_utils::TestMessageRouter::new_default(Arc::clone(&network_graph), &keys_manager); let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 597a99dcf93..10a2ad6ab2f 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -34,7 +34,7 @@ use crate::ln::types::ChannelId; use crate::ln::{msgs, wire}; use crate::offers::invoice::UnsignedBolt12Invoice; use crate::onion_message::messenger::{ - DefaultMessageRouter, Destination, MessageRouter, OnionMessagePath, + DefaultMessageRouter, Destination, MessageRouter, NodeIdMessageRouter, OnionMessagePath, }; use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId, RoutingFees}; use crate::routing::router::{ @@ -311,19 +311,35 @@ impl<'a> Drop for TestRouter<'a> { } } -pub struct TestMessageRouter<'a> { - inner: DefaultMessageRouter< - Arc>, - &'a TestLogger, - &'a TestKeysInterface, - >, +pub enum TestMessageRouter<'a> { + Default { + inner: DefaultMessageRouter< + Arc>, + &'a TestLogger, + &'a TestKeysInterface, + >, + }, + + NodeId { + inner: NodeIdMessageRouter< + Arc>, + &'a TestLogger, + &'a TestKeysInterface, + >, + }, } impl<'a> TestMessageRouter<'a> { - pub fn new( + pub fn new_default( network_graph: Arc>, entropy_source: &'a TestKeysInterface, ) -> Self { - Self { inner: DefaultMessageRouter::new(network_graph, entropy_source) } + Self::Default { inner: DefaultMessageRouter::new(network_graph, entropy_source) } + } + + pub fn new_node_id_router( + network_graph: Arc>, entropy_source: &'a TestKeysInterface, + ) -> Self { + Self::NodeId { inner: NodeIdMessageRouter::new(network_graph, entropy_source) } } } @@ -331,21 +347,38 @@ impl<'a> MessageRouter for TestMessageRouter<'a> { fn find_path( &self, sender: PublicKey, peers: Vec, destination: Destination, ) -> Result { - self.inner.find_path(sender, peers, destination) + match self { + Self::Default { inner } => inner.find_path(sender, peers, destination), + Self::NodeId { inner } => inner.find_path(sender, peers, destination), + } } fn create_blinded_paths( &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.inner.create_blinded_paths(recipient, context, peers, secp_ctx) + match self { + Self::Default { inner } => { + inner.create_blinded_paths(recipient, context, peers, secp_ctx) + }, + Self::NodeId { inner } => { + inner.create_blinded_paths(recipient, context, peers, secp_ctx) + }, + } } fn create_compact_blinded_paths( &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()> { - self.inner.create_compact_blinded_paths(recipient, context, peers, secp_ctx) + match self { + Self::Default { inner } => { + inner.create_compact_blinded_paths(recipient, context, peers, secp_ctx) + }, + Self::NodeId { inner } => { + inner.create_compact_blinded_paths(recipient, context, peers, secp_ctx) + }, + } } } From 4ad61c7a2468fb6810212d1d5a46e9daab70fd7c Mon Sep 17 00:00:00 2001 From: shaavan Date: Fri, 13 Jun 2025 17:08:11 +0530 Subject: [PATCH 5/9] Update `DefaultMessageRouter` to always create compact blinded paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reasoning: This change aligns `DefaultMessageRouter`'s default behavior with the most common and practical usage: generating compact blinded paths for BOLT12 offers. While configurability is important, most users won't read all documentation and will rely on defaults that "just work." In line with this PR's principle— "One `MessageRouter`, one type of `BlindedPath`"—the previous default would silently produce full blinded paths, even when a compact one would have been more appropriate. In typical setups (e.g., a user connected to a single LSP), the SCID alias remains stable for the channel's lifetime. Compact paths are not only sufficient in this case, but also result in smaller, more efficient offers. And if the alias becomes invalid, a pubkey-based path wouldn't help either—so compact remains the better default. In brief: This commit makes the default behavior match what users actually want. Thanks to [@TheBlueMatt](https://github.com/TheBlueMatt) for the original reasoning. **Discussion link:** [#3246 (pull request review)](https://github.com/lightningdevkit/rust-lightning/pull/3246#pullrequestreview-2898070836) --- lightning-dns-resolver/src/lib.rs | 2 +- lightning/src/ln/offers_tests.rs | 71 +++++++++++++----------- lightning/src/onion_message/messenger.rs | 13 ++++- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs index 4310cbcd74e..51045d88085 100644 --- a/lightning-dns-resolver/src/lib.rs +++ b/lightning-dns-resolver/src/lib.rs @@ -456,7 +456,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); diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index ad0a8eea2aa..ef9e53fb194 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -149,6 +149,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); @@ -1391,7 +1398,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 @@ -1400,7 +1407,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]); @@ -1421,7 +1428,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); @@ -1451,7 +1458,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); } @@ -1502,7 +1509,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. @@ -1553,7 +1560,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); @@ -1610,7 +1617,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); @@ -1644,7 +1651,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(); diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 9ed759ecb5f..74b2696c4f7 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -522,6 +522,17 @@ 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. +/// +/// # Compact Blinded Paths +/// +/// Compact blinded paths use short channel IDs (SCIDs) instead of pubkeys, resulting in smaller +/// serialization. This is particularly useful when encoding data into space-constrained formats +/// such as QR codes. The SCID is communicated via a [`MessageForwardNode`], but may be `None` +/// to allow for graceful degradation. +/// /// # Privacy /// /// Creating [`BlindedMessagePath`]s may affect privacy since, if a suitable path cannot be found, @@ -686,7 +697,7 @@ where peers.into_iter(), &self.entropy_source, secp_ctx, - false, + true, ) } From 405736d55b2d8552c7d7e7809a6a8460ac1d8ce3 Mon Sep 17 00:00:00 2001 From: shaavan Date: Fri, 20 Jun 2025 18:54:42 +0530 Subject: [PATCH 6/9] Update `create_offer_builder` to use `create_blinded_paths` To simplify blinded path creation and uphold the principle of "One `MessageRouter`, one `BlindedPath` type," this commit updates `create_offer_builder` to use the `create_blinded_paths` method of the `MessageRouter`. Now, when `create_offer_builder` is called, the offer will be created using the `MessageRouter` implementation that the `ChannelManager` or `OffersMessageFlow` is parameterized with. If a user wishes to create an offer with a different type of blinded path, they can explicitly use `create_offer_builder_using_router`, which allows passing a custom `MessageRouter`. The reasoning behind this change is to give users clearer, more deterministic control over the type of blinded path used in the offer. It also improves user awareness, ensuring that creating a non-default blinded path becomes an *intentional choice*. --- lightning-dns-resolver/src/lib.rs | 2 +- lightning/src/ln/channelmanager.rs | 55 ++++++-- .../src/ln/max_payment_path_len_tests.rs | 2 +- lightning/src/ln/offers_tests.rs | 58 ++++---- lightning/src/offers/flow.rs | 128 +++++++++++------- 5 files changed, 152 insertions(+), 93 deletions(-) diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs index 51045d88085..f0b94e4f4d6 100644 --- a/lightning-dns-resolver/src/lib.rs +++ b/lightning-dns-resolver/src/lib.rs @@ -482,7 +482,7 @@ mod test { let name = HumanReadableName::from_encoded("matt@mattcorallo.com").unwrap(); - let bs_offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap(); + let bs_offer = nodes[1].node.create_offer_builder().unwrap().build().unwrap(); let resolvers = vec![Destination::Node(resolver_id)]; let retry = Retry::Attempts(0); let amt = 42_000; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 14af6a44d6a..2e643e3174c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2212,9 +2212,8 @@ where /// # /// # fn example(channel_manager: T) -> Result<(), Bolt12SemanticError> { /// # let channel_manager = channel_manager.get_cm(); -/// # let absolute_expiry = None; /// let offer = channel_manager -/// .create_offer_builder(absolute_expiry)? +/// .create_offer_builder()? /// # ; /// # // Needed for compiling for c_bindings /// # let builder: lightning::offers::offer::OfferBuilder<_, _> = offer.into(); @@ -10852,10 +10851,9 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { /// /// # Privacy /// - /// Uses [`MessageRouter`] to construct a [`BlindedMessagePath`] for the offer based on the given - /// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for - /// privacy implications as well as those of the parameterized [`Router`], which implements - /// [`MessageRouter`]. + /// Uses the [`ChannelManager`]'s [`MessageRouter`] to construct a [`BlindedMessagePath`] for + /// the offer. See those docs for privacy implications as well as those of the parameterized + /// [`Router`], which implements [`MessageRouter`]. /// /// Also, uses a derived signing pubkey in the offer for recipient privacy. /// @@ -10870,12 +10868,47 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { /// [`BlindedMessagePath`]: crate::blinded_path::message::BlindedMessagePath /// [`Offer`]: crate::offers::offer::Offer /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest - pub fn create_offer_builder( - &$self, absolute_expiry: Option - ) -> Result<$builder, Bolt12SemanticError> { - let entropy = &*$self.entropy_source; + pub fn create_offer_builder(&$self) -> Result<$builder, Bolt12SemanticError> { + let builder = $self.flow.create_offer_builder( + &*$self.entropy_source, $self.get_peers_for_blinded_path() + )?; + + Ok(builder.into()) + } - let builder = $self.flow.create_offer_builder(entropy, absolute_expiry, $self.get_peers_for_blinded_path())?; + /// Creates an [`OfferBuilder`] such that the [`Offer`] it builds is recognized by the + /// [`ChannelManager`] when handling [`InvoiceRequest`] messages for the offer. + /// + /// # Privacy + /// + /// Constructs a [`BlindedMessagePath`] for the offer using a custom [`MessageRouter`]. + /// Users can implement a custom [`MessageRouter`] to define properties of the + /// [`BlindedMessagePath`] as required or opt not to create any `BlindedMessagePath`. + /// + /// Also, uses a derived signing pubkey in the offer for recipient privacy. + /// + /// # Limitations + /// + /// Requires a direct connection to the introduction node in the responding [`InvoiceRequest`]'s + /// reply path. + /// + /// # Errors + /// + /// Errors if the provided [`Router`] is unable to create a blinded path for the offer. + /// + /// [`BlindedMessagePath`]: crate::blinded_path::message::BlindedMessagePath + /// [`Offer`]: crate::offers::offer::Offer + /// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest + pub fn create_offer_builder_using_router( + &$self, + router: ME, + ) -> Result<$builder, Bolt12SemanticError> + where + ME::Target: MessageRouter, + { + let builder = $self.flow.create_offer_builder_using_router( + router, &*$self.entropy_source, $self.get_peers_for_blinded_path() + )?; Ok(builder.into()) } diff --git a/lightning/src/ln/max_payment_path_len_tests.rs b/lightning/src/ln/max_payment_path_len_tests.rs index ff5053644d8..4efa105e0ad 100644 --- a/lightning/src/ln/max_payment_path_len_tests.rs +++ b/lightning/src/ln/max_payment_path_len_tests.rs @@ -517,7 +517,7 @@ fn bolt12_invoice_too_large_blinded_paths() { ), ]); - let offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap(); + let offer = nodes[1].node.create_offer_builder().unwrap().build().unwrap(); let payment_id = PaymentId([1; 32]); let route_config = RouteParametersConfig::default(); nodes[0] diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index ef9e53fb194..fe5946fe418 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -307,7 +307,7 @@ fn prefers_non_tor_nodes_in_blinded_paths() { announce_node_address(charlie, &[alice, bob, david, &nodes[4], &nodes[5]], tor.clone()); let offer = bob.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id)); @@ -323,7 +323,7 @@ fn prefers_non_tor_nodes_in_blinded_paths() { announce_node_address(&nodes[5], &[alice, bob, charlie, david, &nodes[4]], tor.clone()); let offer = bob.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id)); @@ -374,7 +374,7 @@ fn prefers_more_connected_nodes_in_blinded_paths() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = bob.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(bob_id)); @@ -399,11 +399,9 @@ fn creates_short_lived_offer() { let alice_id = alice.node.get_our_node_id(); let bob = &nodes[1]; - let absolute_expiry = alice.node.duration_since_epoch() + MAX_SHORT_LIVED_RELATIVE_EXPIRY; let offer = alice.node - .create_offer_builder(Some(absolute_expiry)).unwrap() + .create_offer_builder().unwrap() .build().unwrap(); - assert_eq!(offer.absolute_expiry(), Some(absolute_expiry)); assert!(!offer.paths().is_empty()); for path in offer.paths() { let introduction_node_id = resolve_introduction_node(bob, &path); @@ -425,20 +423,17 @@ fn creates_long_lived_offer() { let alice = &nodes[0]; let alice_id = alice.node.get_our_node_id(); - let absolute_expiry = alice.node.duration_since_epoch() + MAX_SHORT_LIVED_RELATIVE_EXPIRY - + Duration::from_secs(1); let offer = alice.node - .create_offer_builder(Some(absolute_expiry)) + .create_offer_builder() .unwrap() .build().unwrap(); - assert_eq!(offer.absolute_expiry(), Some(absolute_expiry)); assert!(!offer.paths().is_empty()); for path in offer.paths() { assert_eq!(path.introduction_node(), &IntroductionNode::NodeId(alice_id)); } let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .build().unwrap(); assert_eq!(offer.absolute_expiry(), None); assert!(!offer.paths().is_empty()); @@ -542,7 +537,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None) + .create_offer_builder() .unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -709,7 +704,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() { let bob_id = bob.node.get_our_node_id(); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id)); @@ -832,7 +827,7 @@ fn pays_for_offer_without_blinded_paths() { let bob_id = bob.node.get_our_node_id(); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .clear_paths() .amount_msats(10_000_000) .build().unwrap(); @@ -956,7 +951,7 @@ fn send_invoice_requests_with_distinct_reply_path() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None) + .create_offer_builder() .unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1092,7 +1087,7 @@ fn creates_and_pays_for_offer_with_retry() { let bob_id = bob.node.get_our_node_id(); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id)); @@ -1168,7 +1163,7 @@ fn pays_bolt12_invoice_asynchronously() { let bob_id = bob.node.get_our_node_id(); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1260,7 +1255,7 @@ fn creates_offer_with_blinded_path_using_unannounced_introduction_node() { let bob_id = bob.node.get_our_node_id(); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); assert_ne!(offer.issuer_signing_pubkey(), Some(alice_id)); @@ -1390,7 +1385,7 @@ fn fails_authentication_when_handling_invoice_request() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None) + .create_offer_builder() .unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1402,7 +1397,7 @@ fn fails_authentication_when_handling_invoice_request() { } let invalid_path = alice.node - .create_offer_builder(None) + .create_offer_builder() .unwrap() .build().unwrap() .paths().first().unwrap() @@ -1502,7 +1497,7 @@ fn fails_authentication_when_handling_invoice_for_offer() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None) + .create_offer_builder() .unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1698,8 +1693,7 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { disconnect_peers(alice, &[bob, charlie, david, &nodes[4], &nodes[5]]); disconnect_peers(david, &[bob, charlie, &nodes[4], &nodes[5]]); - let absolute_expiry = alice.node.duration_since_epoch() + MAX_SHORT_LIVED_RELATIVE_EXPIRY; - match alice.node.create_offer_builder(Some(absolute_expiry)) { + match alice.node.create_offer_builder() { Ok(_) => panic!("Expected error"), Err(e) => assert_eq!(e, Bolt12SemanticError::MissingPaths), } @@ -1708,9 +1702,11 @@ fn fails_creating_or_paying_for_offer_without_connected_peers() { args.send_channel_ready = (true, true); reconnect_nodes(args); + let absolute_expiry = alice.node.duration_since_epoch() + MAX_SHORT_LIVED_RELATIVE_EXPIRY; let offer = alice.node - .create_offer_builder(Some(absolute_expiry)).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) + .absolute_expiry(absolute_expiry) .build().unwrap(); let payment_id = PaymentId([1; 32]); @@ -1813,7 +1809,7 @@ fn fails_creating_invoice_request_for_unsupported_chain() { let bob = &nodes[1]; let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .clear_chains() .chain(Network::Signet) .build().unwrap(); @@ -1872,7 +1868,7 @@ fn fails_creating_invoice_request_without_blinded_reply_path() { disconnect_peers(david, &[bob, charlie, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1906,7 +1902,7 @@ fn fails_creating_invoice_request_with_duplicate_payment_id() { disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -1992,7 +1988,7 @@ fn fails_sending_invoice_without_blinded_payment_paths_for_offer() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -2201,7 +2197,7 @@ fn fails_paying_invoice_with_unknown_required_features() { disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); let offer = alice.node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .amount_msats(10_000_000) .build().unwrap(); @@ -2280,7 +2276,7 @@ fn rejects_keysend_to_non_static_invoice_path() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); // First pay the offer and save the payment preimage and invoice. - let offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap(); + let offer = nodes[1].node.create_offer_builder().unwrap().build().unwrap(); let amt_msat = 5000; let payment_id = PaymentId([1; 32]); nodes[0].node.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(1), RouteParametersConfig::default()).unwrap(); @@ -2360,7 +2356,7 @@ fn no_double_pay_with_stale_channelmanager() { let amt_msat = nodes[0].node.list_usable_channels()[0].next_outbound_htlc_limit_msat + 1; // Force MPP let offer = nodes[1].node - .create_offer_builder(None).unwrap() + .create_offer_builder().unwrap() .clear_paths() .amount_msats(amt_msat) .build().unwrap(); diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index bac321f1679..11a856c76d4 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -449,6 +449,36 @@ where } } + fn create_offer_builder_intern( + &self, entropy_source: ES, make_paths: PF, + ) -> Result<(OfferBuilder, Nonce), Bolt12SemanticError> + where + ES::Target: EntropySource, + PF: FnOnce( + PublicKey, + MessageContext, + &secp256k1::Secp256k1, + ) -> Result, Bolt12SemanticError>, + { + let node_id = self.get_our_node_id(); + let expanded_key = &self.inbound_payment_key; + let entropy = entropy_source; + let secp_ctx = &self.secp_ctx; + + let nonce = Nonce::from_entropy_source(entropy); + let context = MessageContext::Offers(OffersContext::InvoiceRequest { nonce }); + + let mut builder = + OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) + .chain_hash(self.chain_hash); + + for path in make_paths(node_id, context, secp_ctx)? { + builder = builder.path(path) + } + + Ok((builder.into(), nonce)) + } + /// Creates an [`OfferBuilder`] such that the [`Offer`] it builds is recognized by the /// [`OffersMessageFlow`], and any corresponding [`InvoiceRequest`] can be verified using /// [`Self::verify_invoice_request`]. The offer will expire at `absolute_expiry` if `Some`, @@ -456,18 +486,17 @@ where /// /// # Privacy /// - /// Uses [`MessageRouter`] to construct a [`BlindedMessagePath`] for the offer based on the given - /// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for - /// privacy implications, as well as those of the parameterized [`Router`], which implements - /// [`MessageRouter`]. + /// Uses the [`OffersMessageFlow`]'s [`MessageRouter`] to construct a [`BlindedMessagePath`] + /// for the offer. See those docs for privacy implications as well as those of the parameterized + /// [`Router`], which implements [`MessageRouter`]. /// /// Also uses a derived signing pubkey in the offer for recipient privacy. /// /// # Limitations /// /// If [`DefaultMessageRouter`] is used to parameterize the [`OffersMessageFlow`], a direct - /// connection to the introduction node in the responding [`InvoiceRequest`]'s reply path is required. - /// See the [`DefaultMessageRouter`] documentation for more details. + /// connection to the introduction node in the responding [`InvoiceRequest`]'s reply path is + /// required. See the [`DefaultMessageRouter`] documentation for more details. /// /// # Errors /// @@ -475,35 +504,52 @@ where /// /// [`DefaultMessageRouter`]: crate::onion_message::messenger::DefaultMessageRouter pub fn create_offer_builder( - &self, entropy_source: ES, absolute_expiry: Option, - peers: Vec, + &self, entropy_source: ES, peers: Vec, ) -> Result, Bolt12SemanticError> where ES::Target: EntropySource, { - let node_id = self.get_our_node_id(); - let expanded_key = &self.inbound_payment_key; - let entropy = &*entropy_source; - let secp_ctx = &self.secp_ctx; - - let nonce = Nonce::from_entropy_source(entropy); - let context = OffersContext::InvoiceRequest { nonce }; - - let path = self - .create_blinded_paths_using_absolute_expiry(context, absolute_expiry, peers) - .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; - - let builder = OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) - .chain_hash(self.chain_hash) - .path(path); - - let builder = match absolute_expiry { - None => builder, - Some(absolute_expiry) => builder.absolute_expiry(absolute_expiry), - }; + self.create_offer_builder_intern(&*entropy_source, |_, context, _| { + self.create_blinded_paths(peers, context) + .map(|paths| paths.into_iter().take(1).collect()) + .map_err(|_| Bolt12SemanticError::MissingPaths) + }) + .map(|(builder, _)| builder) + } - Ok(builder) + /// Creates an [`OfferBuilder`] such that the [`Offer`] it builds is recognized by the + /// [`OffersMessageFlow`] when handling [`InvoiceRequest`] messages for the offer. + /// + /// # Privacy + /// + /// Constructs a [`BlindedMessagePath`] for the offer using a custom [`MessageRouter`]. + /// Users can implement a custom [`MessageRouter`] to define properties of the + /// [`BlindedMessagePath`] as required or opt not to create any `BlindedMessagePath`. + /// + /// Also, uses a derived signing pubkey in the offer for recipient privacy. + /// + /// # Limitations + /// + /// Requires a direct connection to the introduction node in the responding [`InvoiceRequest`]'s + /// reply path. + /// + /// # Errors + /// + /// Errors if the provided [`Router`] is unable to create a blinded path for the offer. + pub fn create_offer_builder_using_router( + &self, router: ME, entropy_source: ES, peers: Vec, + ) -> Result, Bolt12SemanticError> + where + ME::Target: MessageRouter, + ES::Target: EntropySource, + { + self.create_offer_builder_intern(&*entropy_source, |node_id, context, secp_ctx| { + router + .create_blinded_paths(node_id, context, peers, secp_ctx) + .map(|paths| paths.into_iter().take(1).collect()) + .map_err(|_| Bolt12SemanticError::MissingPaths) + }) + .map(|(builder, _)| builder) } /// Create an offer for receiving async payments as an often-offline recipient. @@ -521,25 +567,9 @@ where where ES::Target: EntropySource, { - if message_paths_to_always_online_node.is_empty() { - return Err(Bolt12SemanticError::MissingPaths); - } - - let node_id = self.get_our_node_id(); - let expanded_key = &self.inbound_payment_key; - let entropy = &*entropy_source; - let secp_ctx = &self.secp_ctx; - - let nonce = Nonce::from_entropy_source(entropy); - let mut builder = - OfferBuilder::deriving_signing_pubkey(node_id, expanded_key, nonce, secp_ctx) - .chain_hash(self.chain_hash); - - for path in message_paths_to_always_online_node { - builder = builder.path(path); - } - - Ok((builder.into(), nonce)) + self.create_offer_builder_intern(&*entropy_source, |_, _, _| { + Ok(message_paths_to_always_online_node) + }) } /// Creates a [`RefundBuilder`] such that the [`Refund`] it builds is recognized by the From 13855b81768412f70f6a5d9a511e5725ce351f48 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sun, 22 Jun 2025 00:18:24 +0530 Subject: [PATCH 7/9] Update `create_refund_builder` to use `create_blinded_paths` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change mirrors the previous update to `create_offer_builder`, applying the **“One `MessageRouter`, one `BlindedPath` type”** principle to refund creation. Now, `create_refund_builder` uses the `create_blinded_paths` method of the `MessageRouter` associated with the `ChannelManager` or `OffersMessageFlow`. For non-default path behavior, users can call `create_refund_builder_using_router` and pass a custom `MessageRouter`. See previous commit for detailed reasoning. --- lightning/src/ln/channelmanager.rs | 77 ++++++++++++++- lightning/src/offers/flow.rs | 146 +++++++++++++++++++++++------ 2 files changed, 189 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e643e3174c..62f0097cbff 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10937,10 +10937,8 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { /// /// # Privacy /// - /// Uses [`MessageRouter`] to construct a [`BlindedMessagePath`] for the refund based on the given - /// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for - /// privacy implications as well as those of the parameterized [`Router`], which implements - /// [`MessageRouter`]. + /// Uses the [`ChannelManager`]'s [`MessageRouter`] to construct a [`BlindedMessagePath`] + /// for the offer. See those docs for privacy implications. /// /// Also, uses a derived payer id in the refund for payer privacy. /// @@ -10978,6 +10976,77 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { Ok(builder.into()) } + + /// Creates a [`RefundBuilder`] such that the [`Refund`] it builds is recognized by the + /// [`ChannelManager`] when handling [`Bolt12Invoice`] messages for the refund. + /// + /// # Payment + /// + /// The provided `payment_id` is used to ensure that only one invoice is paid for the refund. + /// See [Avoiding Duplicate Payments] for other requirements once the payment has been sent. + /// + /// The builder will have the provided expiration set. Any changes to the expiration on the + /// returned builder will not be honored by [`ChannelManager`]. For non-`std`, the highest seen + /// block time minus two hours is used for the current time when determining if the refund has + /// expired. + /// + /// To revoke the refund, use [`ChannelManager::abandon_payment`] prior to receiving the + /// invoice. If abandoned, or an invoice isn't received before expiration, the payment will fail + /// with an [`Event::PaymentFailed`]. + /// + /// If `max_total_routing_fee_msat` is not specified, The default from + /// [`RouteParameters::from_payment_params_and_value`] is applied. + /// + /// # Privacy + /// + /// Constructs a [`BlindedMessagePath`] for the refund using a custom [`MessageRouter`]. + /// Users can implement a custom [`MessageRouter`] to define properties of the + /// [`BlindedMessagePath`] as required or opt not to create any `BlindedMessagePath`. + /// + /// Also, uses a derived payer id in the refund for payer privacy. + /// + /// # Limitations + /// + /// Requires a direct connection to an introduction node in the responding + /// [`Bolt12Invoice::payment_paths`]. + /// + /// # Errors + /// + /// Errors if: + /// - a duplicate `payment_id` is provided given the caveats in the aforementioned link, + /// - `amount_msats` is invalid, or + /// - the provided [`Router`] is unable to create a blinded path for the refund. + /// + /// [`Refund`]: crate::offers::refund::Refund + /// [`BlindedMessagePath`]: crate::blinded_path::message::BlindedMessagePath + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice + /// [`Bolt12Invoice::payment_paths`]: crate::offers::invoice::Bolt12Invoice::payment_paths + /// [Avoiding Duplicate Payments]: #avoiding-duplicate-payments + pub fn create_refund_builder_using_router( + &$self, router: ME, amount_msats: u64, absolute_expiry: Duration, payment_id: PaymentId, + retry_strategy: Retry, route_params_config: RouteParametersConfig + ) -> Result<$builder, Bolt12SemanticError> + where + ME::Target: MessageRouter, + { + let entropy = &*$self.entropy_source; + + let builder = $self.flow.create_refund_builder_using_router( + router, entropy, amount_msats, absolute_expiry, + payment_id, $self.get_peers_for_blinded_path() + )?; + + let _persistence_guard = PersistenceNotifierGuard::notify_on_drop($self); + + let expiration = StaleExpiration::AbsoluteTimeout(absolute_expiry); + $self.pending_outbound_payments + .add_new_awaiting_invoice( + payment_id, expiration, retry_strategy, route_params_config, None, + ) + .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; + + Ok(builder.into()) + } } } impl< diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index 11a856c76d4..a69e8d74c21 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -572,28 +572,70 @@ where }) } + fn create_refund_builder_intern( + &self, make_paths: PF, entropy_source: ES, amount_msats: u64, absolute_expiry: Duration, + payment_id: PaymentId, + ) -> Result, Bolt12SemanticError> + where + PF: FnOnce( + PublicKey, + MessageContext, + &secp256k1::Secp256k1, + ) -> Result, Bolt12SemanticError>, + ES::Target: EntropySource, + { + let node_id = self.get_our_node_id(); + let expanded_key = &self.inbound_payment_key; + let entropy = &*entropy_source; + let secp_ctx = &self.secp_ctx; + + let nonce = Nonce::from_entropy_source(entropy); + let context = MessageContext::Offers(OffersContext::OutboundPayment { + payment_id, + nonce, + hmac: None, + }); + + // Create the base builder with common properties + let mut builder = RefundBuilder::deriving_signing_pubkey( + node_id, + expanded_key, + nonce, + secp_ctx, + amount_msats, + payment_id, + )? + .chain_hash(self.chain_hash) + .absolute_expiry(absolute_expiry); + + for path in make_paths(node_id, context, secp_ctx)? { + builder = builder.path(path); + } + + Ok(builder.into()) + } + /// Creates a [`RefundBuilder`] such that the [`Refund`] it builds is recognized by the /// [`OffersMessageFlow`], and any corresponding [`Bolt12Invoice`] received for the refund /// can be verified using [`Self::verify_bolt12_invoice`]. /// + /// # Privacy + /// + /// Uses the [`OffersMessageFlow`]'s [`MessageRouter`] to construct a [`BlindedMessagePath`] + /// for the offer. See those docs for privacy implications. + /// /// The builder will have the provided expiration set. Any changes to the expiration on the /// returned builder will not be honored by [`OffersMessageFlow`]. For non-`std`, the highest seen /// block time minus two hours is used for the current time when determining if the refund has /// expired. /// - /// To refund can be revoked by the user prior to receiving the invoice. + /// The refund can be revoked by the user prior to receiving the invoice. /// If abandoned, or if an invoice is not received before expiration, the payment will fail /// with an [`Event::PaymentFailed`]. /// /// If `max_total_routing_fee_msat` is not specified, the default from /// [`RouteParameters::from_payment_params_and_value`] is applied. /// - /// # Privacy - /// - /// Uses [`MessageRouter`] to construct a [`BlindedMessagePath`] for the refund based on the given - /// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for - /// privacy implications. - /// /// Also uses a derived payer id in the refund for payer privacy. /// /// # Errors @@ -612,32 +654,76 @@ where where ES::Target: EntropySource, { - let node_id = self.get_our_node_id(); - let expanded_key = &self.inbound_payment_key; - let entropy = &*entropy_source; - let secp_ctx = &self.secp_ctx; - - let nonce = Nonce::from_entropy_source(entropy); - let context = OffersContext::OutboundPayment { payment_id, nonce, hmac: None }; - - let path = self - .create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry), peers) - .and_then(|paths| paths.into_iter().next().ok_or(())) - .map_err(|_| Bolt12SemanticError::MissingPaths)?; - - let builder = RefundBuilder::deriving_signing_pubkey( - node_id, - expanded_key, - nonce, - secp_ctx, + self.create_refund_builder_intern( + |_, context, _| { + self.create_blinded_paths(peers, context) + .map(|paths| paths.into_iter().take(1).collect()) + .map_err(|_| Bolt12SemanticError::MissingPaths) + }, + &*entropy_source, amount_msats, + absolute_expiry, payment_id, - )? - .chain_hash(self.chain_hash) - .absolute_expiry(absolute_expiry) - .path(path); + ) + } - Ok(builder) + /// Creates a [`RefundBuilder`] such that the [`Refund`] it builds is recognized by the + /// [`OffersMessageFlow`] when handling [`Bolt12Invoice`] messages for the refund. + /// + /// # Privacy + /// + /// Constructs a [`BlindedMessagePath`] for the refund using a custom [`MessageRouter`]. + /// Users can implement a custom [`MessageRouter`] to define properties of the + /// [`BlindedMessagePath`] as required or opt not to create any `BlindedMessagePath`. + /// + /// # Payment + /// + /// The provided `payment_id` is used to ensure that only one invoice is paid for the refund. + /// See [Avoiding Duplicate Payments] for other requirements once the payment has been sent. + /// + /// The builder will have the provided expiration set. Any changes to the expiration on the + /// returned builder will not be honored by [`OffersMessageFlow`]. For non-`std`, the highest seen + /// block time minus two hours is used for the current time when determining if the refund has + /// expired. + /// + /// The refund can be revoked by the user prior to receiving the invoice. + /// If abandoned, or if an invoice is not received before expiration, the payment will fail + /// with an [`Event::PaymentFailed`]. + /// + /// If `max_total_routing_fee_msat` is not specified, The default from + /// [`RouteParameters::from_payment_params_and_value`] is applied. + /// + /// Also, uses a derived payer id in the refund for payer privacy. + /// + /// # Errors + /// + /// Errors if: + /// - a duplicate `payment_id` is provided given the caveats in the aforementioned link, + /// - `amount_msats` is invalid, or + /// - the provided [`Router`] is unable to create a blinded path for the refund. + /// + /// [`Event::PaymentFailed`]: crate::events::Event::PaymentFailed + /// [`RouteParameters::from_payment_params_and_value`]: crate::routing::router::RouteParameters::from_payment_params_and_value + pub fn create_refund_builder_using_router( + &self, router: ME, entropy_source: ES, amount_msats: u64, absolute_expiry: Duration, + payment_id: PaymentId, peers: Vec, + ) -> Result, Bolt12SemanticError> + where + ME::Target: MessageRouter, + ES::Target: EntropySource, + { + self.create_refund_builder_intern( + |node_id, context, secp_ctx| { + router + .create_blinded_paths(node_id, context, peers, secp_ctx) + .map(|paths| paths.into_iter().take(1).collect()) + .map_err(|_| Bolt12SemanticError::MissingPaths) + }, + &*entropy_source, + amount_msats, + absolute_expiry, + payment_id, + ) } /// Creates an [`InvoiceRequestBuilder`] such that the [`InvoiceRequest`] it builds is recognized From 86c000db1f7e1c3305896f9c97c93103116a1284 Mon Sep 17 00:00:00 2001 From: shaavan Date: Sun, 22 Jun 2025 00:35:23 +0530 Subject: [PATCH 8/9] Cleanup: Remove redundant `create_blinded_paths` variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit completes the series implementing the principle: **“One `MessageRouter`, one `BlindedPath` type.”** As the final step, it removes now-redundant variations of the blinded path creation functions, streamlining the API and simplifying the blinded path creation process. --- lightning/src/ln/channelmanager.rs | 4 +- lightning/src/offers/flow.rs | 43 +----------------- lightning/src/onion_message/messenger.rs | 57 ------------------------ lightning/src/util/test_utils.rs | 14 ------ 4 files changed, 3 insertions(+), 115 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 62f0097cbff..430ab3360d0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3021,9 +3021,7 @@ const MAX_NO_CHANNEL_PEERS: usize = 250; /// short-lived, while anything with a greater expiration is considered long-lived. /// /// Using [`ChannelManager::create_offer_builder`] or [`ChannelManager::create_refund_builder`], -/// will included a [`BlindedMessagePath`] created using: -/// - [`MessageRouter::create_compact_blinded_paths`] when short-lived, and -/// - [`MessageRouter::create_blinded_paths`] when long-lived. +/// will included a [`BlindedMessagePath`] created using [`MessageRouter::create_blinded_paths`] /// /// Using compact [`BlindedMessagePath`]s may provide better privacy as the [`MessageRouter`] could select /// more hops. However, since they use short channel ids instead of pubkeys, they are more likely to diff --git a/lightning/src/offers/flow.rs b/lightning/src/offers/flow.rs index a69e8d74c21..87beab6ee06 100644 --- a/lightning/src/offers/flow.rs +++ b/lightning/src/offers/flow.rs @@ -33,7 +33,7 @@ use crate::prelude::*; use crate::chain::BestBlock; use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager::{ - Verification, {PaymentId, CLTV_FAR_FAR_AWAY, MAX_SHORT_LIVED_RELATIVE_EXPIRY}, + Verification, {PaymentId, CLTV_FAR_FAR_AWAY}, }; use crate::ln::inbound_payment; use crate::offers::invoice::{ @@ -141,6 +141,7 @@ where self.our_network_pubkey } + #[cfg(async_payments)] fn duration_since_epoch(&self) -> Duration { #[cfg(not(feature = "std"))] let now = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64); @@ -199,26 +200,6 @@ impl OffersMessageFlow where MR::Target: MessageRouter, { - /// Creates a collection of blinded paths by delegating to [`MessageRouter`] based on - /// the path's intended lifetime. - /// - /// Whether or not the path is compact depends on whether the path is short-lived or long-lived, - /// respectively, based on the given `absolute_expiry` as seconds since the Unix epoch. See - /// [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. - fn create_blinded_paths_using_absolute_expiry( - &self, context: OffersContext, absolute_expiry: Option, - peers: Vec, - ) -> Result, ()> { - let now = self.duration_since_epoch(); - let max_short_lived_absolute_expiry = now.saturating_add(MAX_SHORT_LIVED_RELATIVE_EXPIRY); - - if absolute_expiry.unwrap_or(Duration::MAX) <= max_short_lived_absolute_expiry { - self.create_compact_blinded_paths(peers, context) - } else { - self.create_blinded_paths(peers, MessageContext::Offers(context)) - } - } - /// Creates a collection of blinded paths by delegating to /// [`MessageRouter::create_blinded_paths`]. /// @@ -234,26 +215,6 @@ where .and_then(|paths| (!paths.is_empty()).then(|| paths).ok_or(())) } - /// Creates a collection of blinded paths by delegating to - /// [`MessageRouter::create_compact_blinded_paths`]. - /// - /// Errors if the `MessageRouter` errors. - fn create_compact_blinded_paths( - &self, peers: Vec, context: OffersContext, - ) -> Result, ()> { - let recipient = self.get_our_node_id(); - let secp_ctx = &self.secp_ctx; - - self.message_router - .create_compact_blinded_paths( - recipient, - MessageContext::Offers(context), - peers, - secp_ctx, - ) - .and_then(|paths| (!paths.is_empty()).then(|| paths).ok_or(())) - } - /// Creates multi-hop blinded payment paths for the given `amount_msats` by delegating to /// [`Router::create_blinded_payment_paths`]. fn create_blinded_payment_paths( diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 74b2696c4f7..3db6e2a5713 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -498,26 +498,6 @@ pub trait MessageRouter { &self, recipient: PublicKey, context: MessageContext, peers: Vec, secp_ctx: &Secp256k1, ) -> Result, ()>; - - /// Creates compact [`BlindedMessagePath`]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 [`MessageForwardNode`] but may be `None` for graceful degradation. - /// - /// Implementations using additional intermediate nodes are responsible for using a - /// [`MessageForwardNode`] with `Some` short channel id, if possible. Similarly, implementations - /// should call [`BlindedMessagePath::use_compact_introduction_node`]. - /// - /// The provided implementation simply delegates to [`MessageRouter::create_blinded_paths`], - /// ignoring the short channel ids. - fn create_compact_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, - secp_ctx: &Secp256k1, - ) -> Result, ()> { - self.create_blinded_paths(recipient, context, peers, secp_ctx) - } } /// A [`MessageRouter`] that can only route to a directly connected [`Destination`]. @@ -700,21 +680,6 @@ where true, ) } - - fn create_compact_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, - secp_ctx: &Secp256k1, - ) -> Result, ()> { - Self::create_blinded_paths_from_iter( - &self.network_graph, - recipient, - context, - peers.into_iter(), - &self.entropy_source, - secp_ctx, - true, - ) - } } /// This message router is similar to [`DefaultMessageRouter`], but it always creates @@ -774,21 +739,6 @@ where false, ) } - - fn create_compact_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, - secp_ctx: &Secp256k1, - ) -> Result, ()> { - DefaultMessageRouter::create_blinded_paths_from_iter( - &self.network_graph, - recipient, - context, - peers.into_iter(), - &self.entropy_source, - secp_ctx, - false, - ) - } } /// A special [`MessageRouter`] implementation that performs no routing. @@ -816,13 +766,6 @@ impl MessageRouter for NullMessageRouter { ) -> Result, ()> { Ok(vec![]) } - - fn create_compact_blinded_paths( - &self, _recipient: PublicKey, _context: MessageContext, _peers: Vec, - _secp_ctx: &Secp256k1, - ) -> Result, ()> { - Ok(vec![]) - } } /// A path for sending an [`OnionMessage`]. diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 10a2ad6ab2f..287b7846000 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -366,20 +366,6 @@ impl<'a> MessageRouter for TestMessageRouter<'a> { }, } } - - fn create_compact_blinded_paths( - &self, recipient: PublicKey, context: MessageContext, peers: Vec, - secp_ctx: &Secp256k1, - ) -> Result, ()> { - match self { - Self::Default { inner } => { - inner.create_compact_blinded_paths(recipient, context, peers, secp_ctx) - }, - Self::NodeId { inner } => { - inner.create_compact_blinded_paths(recipient, context, peers, secp_ctx) - }, - } - } } pub struct OnlyReadsKeysInterface {} From 62d056bedde4b772a07cc13ca85d4de4bd42c792 Mon Sep 17 00:00:00 2001 From: shaavan Date: Thu, 3 Oct 2024 17:34:08 +0530 Subject: [PATCH 9/9] Test: Add coverage for Offers and Refunds without blinded paths Introduced tests to validate the behavior of Offers and Refunds created without blinded paths, using `NullMessageRouter`. --- lightning/src/ln/offers_tests.rs | 51 +++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index fe5946fe418..7807c7cecb2 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -60,7 +60,7 @@ use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{InvoiceRequest, InvoiceRequestFields}; use crate::offers::nonce::Nonce; use crate::offers::parse::Bolt12SemanticError; -use crate::onion_message::messenger::{Destination, PeeledOnion, MessageSendInstructions}; +use crate::onion_message::messenger::{Destination, MessageSendInstructions, NullMessageRouter, PeeledOnion}; use crate::onion_message::offers::OffersMessage; use crate::routing::gossip::{NodeAlias, NodeId}; use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig}; @@ -265,6 +265,55 @@ fn extract_invoice_error<'a, 'b, 'c>( } } +/// Checks that an offer can be created with no blinded paths. +#[test] +fn create_offer_with_no_blinded_path() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + + let alice = &nodes[0]; + let alice_id = alice.node.get_our_node_id(); + + let router = NullMessageRouter {}; + let offer = alice.node + .create_offer_builder_using_router(&router).unwrap() + .amount_msats(10_000_000) + .build().unwrap(); + assert_eq!(offer.issuer_signing_pubkey(), Some(alice_id)); + assert!(offer.paths().is_empty()); +} + +/// Checks that a refund can be created with no blinded paths. +#[test] +fn create_refund_with_no_blinded_path() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + + let alice = &nodes[0]; + let alice_id = alice.node.get_our_node_id(); + + let absolute_expiry = Duration::from_secs(u64::MAX); + let payment_id = PaymentId([1; 32]); + + let router = NullMessageRouter {}; + let refund = alice.node + .create_refund_builder_using_router(&router, 10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), RouteParametersConfig::default()) + .unwrap() + .build().unwrap(); + assert_eq!(refund.amount_msats(), 10_000_000); + assert_eq!(refund.absolute_expiry(), Some(absolute_expiry)); + assert_eq!(refund.payer_signing_pubkey(), alice_id); + assert!(refund.paths().is_empty()); +} + /// Checks that blinded paths without Tor-only nodes are preferred when constructing an offer. #[test] fn prefers_non_tor_nodes_in_blinded_paths() {