Skip to content
Open
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
46 changes: 46 additions & 0 deletions .github/workflows/hrn-integration.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: CI Checks - HRN Integration Tests

on: [push, pull_request]

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

jobs:
hrn_integration:
name: HRN Tests - Ubuntu Stable
runs-on: ubuntu-latest

Copy link
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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's copy what we do over at rust.yml already.

run: |
rustup update stable
rustup default stable

- name: Enable caching for bitcoind/electrs
id: cache-deps
uses: actions/cache@v4
with:
path: bin/
key: hrn-deps-${{ runner.os }}-${{ hashFiles('scripts/download_bitcoind_electrs.sh') }}

- name: Download bitcoind/electrs
if: steps.cache-deps.outputs.cache-hit != 'true'
run: |
source ./scripts/download_bitcoind_electrs.sh
mkdir -p bin
mv "$BITCOIND_EXE" bin/bitcoind
mv "$ELECTRS_EXE" bin/electrs

- name: Set environment variables
run: |
echo "BITCOIND_EXE=$(pwd)/bin/bitcoind" >> "$GITHUB_ENV"
echo "ELECTRS_EXE=$(pwd)/bin/electrs" >> "$GITHUB_ENV"

- name: Run HRN Integration Tests
env:
RUSTFLAGS: "--cfg no_download --cfg hrn_tests"
run: cargo test --test integration_tests_hrn
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 = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0", features = ["std"] }
lightning-types = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0" }
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 = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0", features = ["esplora-async-https", "time", "electrum-rustls-ring"] }
lightning-liquidity = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0", features = ["std"] }
lightning-macros = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0" }
lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightning", rev = "b6c17c593a5d7bacb18fe3b9f69074a0596ae8f0" }

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 @@ -123,6 +125,7 @@ check-cfg = [
"cfg(cln_test)",
"cfg(lnd_test)",
"cfg(cycle_tests)",
"cfg(hrn_tests)",
]

[[bench]]
Expand All @@ -141,6 +144,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 @@ -153,6 +157,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 @@ -165,6 +170,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 @@ -181,3 +187,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" }
15 changes: 15 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ dictionary Config {
u64 probing_liquidity_limit_multiplier;
AnchorChannelsConfig? anchor_channels_config;
RouteParametersConfig? route_parameters;
HumanReadableNamesConfig? hrn_config;
};

dictionary AnchorChannelsConfig {
Expand Down Expand Up @@ -364,6 +365,8 @@ enum NodeError {
"InvalidBlindedPaths",
"AsyncPaymentServicesDisabled",
"HrnParsingFailed",
"HrnResolverNotConfigured",
"NoDnsResolvers",
};

dictionary NodeStatus {
Expand Down Expand Up @@ -398,6 +401,8 @@ enum BuildError {
"LoggerSetupFailed",
"NetworkMismatch",
"AsyncPaymentsConfigMismatch",
"DNSResolverSetupFailed",
"PeerManagerSetupFailed",
};

[Trait]
Expand Down Expand Up @@ -517,6 +522,16 @@ dictionary RouteParametersConfig {
u8 max_channel_saturation_power_of_half;
};

[Enum]
interface HRNResolverConfig {
Blip32();
Dns(string dns_server_address, boolean enable_hrn_resolution_service);
};

dictionary HumanReadableNamesConfig {
HRNResolverConfig resolution_config;
};

dictionary CustomTlvRecord {
u64 type_num;
sequence<u8> value;
Expand Down
111 changes: 95 additions & 16 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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::{
Expand All @@ -39,13 +40,15 @@ 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::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,
BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, HRNResolverConfig,
DEFAULT_ESPLORA_SERVER_URL, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL,
};
use crate::connection::ConnectionManager;
Expand Down Expand Up @@ -76,8 +79,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;
Expand Down Expand Up @@ -189,6 +192,10 @@ 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,
/// Failed to set up the peer manager.
PeerManagerSetupFailed,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we'd ever want to run into this case. It's also very unclear what happened if we do.

}

impl fmt::Display for BuildError {
Expand Down Expand Up @@ -221,6 +228,12 @@ 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.")
},
Self::PeerManagerSetupFailed => {
write!(f, "Failed to set up the peer manager.")
},
}
}
}
Expand Down Expand Up @@ -1545,7 +1558,74 @@ fn build_with_store_internal(
})?;
}

