Add support for resolving BIP 353 Human-Readable Names #630

chuksys · 2025-09-05T14:41:29Z

This PR makes it possible to configure LDK-Node as a DNS resolver for other nodes.

Changes

Added a bool flag named is_hrn_resolver to Config.
Based on the value of is_hrn_resolver, a new node either has the ability to send payments to HRNs or do DNS resolution for other nodes.
Expanded setup_two_nodes in the integration test suite to allow configuring a node to act as a DNS resolver for other nodes.
Added an end-to-end test-case asserting that sending to HRNs works.

This PR fixes #436

This rename reflects that this module is a unified payment interface for both QR code payments and HRN payments passed in as a string without scanning a QR code

…UnifiedPaymentResult These renamings are necessary to reflect the expanded responsibilities for this module.

This commit adds a HRN Resolver to the Node struct which will be useful for resolving HRNs when making BIP 353 payments. It also passes the HRN Resolver into UnifiedPayment.

…353 HRNs

This commit allows us to enable our node to either be able to send HRN resolution requests or acts as a resolver for 3rd party nodes We also pass HRNResolver as an optional parameter to Node

This commit will be dropped later

This commit adds an end-to-end test that asserts that HRNs are properly parsed and resolved, and sending to the corresponding offer works

ldk-reviews-bot · 2025-09-05T14:41:32Z

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

chuksys · 2025-09-05T15:04:02Z

This PR builds upon #607. It is currently opened in draft because the end-to-end test I added runs indefinitely without resolving or timing out.

tnull

First look at the additional changes here, had to cut the review round short though.

tnull · 2025-09-10T12:56:18Z

Cargo.toml

 vss-client = "0.3"
 prost = { version = "0.11.6", default-features = false}
 bitcoin-payment-instructions = { version = "0.5" }
+lightning-dns-resolver = "0.2.0"


Please add this above to the group(s) of lightning-* imports (also the commented-out ones) and follow the scheme there.

tnull · 2025-09-10T12:57:54Z

bindings/ldk_node.udl

 	u64 probing_liquidity_limit_multiplier;
 	AnchorChannelsConfig? anchor_channels_config;
 	SendingParameters? sending_parameters;
+	boolean is_hrn_resolver;


Hmm, I think over at the first PR we discussed to introduce a dedicated config for HRN-related things, also to specify default resolvers, for example? Probably this should be part of such a config?

Yes we did - I'll re-introduce the dedicated config for HRN-related things.

tnull · 2025-09-10T13:02:40Z

src/builder.rs

-	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)?,


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

Yes that makes sense - will do.

tnull · 2025-09-10T13:29:00Z

src/payment/unified.rs

+			Error::HrnResolverNotConfigured
 		})?;

+		println!("Parsing instructions...");


Oh, please remove any printlns.

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.

tnull · 2025-09-10T13:31:06Z

src/builder.rs

-		peer_manager_clone.process_events();
-	}));
+	let hrn_resolver = match resolver {
+		Resolver::DNS(_) => None,


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

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.

tnull

Was able to reproduce the behavior you were running into (see below).

tnull · 2025-09-17T12:12:33Z

tests/integration_tests_rust.rs

+	// Sleep one more sec to make sure the node announcement propagates.
+	std::thread::sleep(std::time::Duration::from_secs(1));
+
+	let hrn = "chuks@peepswire.com";


I was now able to reproduce the infinite loop you were seeing, and it seems that this HRN doesn't have a valid DNSSEC proof attached (i.e., we hit the else clause of this case in the DNS resolver: https://github.com/lightningdevkit/rust-lightning/blob/674861d06fb333222cff9c2caaeb3783728672a1/lightning-dns-resolver/src/lib.rs#L134). If I plug in matt@mattcorallo.com the resolution actually succeeds, however, it of course doesn't have any suitable regtest payment instructions attached. I'm not fully sure if there are any plans for local-testing end-to-end lightning-dns-resolver (cc @TheBlueMatt ?), but we may indeed need to spin up our own DNS server (instead of using 8.8.8.8) and publish a correct DNSSEC proof to make the test fully work end-to-end in regtest.

Also opened an issue here: rust-bitcoin/bitcoin-payment-instructions#11

We do have end-to-end tests in lightning-dns-resolver tho they are a bit awkward. The hook ChanelManager::testing_dnssec_proof_offer_resolution_override but then do a totally normal resolution (which must pass DNSSEC validation) and then swap out the returned offer, see https://github.com/lightningdevkit/rust-lightning/blob/main/lightning-dns-resolver/src/lib.rs#L387

Thanks @tnull and @TheBlueMatt - I'll look into ChannelManager::testing_dnssec_proof_offer_resolution_override and also explore spinning up our own DNS server to publish a correct DNSSEC proof.

For consistency, probably good to just use matt@mattcorallo.com as the test string, let it do DNS resolution, then have a similar override hook in ldk-node.

Okay, thanks.

tnull · 2025-09-17T12:13:21Z

src/payment/unified.rs

+		println!("Parsing instructions...");
+
+		let instructions =
+			PaymentInstructions::parse(uri_str, self.config.network, resolver.as_ref(), false)


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

chuksys added 12 commits September 4, 2025 16:48

Rename unified_qr to unified

9ea92d5

This rename reflects that this module is a unified payment interface for both QR code payments and HRN payments passed in as a string without scanning a QR code

rename UnifiedQRPayment to UnifiedPayment, rename QRPaymentResult to …

b11fead

…UnifiedPaymentResult These renamings are necessary to reflect the expanded responsibilities for this module.

add bitcoin-payment-instructions to cargo.toml

998bdab

Add hrn_resolver to Node and pass into UnifiedPayment

55522b9

This commit adds a HRN Resolver to the Node struct which will be useful for resolving HRNs when making BIP 353 payments. It also passes the HRN Resolver into UnifiedPayment.

Refactor unified.rs to support sending to BIP 21 URIs as well as BIP …

f1d1fa1

…353 HRNs

Fix typo in unified_payment send test and improve test name.

869839a

Switch to explicit imports in unified.rs mod tests

38e17d1

Add lightning-dns-resolver to Cargo.toml

da52af8

Enable setting Node as HRN Resolver for 3rd parties in Config

7973b8c

Pass HRNResolver or DomainResolver into OnionMessenger

6068c1c

This commit allows us to enable our node to either be able to send HRN resolution requests or acts as a resolver for 3rd party nodes We also pass HRNResolver as an optional parameter to Node

Testing Node Ann Broadcast when node is_hrn_resolver

2149475

This commit will be dropped later

Add end-to-end test for HRN resolution

0517dbc

This commit adds an end-to-end test that asserts that HRNs are properly parsed and resolved, and sending to the corresponding offer works

tnull reviewed Sep 10, 2025

View reviewed changes

tnull self-requested a review September 17, 2025 11:11

tnull reviewed Sep 17, 2025

View reviewed changes

tnull mentioned this pull request Sep 17, 2025

PaymentInstructions::parse never returns if OM-based HRN lookup fails rust-bitcoin/bitcoin-payment-instructions#11

Closed

Add support for resolving BIP 353 Human-Readable Names #630

Are you sure you want to change the base?

Add support for resolving BIP 353 Human-Readable Names #630

Uh oh!

Conversation

chuksys commented Sep 5, 2025

Changes

Uh oh!

ldk-reviews-bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chuksys commented Sep 5, 2025

Uh oh!

tnull left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldk-reviews-bot commented Sep 5, 2025 •

edited

Loading

tnull left a comment •

edited

Loading