Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ enum NodeError {
"InsufficientFunds",
"LiquiditySourceUnavailable",
"LiquidityFeeTooHigh",
"HrnResolverNotConfigured",
};

dictionary NodeStatus {
Expand Down Expand Up @@ -348,6 +349,7 @@ enum BuildError {
"WalletSetupFailed",
"LoggerSetupFailed",
"NetworkMismatch",
"DNSResolverSetupFailed",
};

[Trait]
Expand Down
50 changes: 42 additions & 8 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use crate::peer_store::PeerStore;
use crate::runtime::Runtime;
use crate::tx_broadcaster::TransactionBroadcaster;
use crate::types::{
ChainMonitor, ChannelManager, DynStore, GossipSync, Graph, KeysManager, MessageRouter,
OnionMessenger, PaymentStore, PeerManager,
ChainMonitor, ChannelManager, DomainResolver, DynStore, GossipSync, Graph, HRNResolver,
KeysManager, MessageRouter, OnionMessenger, PaymentStore, PeerManager,
};
use crate::wallet::persist::KVStoreWalletPersister;
use crate::wallet::Wallet;
Expand All @@ -43,6 +43,7 @@ use lightning::io::Cursor;
use lightning::ln::channelmanager::{self, ChainParameters, ChannelManagerReadArgs};
use lightning::ln::msgs::{RoutingMessageHandler, SocketAddress};
use lightning::ln::peer_handler::{IgnoringMessageHandler, MessageHandler};
use lightning::onion_message::dns_resolution::DNSResolverMessageHandler;
use lightning::routing::gossip::NodeAlias;
use lightning::routing::router::DefaultRouter;
use lightning::routing::scoring::{
Expand All @@ -59,6 +60,8 @@ use lightning::util::sweep::OutputSweeper;

use lightning_persister::fs_store::FilesystemStore;

use lightning_dns_resolver::OMDomainResolver;

use bdk_wallet::template::Bip84;
use bdk_wallet::KeychainKind;
use bdk_wallet::Wallet as BdkWallet;
Expand Down Expand Up @@ -193,6 +196,8 @@ pub enum BuildError {
LoggerSetupFailed,
/// The given network does not match the node's previously configured network.
NetworkMismatch,
/// An attempt to setup a DNS Resolver failed.
DNSResolverSetupFailed,
}

impl fmt::Display for BuildError {
Expand Down Expand Up @@ -221,12 +226,20 @@ impl fmt::Display for BuildError {
Self::NetworkMismatch => {
write!(f, "Given network does not match the node's previously configured network.")
},
Self::DNSResolverSetupFailed => {
write!(f, "An attempt to setup a DNS resolver has failed.")
},
}
}
}

impl std::error::Error for BuildError {}

enum Resolver {
HRN(Arc<HRNResolver>),
DNS(Arc<DomainResolver>),
}

/// A builder for an [`Node`] instance, allowing to set some configuration and module choices from
/// the getgo.
///
Expand Down Expand Up @@ -1456,7 +1469,22 @@ fn build_with_store_internal(
})?;
}

let hrn_resolver = Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph)));
let resolver = if config.is_hrn_resolver {
Resolver::DNS(Arc::new(OMDomainResolver::ignoring_incoming_proofs(
"8.8.8.8:53".parse().map_err(|_| BuildError::DNSResolverSetupFailed)?,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want to make the DNS server used configurable via the config?

Copy link
Author

Choose a reason for hiding this comment

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

Yes that makes sense - will do.

)))
} else {
Resolver::HRN(Arc::new(LDKOnionMessageDNSSECHrnResolver::new(Arc::clone(&network_graph))))
};

let om_resolver = match resolver {
Resolver::DNS(ref dns_resolver) => {
Arc::clone(dns_resolver) as Arc<dyn DNSResolverMessageHandler + Send + Sync>
},
Resolver::HRN(ref hrn_resolver) => {
Arc::clone(hrn_resolver) as Arc<dyn DNSResolverMessageHandler + Send + Sync>
},
};

// Initialize the PeerManager
let onion_messenger: Arc<OnionMessenger> = Arc::new(OnionMessenger::new(
Expand All @@ -1467,7 +1495,7 @@ fn build_with_store_internal(
message_router,
Arc::clone(&channel_manager),
IgnoringMessageHandler {},
Arc::clone(&hrn_resolver),
Arc::clone(&om_resolver),
IgnoringMessageHandler {},
));
let ephemeral_bytes: [u8; 32] = keys_manager.get_secure_random_bytes();
Expand Down Expand Up @@ -1595,9 +1623,15 @@ fn build_with_store_internal(

let peer_manager_clone = Arc::clone(&peer_manager);

hrn_resolver.register_post_queue_action(Box::new(move || {
peer_manager_clone.process_events();
}));
let hrn_resolver = match resolver {
Resolver::DNS(_) => None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, so if we're resolving for others, we won't be able to send to HRNs ourselves?

Copy link
Author

Choose a reason for hiding this comment

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

From my understanding, it seems this is the case. The onion messenger can take only one type of dns_resolver at a time - either the one sending out the query or the one handling the query and responding with the proof. If our node is configured to resolve for others, we'd have to pass the appropriate resolver to the onion messenger, preventing us from sending to HRNs ourselves.

Resolver::HRN(ref hrn_resolver) => {
hrn_resolver.register_post_queue_action(Box::new(move || {
peer_manager_clone.process_events();
}));
Some(hrn_resolver)
},
};

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

Expand Down Expand Up @@ -1706,7 +1740,7 @@ fn build_with_store_internal(
is_running,
is_listening,
node_metrics,
hrn_resolver,
hrn_resolver: hrn_resolver.cloned(),
})
}

