Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .github/workflows/hrn-integration.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: CI Checks - HRN Integration Tests

on: [push, pull_request]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
build-and-test:
runs-on: ubuntu-latest

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Please avoid whitespace lines between steps.

steps:
- name: Checkout source code
uses: actions/checkout@v3
- name: Install Rust stable toolchain
run: |
curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain stable
- name: Enable caching for bitcoind
id: cache-bitcoind
uses: actions/cache@v4
with:
path: bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
key: bitcoind-29.0-${{ runner.os }}-${{ runner.arch }}
- name: Enable caching for electrs
id: cache-electrs
uses: actions/cache@v4
with:
path: bin/electrs-${{ runner.os }}-${{ runner.arch }}
key: electrs-${{ runner.os }}-${{ runner.arch }}
- name: Download bitcoind/electrs
if: "steps.cache-bitcoind.outputs.cache-hit != 'true' || steps.cache-electrs.outputs.cache-hit != 'true'"
run: |
source ./scripts/download_bitcoind_electrs.sh
mkdir -p bin
mv "$BITCOIND_EXE" bin/bitcoind-${{ runner.os }}-${{ runner.arch }}
mv "$ELECTRS_EXE" bin/electrs-${{ runner.os }}-${{ runner.arch }}
- name: Set bitcoind/electrs environment variables
run: |
echo "BITCOIND_EXE=$( pwd )/bin/bitcoind-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
echo "ELECTRS_EXE=$( pwd )/bin/electrs-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV"
- name: Run HRN Integration Tests
run: |
RUSTFLAGS="--cfg no_download --cfg hrn_tests $RUSTFLAGS" cargo test --test integration_tests_hrn
RUSTFLAGS="--cfg no_download --cfg hrn_tests $RUSTFLAGS" cargo test --test integration_tests_hrn --features uniffi
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,4 @@ jobs:
- uses: actions/checkout@v6
- uses: dtolnay/rust-toolchain@nightly
- uses: dtolnay/install@cargo-docs-rs
- run: cargo docs-rs
- run: cargo docs-rs
7 changes: 7 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ default = []
#lightning-transaction-sync = { version = "0.2.0", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
#lightning-liquidity = { version = "0.2.0", features = ["std"] }
#lightning-macros = { version = "0.2.0" }
#lightning-dns-resolver = { version = "0.3.0" }

lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6" }
Expand All @@ -50,6 +51,7 @@ lightning-block-sync = { git = "https://github.com/lightningdevkit/rust-lightnin
lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6" }
lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "38a62c32454d3eac22578144c479dbf9a6d9bff6" }

bdk_chain = { version = "0.23.0", default-features = false, features = ["std"] }
bdk_esplora = { version = "0.22.0", default-features = false, features = ["async-https-rustls", "tokio"]}
Expand Down Expand Up @@ -126,6 +128,7 @@ check-cfg = [
"cfg(cln_test)",
"cfg(lnd_test)",
"cfg(cycle_tests)",
"cfg(hrn_tests)",
]

[[bench]]
Expand All @@ -144,6 +147,7 @@ harness = false
#lightning-transaction-sync = { path = "../rust-lightning/lightning-transaction-sync" }
#lightning-liquidity = { path = "../rust-lightning/lightning-liquidity" }
#lightning-macros = { path = "../rust-lightning/lightning-macros" }
#lightning-dns-resolver = { path = "../rust-lightning/lightning-dns-resolver" }

#lightning = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }
#lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }
Expand All @@ -156,6 +160,7 @@ harness = false
#lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }
#lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }
#lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }
#lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", branch = "main" }

#lightning = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }
#lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }
Expand All @@ -168,6 +173,7 @@ harness = false
#lightning-transaction-sync = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }
#lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }
#lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }
#lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "21e9a9c0ef80021d0669f2a366f55d08ba8d9b03" }

