Configurable HumanReadableName resolver#195
Configurable HumanReadableName resolver#195tnull wants to merge 2 commits intolightningdevkit:mainfrom
HumanReadableName resolver#195Conversation
Picks up support for resolving BIP 353 Human-Readable Names (ldk-node PR #630), which adds `Config::hrn_config` and the `HumanReadableNamesConfig` / `HRNResolverConfig` types. Co-Authored-By: HAL 9000
Exposes LDK Node's new `HumanReadableNamesConfig` via a new `[hrn]` section in `config.toml`. Users can pick between resolving BIP 353 names locally over DNS (`mode = "dns"`, the default) or via bLIP-32 (`mode = "blip32"`), and, when using DNS, optionally run as an HRN resolution service for the rest of the network. Defaults match LDK Node's: resolve via DNS against `8.8.8.8:53` with the resolution service disabled (enabling it requires the node to be announceable so others can route resolution requests to us). Co-Authored-By: HAL 9000
|
I've assigned @benthecarman as a reviewer! |
benthecarman
left a comment
benthecarman
left a comment
There was a problem hiding this comment.
would be nice to one day add http support as well but mostly looks good!
| let mut config = HumanReadableNamesConfig::default(); | ||
|
|
||
| match value.mode.as_deref() { | ||
| None | Some("dns") => {}, |
There was a problem hiding this comment.
here we assume that the default is HRNResolverConfig::Dns and then modify the settings below. Would be safer to have the logic of the if let HRNResolverConfig::Dns ... in this arm so if the default ever changed (say to http to support lnurl) then we could better handle it and not silently drop the dns_server_address and enable_resolution_service settings.
| match value.mode.as_deref() { | ||
| None | Some("dns") => {}, | ||
| Some("blip32") => { | ||
| config.resolution_config = HRNResolverConfig::Blip32; |
There was a problem hiding this comment.
Would be good to throw an error if enable_resolution_service or dns_server_address is set. Otherwise users could be confused that they have something even though we are silently dropping the config option
| &mut config.resolution_config | ||
| { | ||
| if let Some(addr) = value.dns_server_address.as_deref() { | ||
| *dns_server_address = SocketAddress::from_str(addr).map_err(|e| { |
There was a problem hiding this comment.
Can we do something here to assume port 53 if its not provided?
3 participants
Bump LDK Node and add configurable HRN resolution.