-
Notifications
You must be signed in to change notification settings - Fork 130
Add support for resolving BIP 353 Human-Readable Names #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
02307a1
8985adf
a9ee7d4
1006340
63d1f0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| 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 | ||
|
|
||
| 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-${{ 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -269,6 +269,10 @@ dictionary RouteParametersConfig { | |
| u8 max_channel_saturation_power_of_half; | ||
| }; | ||
|
|
||
| typedef interface HRNResolverConfig; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Please move these down to the other |
||
|
|
||
| typedef dictionary HumanReadableNamesConfig; | ||
|
|
||
| [Remote] | ||
| dictionary LSPS1OrderStatus { | ||
| LSPS1OrderId order_id; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ use std::collections::HashMap; | |
| use std::convert::TryInto; | ||
| use std::default::Default; | ||
| 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}; | ||
|
|
||
|
|
@@ -19,12 +19,13 @@ 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::{ | ||
|
|
@@ -39,14 +40,16 @@ use lightning::util::persist::{ | |
| }; | ||
| use lightning::util::ser::ReadableArgs; | ||
| use lightning::util::sweep::OutputSweeper; | ||
| use lightning::{log_trace, log_warn}; | ||
| 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; | ||
|
|
@@ -77,8 +80,8 @@ use crate::runtime::{Runtime, RuntimeSpawner}; | |
| use crate::tx_broadcaster::TransactionBroadcaster; | ||
| use crate::types::{ | ||
| AsyncPersister, ChainMonitor, ChannelManager, DynStore, DynStoreWrapper, GossipSync, Graph, | ||
| KeysManager, MessageRouter, OnionMessenger, PaymentStore, PeerManager, PendingPaymentStore, | ||
| Persister, SyncAndAsyncKVStore, | ||
| HRNResolver, KeysManager, MessageRouter, OnionMessenger, PaymentStore, PeerManager, | ||
| PendingPaymentStore, Persister, SyncAndAsyncKVStore, | ||
| }; | ||
| use crate::wallet::persist::KVStoreWalletPersister; | ||
| use crate::wallet::Wallet; | ||
|
|
@@ -193,6 +196,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 { | ||
|
|
@@ -226,6 +231,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.") | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -1722,7 +1730,75 @@ 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)); | ||
| 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.parse().map_err(|_| { | ||
| log_error!(logger, "Failed to parse DNS server address: {}", dns_server_address); | ||
| BuildError::DNSResolverSetupFailed | ||
| })?; | ||
|
|
||
| if *enable_hrn_resolution_service && may_announce_channel(&config).is_ok() { | ||
| let hrn_res = Arc::new(DNSHrnResolver(addr)); | ||
| hrn_resolver = HRNResolver::Local(hrn_res); | ||
|
|
||
| Arc::new(OMDomainResolver::<IgnoringMessageHandler>::with_runtime( | ||
| addr, | ||
| None, | ||
| Some(runtime_handle.clone()), | ||
| )) as Arc<dyn DNSResolverMessageHandler + Send + Sync> | ||
| } else { | ||
| if *enable_hrn_resolution_service { | ||
| log_warn!(logger, "Unable to act as an HRN resolution service. To act as an HRN resolution service, the node must be configured to announce channels."); | ||
| } | ||
|
|
||
| // Fallback/Default: Onion resolver | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, no? We should fallback to not act as a resolver, but still use local resolution if the user configured it, no? I do also wonder if we should error out if the user enabled resolution but can't announce it? |
||
| let hrn_res = | ||
| Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph))); | ||
| hrn_resolver = HRNResolver::Onion(Arc::clone(&hrn_res)); | ||
|
|
||
| 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> | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| // Initialize the PeerManager | ||
| let onion_messenger: Arc<OnionMessenger> = | ||
|
|
@@ -1735,7 +1811,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 { | ||
|
|
@@ -1747,7 +1823,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 {}, | ||
| )) | ||
| }; | ||
|
|
@@ -1879,12 +1955,7 @@ 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(); | ||
| } | ||
| })); | ||
| *peer_manager_hook.lock().unwrap() = Some(Arc::downgrade(&peer_manager)); | ||
|
|
||
| liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager))); | ||
|
|
||
|
|
@@ -1998,7 +2069,7 @@ fn build_with_store_internal( | |
| node_metrics, | ||
| om_mailbox, | ||
| async_payments_role, | ||
| hrn_resolver, | ||
| hrn_resolver: Arc::new(hrn_resolver), | ||
| #[cfg(cycle_tests)] | ||
| _leak_checker, | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,7 @@ pub(crate) const LNURL_AUTH_TIMEOUT_SECS: u64 = 15; | |
| /// | `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. | ||
|
|
@@ -199,6 +200,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 { | ||
|
|
@@ -214,6 +219,56 @@ 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) when acting as a client. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Mentioning 'when acting as a client' is a bit confusing here, as below |
||
| /// | ||
| /// [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: String, | ||
| /// If set to true, this allows others to use our node for HRN resolutions. | ||
| /// | ||
| /// **Note:** Enabling `enable_hrn_resolution_service` is only one part of the | ||
| /// configuration. For resolution to function correctly, the local node must | ||
| /// also be configured as an **announceable node** within the network. | ||
| 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 (`true`) | ||
| pub resolution_config: HRNResolverConfig, | ||
| } | ||
|
|
||
| impl Default for HumanReadableNamesConfig { | ||
| fn default() -> Self { | ||
| HumanReadableNamesConfig { | ||
| resolution_config: HRNResolverConfig::Dns { | ||
| dns_server_address: "8.8.8.8:53".to_string(), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please document the default choices in the |
||
| enable_hrn_resolution_service: true, | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
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.