#vss-client-ng = { path = "../vss-client" }
#vss-client-ng = { git = "https://github.com/lightningdevkit/vss-client", branch = "main" }
Expand All @@ -184,3 +190,4 @@ harness = false
#lightning-transaction-sync = { path = "../rust-lightning/lightning-transaction-sync" }
#lightning-liquidity = { path = "../rust-lightning/lightning-liquidity" }
#lightning-macros = { path = "../rust-lightning/lightning-macros" }
#lightning-dns-resolver = { path = "../rust-lightning/lightning-dns-resolver" }
4 changes: 4 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -416,3 +416,7 @@ typedef string LSPSDateTime;
typedef string ScriptBuf;

typedef enum Event;

typedef interface HRNResolverConfig;

typedef dictionary HumanReadableNamesConfig;
100 changes: 85 additions & 15 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
use std::collections::HashMap;
use std::convert::TryInto;
use std::default::Default;
use std::net::ToSocketAddrs;
use std::path::PathBuf;
use std::sync::{Arc, Mutex, Once, RwLock};
use std::sync::{Arc, Mutex, Once, RwLock, Weak};
use std::time::SystemTime;
use std::{fmt, fs};

Expand All @@ -19,12 +20,14 @@ use bitcoin::bip32::{ChildNumber, Xpriv};
use bitcoin::key::Secp256k1;
use bitcoin::secp256k1::PublicKey;
use bitcoin::{BlockHash, Network};
use bitcoin_payment_instructions::dns_resolver::DNSHrnResolver;
use bitcoin_payment_instructions::onion_message_resolver::LDKOnionMessageDNSSECHrnResolver;
use lightning::chain::{chainmonitor, BestBlock};
use lightning::ln::channelmanager::{self, ChainParameters, ChannelManagerReadArgs};
use lightning::ln::msgs::{RoutingMessageHandler, SocketAddress};
use lightning::ln::peer_handler::{IgnoringMessageHandler, MessageHandler};
use lightning::log_trace;
use lightning::onion_message::dns_resolution::DNSResolverMessageHandler;
use lightning::routing::gossip::NodeAlias;
use lightning::routing::router::DefaultRouter;
use lightning::routing::scoring::{
Expand All @@ -39,14 +42,15 @@ use lightning::util::persist::{
};
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
use crate::config::{
default_user_config, may_announce_channel, AnnounceError, AsyncPaymentsRole,
BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, TorConfig,
DEFAULT_ESPLORA_SERVER_URL, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL,
BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, HRNResolverConfig,
TorConfig, DEFAULT_ESPLORA_SERVER_URL, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL,
};
use crate::connection::ConnectionManager;
use crate::entropy::NodeEntropy;
Expand Down Expand Up @@ -77,8 +81,8 @@ use crate::runtime::{Runtime, RuntimeSpawner};
use crate::tx_broadcaster::TransactionBroadcaster;
use crate::types::{
AsyncPersister, ChainMonitor, ChannelManager, DynStore, DynStoreRef, DynStoreWrapper,
GossipSync, Graph, KeysManager, MessageRouter, OnionMessenger, PaymentStore, PeerManager,
PendingPaymentStore, SyncAndAsyncKVStore,
GossipSync, Graph, HRNResolver, KeysManager, MessageRouter, OnionMessenger, PaymentStore,
PeerManager, PendingPaymentStore, SyncAndAsyncKVStore,
};
use crate::wallet::persist::KVStoreWalletPersister;
use crate::wallet::Wallet;
Expand Down Expand Up @@ -195,6 +199,8 @@ pub enum BuildError {
NetworkMismatch,
/// The role of the node in an asynchronous payments context is not compatible with the current configuration.
AsyncPaymentsConfigMismatch,
/// An attempt to setup a DNS Resolver failed.
DNSResolverSetupFailed,
}

impl fmt::Display for BuildError {
Expand Down Expand Up @@ -229,6 +235,9 @@ impl fmt::Display for BuildError {
"The async payments role is not compatible with the current configuration."
)
},
Self::DNSResolverSetupFailed => {
write!(f, "An attempt to setup a DNS resolver has failed.")
},
}
}
}
Expand Down Expand Up @@ -1726,7 +1735,71 @@ fn build_with_store_internal(
})?;
}