Expand Down
5 changes: 5 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ pub enum Error {
LiquiditySourceUnavailable,
/// The given operation failed due to the LSP's required opening fee being too high.
LiquidityFeeTooHigh,
/// A HRN resolver was not configured
HrnResolverNotConfigured,
}

impl fmt::Display for Error {
Expand Down Expand Up @@ -193,6 +195,9 @@ impl fmt::Display for Error {
Self::LiquidityFeeTooHigh => {
write!(f, "The given operation failed due to the LSP's required opening fee being too high.")
},
Self::HrnResolverNotConfigured => {
write!(f, "A HRN resolver was not configured.")
},
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ pub struct Node {
is_running: Arc<RwLock<bool>>,
is_listening: Arc<AtomicBool>,
node_metrics: Arc<RwLock<NodeMetrics>>,
hrn_resolver: Arc<HRNResolver>,
hrn_resolver: Option<Arc<HRNResolver>>,
}

impl Node {
Expand Down Expand Up @@ -901,7 +901,7 @@ impl Node {
self.bolt12_payment().into(),
Arc::clone(&self.config),
Arc::clone(&self.logger),
Arc::clone(&self.hrn_resolver),
Arc::new(self.hrn_resolver.clone()),
)
}

Expand All @@ -922,7 +922,7 @@ impl Node {
self.bolt12_payment(),
Arc::clone(&self.config),
Arc::clone(&self.logger),
Arc::clone(&self.hrn_resolver),
Arc::new(self.hrn_resolver.clone()),
))
}

Expand Down
32 changes: 19 additions & 13 deletions src/payment/unified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ pub struct UnifiedPayment {
bolt12_payment: Arc<Bolt12Payment>,
config: Arc<Config>,
logger: Arc<Logger>,
hrn_resolver: Arc<HRNResolver>,
hrn_resolver: Arc<Option<Arc<HRNResolver>>>,
}

impl UnifiedPayment {
pub(crate) fn new(
onchain_payment: Arc<OnchainPayment>, bolt11_invoice: Arc<Bolt11Payment>,
bolt12_payment: Arc<Bolt12Payment>, config: Arc<Config>, logger: Arc<Logger>,
hrn_resolver: Arc<HRNResolver>,
hrn_resolver: Arc<Option<Arc<HRNResolver>>>,
) -> Self {
Self { onchain_payment, bolt11_invoice, bolt12_payment, config, logger, hrn_resolver }
}
Expand Down Expand Up @@ -155,18 +155,24 @@ impl UnifiedPayment {
pub async fn send(
&self, uri_str: &str, amount_msat: Option<u64>,
) -> Result<UnifiedPaymentResult, Error> {
let instructions = PaymentInstructions::parse(
uri_str,
self.config.network,
self.hrn_resolver.as_ref(),
false,
)
.await
.map_err(|e| {
log_error!(self.logger, "Failed to parse payment instructions: {:?}", e);
Error::UriParameterParsingFailed
let resolver = self.hrn_resolver.as_ref().clone().ok_or_else(|| {
log_error!(self.logger, "No HRN resolver configured. Cannot resolve HRNs.");
Error::HrnResolverNotConfigured
})?;

println!("Parsing instructions...");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, please remove any printlns.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah - will do, just left this here because I am still debugging the issue with the end-to-end test not resolving HRNs, erroring out or timing out.


let instructions =
PaymentInstructions::parse(uri_str, self.config.network, resolver.as_ref(), false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We def. need to wrap this in a tokio::timeout to abandon the future if it never returns.

.await
.map_err(|e| {
log_error!(self.logger, "Failed to parse payment instructions: {:?}", e);
println!("Failed to parse payment instructions: {:?}", e);
Error::UriParameterParsingFailed
})?;

println!("Sending...");

let resolved = match instructions {
PaymentInstructions::ConfigurableAmount(instr) => {
let amount = amount_msat.ok_or_else(|| {
Expand All @@ -179,7 +185,7 @@ impl UnifiedPayment {
Error::InvalidAmount
})?;

instr.set_amount(amt, self.hrn_resolver.as_ref()).await.map_err(|e| {
instr.set_amount(amt, resolver.as_ref()).await.map_err(|e| {
log_error!(self.logger, "Failed to set amount: {:?}", e);
Error::InvalidAmount
})?
Expand Down
6 changes: 5 additions & 1 deletion src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use lightning::ln::msgs::RoutingMessageHandler;
use lightning::ln::msgs::SocketAddress;
use lightning::ln::peer_handler::IgnoringMessageHandler;
use lightning::ln::types::ChannelId;
use lightning::onion_message::dns_resolution::DNSResolverMessageHandler;
use lightning::routing::gossip;
use lightning::routing::router::DefaultRouter;
use lightning::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters};
Expand All @@ -33,6 +34,8 @@ use lightning_block_sync::gossip::{GossipVerifier, UtxoSource};

use lightning_net_tokio::SocketDescriptor;

use lightning_dns_resolver::OMDomainResolver;

use bitcoin::secp256k1::PublicKey;
use bitcoin::OutPoint;

Expand Down Expand Up @@ -119,11 +122,12 @@ pub(crate) type OnionMessenger = lightning::onion_message::messenger::OnionMesse
Arc<MessageRouter>,
Arc<ChannelManager>,
IgnoringMessageHandler,
Arc<HRNResolver>,
Arc<dyn DNSResolverMessageHandler + Sync + Send>,
IgnoringMessageHandler,
>;

pub(crate) type HRNResolver = LDKOnionMessageDNSSECHrnResolver<Arc<Graph>, Arc<Logger>>;
pub(crate) type DomainResolver = OMDomainResolver<IgnoringMessageHandler>;

pub(crate) type MessageRouter = lightning::onion_message::messenger::DefaultMessageRouter<
Arc<Graph>,
Expand Down