-
Notifications
You must be signed in to change notification settings - Fork 115
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 1 commit
9ea92d5
b11fead
998bdab
55522b9
f1d1fa1
869839a
38e17d1
da52af8
7973b8c
6068c1c
2149475
0517dbc
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 |
---|---|---|
|
@@ -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; | ||
|
@@ -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::{ | ||
|
@@ -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; | ||
|
@@ -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 { | ||
|
@@ -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. | ||
/// | ||
|
@@ -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)?, | ||
))) | ||
} 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( | ||
|
@@ -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(); | ||
|
@@ -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, | ||
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, so if we're resolving for others, we won't be able to send to HRNs ourselves? 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. From my understanding, it seems this is the case. The onion messenger can take only one type of |
||
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))); | ||
|
||
|
@@ -1706,7 +1740,7 @@ fn build_with_store_internal( | |
is_running, | ||
is_listening, | ||
node_metrics, | ||
hrn_resolver, | ||
hrn_resolver: hrn_resolver.cloned(), | ||
}) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 } | ||
} | ||
|
@@ -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..."); | ||
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. Oh, please remove any 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. 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) | ||
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. We def. need to wrap this in a |
||
.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(|| { | ||
|
@@ -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 | ||
})? | ||
|
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.
We probably want to make the DNS server used configurable via the config?
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.
Yes that makes sense - will do.