let hrn_resolver = Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph)));
// 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<Mutex<Option<Weak<PeerManager>>>> = Arc::new(Mutex::new(None));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude:

The Mutex<Option<Weak<PeerManager>>> hook is more complex than the original

  The old code simply registered the post-queue closure after peer_manager was built:

  let peer_manager_clone = Arc::downgrade(&peer_manager);
  hrn_resolver.register_post_queue_action(Box::new(move || {
      if let Some(upgraded) = peer_manager_clone.upgrade() { upgraded.process_events(); }
  }));

  That still works here — just keep a strong reference to the concrete LDKOnionMessageDNSSECHrnResolver (when Blip32 is selected) in a local Option
   next to hrn_resolver, then register post-PM-construction. No Mutex needed. Current shape (builder.rs:1738-1802) introduces a doc comment about
  reference cycles that the simpler form doesn't have to defend.

let hrn_resolver;

let runtime_handle = runtime.handle();

let om_resolver: Arc<dyn DNSResolverMessageHandler + Send + Sync> = match &config
.hrn_config
.resolution_config
{
HRNResolverConfig::Blip32 => {
let hrn_res =
Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph)));
hrn_resolver = HRNResolver::Onion(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<dyn DNSResolverMessageHandler + Send + Sync>
},
HRNResolverConfig::Dns { dns_server_address, enable_hrn_resolution_service, .. } => {
let addr = dns_server_address
.to_socket_addrs()
.map_err(|_| BuildError::DNSResolverSetupFailed)?
.next()
.ok_or({
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is evaluated eagerly, i.e., we'll log every time this is hit.

log_error!(logger, "No valid address found for: {}", dns_server_address);
BuildError::DNSResolverSetupFailed
})?;
let hrn_res = Arc::new(DNSHrnResolver(addr));
hrn_resolver = HRNResolver::Local(hrn_res);

if *enable_hrn_resolution_service {
if let Err(_) = may_announce_channel(&config) {
log_error!(
logger,
"HRN resolution service enabled, but node is not announceable."
);
return Err(BuildError::DNSResolverSetupFailed);
}

Arc::new(OMDomainResolver::<IgnoringMessageHandler>::with_runtime(
addr,
None,
Some(runtime_handle.clone()),
)) as Arc<dyn DNSResolverMessageHandler + Send + Sync>
} else {
// The user wants to use DNS to pay others, but NOT provide a service to others.
Arc::new(IgnoringMessageHandler {})
as Arc<dyn DNSResolverMessageHandler + Send + Sync>
}
},
};

// Initialize the PeerManager
let onion_messenger: Arc<OnionMessenger> =
Expand All @@ -1739,7 +1812,7 @@ fn build_with_store_internal(
message_router,
Arc::clone(&channel_manager),
Arc::clone(&channel_manager),
Arc::clone(&hrn_resolver),
Arc::clone(&om_resolver),
IgnoringMessageHandler {},
))
} else {
Expand All @@ -1751,7 +1824,7 @@ fn build_with_store_internal(
message_router,
Arc::clone(&channel_manager),
Arc::clone(&channel_manager),
Arc::clone(&hrn_resolver),
Arc::clone(&om_resolver),
IgnoringMessageHandler {},
))
};
Expand Down Expand Up @@ -1882,12 +1955,9 @@ fn build_with_store_internal(
Arc::clone(&keys_manager),
));