let hrn_resolver = Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph)));
let peer_manager_hook: Arc<Mutex<Option<Weak<PeerManager>>>> = Arc::new(Mutex::new(None));
let mut hrn_resolver_out = None;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, a comment here could help explaining what we're doing here exactly. Also not quite sure why hrn_resolver_out is preferable to hrn_resolver, esp. if there is no hrn_resolver_in counterpart or simiar?


let om_resolver: Arc<dyn DNSResolverMessageHandler + Send + Sync> = match &config.hrn_config {
None => Arc::new(IgnoringMessageHandler {}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we just default to DNS resolution?

Some(hrn_config) => {
let runtime_handle = runtime.handle();

let client_resolver: Arc<dyn DNSResolverMessageHandler + Send + Sync> =
match &hrn_config.resolution_config {
HRNResolverConfig::Blip32 => {
let hrn_res = Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(
&network_graph,
)));
hrn_resolver_out = Some(Arc::new(HRNResolver::Onion(Arc::clone(&hrn_res))));

let pm_hook_clone = Arc::clone(&peer_manager_hook);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to clone this?

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, .. } => {
let addr = dns_server_address
.parse()
.map_err(|_| BuildError::DNSResolverSetupFailed)?;
let hrn_res = Arc::new(DNSHrnResolver(addr));
hrn_resolver_out = Some(Arc::new(HRNResolver::Local(hrn_res)));

let resolver =
Arc::new(OMDomainResolver::<IgnoringMessageHandler>::with_runtime(
addr,
None,
Some(runtime_handle.clone()),
));
resolver as Arc<dyn DNSResolverMessageHandler + Send + Sync>
},
};

if let HRNResolverConfig::Dns {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, shouldn't we just move this into the large match we already have above?

enable_hrn_resolution_service: true,
ref dns_server_address,
..
} = hrn_config.resolution_config
{
if may_announce_channel(&config).is_ok() {
let service_dns_addr = dns_server_address
.parse()
.map_err(|_| BuildError::DNSResolverSetupFailed)?;

Arc::new(OMDomainResolver::with_runtime(
service_dns_addr,
Some(client_resolver),
Some(runtime_handle.clone()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we can avoid cloning here?

)) as Arc<dyn DNSResolverMessageHandler + Send + Sync>
} else {
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.");
client_resolver
}
} else {
client_resolver
}
},
};

// Initialize the PeerManager
let onion_messenger: Arc<OnionMessenger> =
Expand All @@ -1558,7 +1638,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 @@ -1570,7 +1650,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 @@ -1691,7 +1771,7 @@ fn build_with_store_internal(
BuildError::InvalidSystemTime
})?;

let peer_manager = Arc::new(PeerManager::new(
let peer_manager: Arc<PeerManager> = Arc::new(PeerManager::new(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is turbofishing really necessary here?

msg_handler,
cur_time.as_secs().try_into().map_err(|e| {
log_error!(logger, "Failed to get current time: {}", e);
Expand All @@ -1702,12 +1782,11 @@ 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() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should just unwrap on the locking parts, really PeerManagerSetupFailed is not an error variant we'd ever want to expose.

*guard = Some(Arc::downgrade(&peer_manager));
} else {
return Err(BuildError::PeerManagerSetupFailed);
}

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

Expand Down Expand Up @@ -1814,7 +1893,7 @@ fn build_with_store_internal(
node_metrics,
om_mailbox,
async_payments_role,
hrn_resolver,
hrn_resolver: Arc::new(hrn_resolver_out),
#[cfg(cycle_tests)]
_leak_checker,
})
Expand Down
Loading
Loading