From 7e11a61c6c1a7674c228b8d2fda59c82299dc133 Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 22 Apr 2026 16:17:30 +0100 Subject: [PATCH 1/5] Replace peer_manager_hook with deferred closure registration Refactors the circular dependency between the HRNResolver and the PeerManager during the Node initialization process. Changes: - Removed the `peer_manager_hook` Mutex/Option pattern used to bridge the initialization gap. - Replaced the global initialization hook with a local deferred registration pattern using `Arc::downgrade` after the PeerManager is constructed. - Optimized `post_queue_action` by utilizing a Weak pointer upgrade instead of locking a Mutex on every call. This change improves performance for HRN resolution events and simplifies the builder logic by making the circular dependency resolution explicit and local to the builder function. --- src/builder.rs | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index b0ff1d03b..2caaaf11d 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -10,7 +10,7 @@ use std::convert::TryInto; use std::default::Default; use std::net::ToSocketAddrs; use std::path::PathBuf; -use std::sync::{Arc, Mutex, Once, RwLock, Weak}; +use std::sync::{Arc, Mutex, Once, RwLock}; use std::time::SystemTime; use std::{fmt, fs}; @@ -1735,15 +1735,8 @@ fn build_with_store_internal( })?; } - // This hook resolves a circular dependency: - // 1. PeerManager requires OnionMessenger (via MessageHandler). - // 2. OnionMessenger (via HRN resolver) needs to call PeerManager::process_events. - // - // We provide the resolver with a Weak pointer via this Mutex-protected "hook." - // This allows us to initialize the resolver before the PeerManager exists, - // and prevents a reference cycle (memory leak). - let peer_manager_hook: Arc>>> = Arc::new(Mutex::new(None)); let hrn_resolver; + let mut blip32_resolver = None; let runtime_handle = runtime.handle(); @@ -1755,16 +1748,8 @@ fn build_with_store_internal( let hrn_res = Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph))); hrn_resolver = HRNResolver::Onion(Arc::clone(&hrn_res)); + blip32_resolver = Some(Arc::clone(&hrn_res)); - // We clone the hook because it's moved into a Send + Sync closure that outlives this scope. - let pm_hook_clone = Arc::clone(&peer_manager_hook); - hrn_res.register_post_queue_action(Box::new(move || { - if let Ok(guard) = pm_hook_clone.lock() { - if let Some(pm) = guard.as_ref().and_then(|weak| weak.upgrade()) { - pm.process_events(); - } - } - })); hrn_res as Arc }, HRNResolverConfig::Dns { dns_server_address, enable_hrn_resolution_service, .. } => { @@ -1945,8 +1930,13 @@ fn build_with_store_internal( Arc::clone(&keys_manager), )); - if let Ok(mut guard) = peer_manager_hook.lock() { - *guard = Some(Arc::downgrade(&peer_manager)); + if let Some(res) = blip32_resolver { + let pm_weak = Arc::downgrade(&peer_manager); + res.register_post_queue_action(Box::new(move || { + if let Some(upgraded_pm) = pm_weak.upgrade() { + upgraded_pm.process_events(); + } + })); } liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager))); From 9b72b11522256982afec081e4ca7dc871d2f997f Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 22 Apr 2026 16:24:25 +0100 Subject: [PATCH 2/5] Remove redundant Arc wrapping of HRNResolver Streamlines the storage and passing of HRNResolver by holding it by value in the Node and UnifiedPayment structs. Changes: - Removed the outer Arc wrapper around HRNResolver in Node and UnifiedPayment. - Implemented Clone for HRNResolver to facilitate easy sharing of the internal Arcs. - Updated UnifiedPayment method calls to pass HRNResolver by reference instead of using .as_ref() on an outer Arc. Since HRNResolver variants already contain internal Arcs, the outer Arc was redundant and added unnecessary memory indirection. --- src/builder.rs | 2 +- src/lib.rs | 6 +++--- src/payment/unified.rs | 8 ++++---- src/types.rs | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 2caaaf11d..befa2f260 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -2051,7 +2051,7 @@ fn build_with_store_internal( node_metrics, om_mailbox, async_payments_role, - hrn_resolver: Arc::new(hrn_resolver), + hrn_resolver, #[cfg(cycle_tests)] _leak_checker, }) diff --git a/src/lib.rs b/src/lib.rs index faeb6d339..db63d6258 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -238,7 +238,7 @@ pub struct Node { node_metrics: Arc>, om_mailbox: Option>, async_payments_role: Option, - hrn_resolver: Arc, + hrn_resolver: HRNResolver, #[cfg(cycle_tests)] _leak_checker: LeakChecker, } @@ -1006,7 +1006,7 @@ impl Node { self.bolt12_payment().into(), Arc::clone(&self.config), Arc::clone(&self.logger), - Arc::clone(&self.hrn_resolver), + self.hrn_resolver.clone(), ) } @@ -1027,7 +1027,7 @@ impl Node { self.bolt12_payment(), Arc::clone(&self.config), Arc::clone(&self.logger), - Arc::clone(&self.hrn_resolver), + self.hrn_resolver.clone(), )) } diff --git a/src/payment/unified.rs b/src/payment/unified.rs index 9352ee974..b9229c347 100644 --- a/src/payment/unified.rs +++ b/src/payment/unified.rs @@ -74,7 +74,7 @@ pub struct UnifiedPayment { bolt12_payment: Arc, config: Arc, logger: Arc, - hrn_resolver: Arc, + hrn_resolver: HRNResolver, #[cfg(hrn_tests)] test_offer: std::sync::Mutex>, } @@ -83,7 +83,7 @@ impl UnifiedPayment { pub(crate) fn new( onchain_payment: Arc, bolt11_invoice: Arc, bolt12_payment: Arc, config: Arc, logger: Arc, - hrn_resolver: Arc, + hrn_resolver: HRNResolver, ) -> Self { Self { onchain_payment, @@ -197,7 +197,7 @@ impl UnifiedPayment { } let parse_fut = - PaymentInstructions::parse(uri_str, target_network, self.hrn_resolver.as_ref(), false); + PaymentInstructions::parse(uri_str, target_network, &self.hrn_resolver, false); let instructions = tokio::time::timeout(Duration::from_secs(HRN_RESOLUTION_TIMEOUT_SECS), parse_fut) @@ -223,7 +223,7 @@ impl UnifiedPayment { Error::InvalidAmount })?; - let fut = instr.set_amount(amt, self.hrn_resolver.as_ref()); + let fut = instr.set_amount(amt, &self.hrn_resolver); tokio::time::timeout(Duration::from_secs(HRN_RESOLUTION_TIMEOUT_SECS), fut) .await diff --git a/src/types.rs b/src/types.rs index 5d5515dcc..962be7f64 100644 --- a/src/types.rs +++ b/src/types.rs @@ -330,6 +330,7 @@ pub(crate) type OnionMessenger = lightning::onion_message::messenger::OnionMesse IgnoringMessageHandler, >; +#[derive(Clone)] pub enum HRNResolver { Onion(Arc, Arc>>), Local(Arc), From de65d2f82f3bfd023edf0b0f502357b3e3afd4fe Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 22 Apr 2026 16:31:00 +0100 Subject: [PATCH 3/5] Use ok_or_else for lazy error logging in DNS resolution Optimizes the DNS resolver setup by deferring log execution until an actual failure occurs. Changes: - Replaced `ok_or` with `ok_or_else` when resolving DNS server addresses. evaluation and unnecessary string formatting on successful builds. This ensures that logging overhead and string interpolation are only incurred during error states, rather than being evaluated on every successful initialization. --- src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder.rs b/src/builder.rs index befa2f260..0b44dc153 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1757,7 +1757,7 @@ fn build_with_store_internal( .to_socket_addrs() .map_err(|_| BuildError::DNSResolverSetupFailed)? .next() - .ok_or({ + .ok_or_else(|| { log_error!(logger, "No valid address found for: {}", dns_server_address); BuildError::DNSResolverSetupFailed })?; From 5d271766b2e5c7b7a47a22fad2d1fa240ce2e95e Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 22 Apr 2026 16:37:51 +0100 Subject: [PATCH 4/5] Fix HRN configuration defaults in docs and table formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates the documentation in config.rs for better accuracy and  readability. Changes: - Corrected the description of HRN resolution_config from 'Enabled'    to 'Disabled' to align with the actual default value of false. - Realigned the Config defaults table to accommodate longer    type names and improve Markdown rendering. These changes are strictly documentation-focused and do not  alter any runtime behavior. --- src/config.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/config.rs b/src/config.rs index 6f786d764..014d6216a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -118,18 +118,18 @@ pub(crate) const LNURL_AUTH_TIMEOUT_SECS: u64 = 15; /// ### Defaults /// /// | Parameter | Value | -/// |----------------------------------------|--------------------| -/// | `storage_dir_path` | /tmp/ldk_node/ | -/// | `network` | Bitcoin | -/// | `listening_addresses` | None | -/// | `announcement_addresses` | None | -/// | `node_alias` | None | -/// | `trusted_peers_0conf` | [] | -/// | `probing_liquidity_limit_multiplier` | 3 | -/// | `anchor_channels_config` | Some(..) | -/// | `route_parameters` | None | -/// | `tor_config` | None | -/// | `hrn_config` | HumanReadableNamesConfig::default() | +/// |----------------------------------------|--------------------------------------| +/// | `storage_dir_path` | /tmp/ldk_node/ | +/// | `network` | Bitcoin | +/// | `listening_addresses` | None | +/// | `announcement_addresses` | None | +/// | `node_alias` | None | +/// | `trusted_peers_0conf` | [] | +/// | `probing_liquidity_limit_multiplier` | 3 | +/// | `anchor_channels_config` | Some(..) | +/// | `route_parameters` | None | +/// | `tor_config` | None | +/// | `hrn_config` | HumanReadableNamesConfig::default() | /// /// See [`AnchorChannelsConfig`] and [`RouteParametersConfig`] for more information regarding their /// respective default values. @@ -264,7 +264,7 @@ pub struct HumanReadableNamesConfig { /// /// By default, this uses the `Dns` variant with the following settings: /// * **DNS Server**: `8.8.8.8:53` (Google Public DNS) - /// * **Resolution Service**: Enabled (`false`) + /// * **Resolution Service**: Disabled (`false`) pub resolution_config: HRNResolverConfig, } From e29ae44677c5f0c68b958f6d6a3006989565da7e Mon Sep 17 00:00:00 2001 From: Chuks Agbakuru Date: Wed, 22 Apr 2026 16:40:05 +0100 Subject: [PATCH 5/5] Remove unused variable prefix in set_test_offer Cleans up the set_test_offer method by removing the underscore from the offer parameter name. Changes: - Renamed `_offer` to `offer` to reflect that the variable is actively used within the method body. - Ensures the code adheres to standard Rust naming conventions for used variables. This is a minor cleanup of the test-only API used for HRN integration testing. --- src/payment/unified.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/payment/unified.rs b/src/payment/unified.rs index b9229c347..3708afe8e 100644 --- a/src/payment/unified.rs +++ b/src/payment/unified.rs @@ -350,8 +350,8 @@ impl UnifiedPayment { /// offers allow us to bypass this resolution step and test the subsequent payment flow. /// /// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki - pub fn set_test_offer(&self, _offer: Offer) { - let _ = self.test_offer.lock().map(|mut guard| *guard = Some(_offer)).map_err(|e| { + pub fn set_test_offer(&self, offer: Offer) { + let _ = self.test_offer.lock().map(|mut guard| *guard = Some(offer)).map_err(|e| { log_error!(self.logger, "Failed to set test offer due to poisoned lock: {:?}", e) }); }