let peer_manager_clone = Arc::downgrade(&peer_manager);
hrn_resolver.register_post_queue_action(Box::new(move || {
if let Some(upgraded_pointer) = peer_manager_clone.upgrade() {
upgraded_pointer.process_events();
}
}));
if let Ok(mut guard) = peer_manager_hook.lock() {
*guard = Some(Arc::downgrade(&peer_manager));
}

liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager)));

Expand Down Expand Up @@ -2001,7 +2071,7 @@ fn build_with_store_internal(
node_metrics,
om_mailbox,
async_payments_role,
hrn_resolver,
hrn_resolver: Arc::new(hrn_resolver),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given HRNResolver still wraps everything as Arc, could we just hold HRNResolver by value?

#[cfg(cycle_tests)]
_leak_checker,
})
Expand Down
62 changes: 62 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! Objects for configuring the node.

use std::fmt;
use std::str::FromStr;
use std::time::Duration;

use bitcoin::secp256k1::PublicKey;
Expand Down Expand Up @@ -128,6 +129,7 @@ pub(crate) const LNURL_AUTH_TIMEOUT_SECS: u64 = 15;
/// | `anchor_channels_config` | Some(..) |
/// | `route_parameters` | None |
/// | `tor_config` | None |
/// | `hrn_config` | HumanReadableNamesConfig::default() |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This mentions HumanReadableNamesConfig::default() which doesn't actually exist as it doesn't implement Default. Also, please align the table columns again.

///
/// See [`AnchorChannelsConfig`] and [`RouteParametersConfig`] for more information regarding their
/// respective default values.
Expand Down Expand Up @@ -199,6 +201,10 @@ pub struct Config {
///
/// **Note**: If unset, connecting to peer OnionV3 addresses will fail.
pub tor_config: Option<TorConfig>,
/// Configuration options for Human-Readable Names ([BIP 353]).
///
/// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki
pub hrn_config: HumanReadableNamesConfig,
}

impl Default for Config {
Expand All @@ -214,6 +220,62 @@ impl Default for Config {
tor_config: None,
route_parameters: None,
node_alias: None,
hrn_config: HumanReadableNamesConfig::default(),
}
}
}

/// Configuration options for how our node resolves Human-Readable Names (BIP 353).
///
/// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki
#[derive(Debug, Clone)]
#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]
pub enum HRNResolverConfig {
/// Use [bLIP-32] to ask other nodes to resolve names for us.
///
/// [bLIP-32]: https://github.com/lightning/blips/blob/master/blip-0032.md
Blip32,
/// Resolve names locally using a specific DNS server.
Dns {
/// The IP and port of the DNS server.
///
/// **Default:** `8.8.8.8:53` (Google Public DNS)
dns_server_address: SocketAddress,
/// If set to true, this allows others to use our node for HRN resolutions.
///
/// **Default:** `false`
///
/// **Note:** Enabling `enable_hrn_resolution_service` allows your node to act
/// as a resolver for the rest of the network. For this to work, your node must
/// be announceable (publicly visible in the network graph) so that other nodes
/// can route resolution requests to you via Onion Messages. This does not affect
/// your node's ability to resolve names for its own outgoing payments.
enable_hrn_resolution_service: bool,
},
}

/// Configuration options for Human-Readable Names ([BIP 353]).
///
/// [BIP 353]: https://github.com/bitcoin/bips/blob/master/bip-0353.mediawiki
#[derive(Debug, Clone)]
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
pub struct HumanReadableNamesConfig {
/// This sets how our node resolves names when we want to send a payment.
///
/// 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`)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled (false) seems wrong.

pub resolution_config: HRNResolverConfig,
}

impl Default for HumanReadableNamesConfig {
fn default() -> Self {
HumanReadableNamesConfig {
resolution_config: HRNResolverConfig::Dns {
dns_server_address: SocketAddress::from_str("8.8.8.8:53")
.expect("Socket address conversion failed."),
enable_hrn_resolution_service: false,
},
}
}
}
Expand Down
Loading
Loading