Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add a BIP which resolves human readable names into payment info #1551
base: master
Are you sure you want to change the base?
Add a BIP which resolves human readable names into payment info #1551
Changes from 1 commit
09db33a
d618202
f7b5932
8839a6c
f67f39e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It would be very helpful if this section was extended with a number of concrete use cases and also mention which use cases are not covered or not a good fit.
At first this seems like an interesting solution to display domain names instead of addresses on hardware wallets for improved security, but for exchanges/brokers/merchants, they'd need a unique address per user and per deposit. For this use-case this does not seem to be a good fit. Downsides / blockers could be:
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.
This would violate
Bitcoin wallets MUST NOT prefer to use DNS-based resolving when methods with explicit public keys are available.
That's okay. Address reuse prevention is probabilistic anyway.
Hmm? If you're a merchant or exchange, you're probably already using a domain anyway :).
Yes, please see lightning/blips#32 and its discussion of wildcard records :)
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.
.well-known
approach can prevent address reuse completely, if HTTP server generates new address for every request.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.
Out of curiosity, why the leading underscore (
_bitcoin-payment
), is that a best practice?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.
Yea, just makes it less likely to collide with any existing actual domain names that exist in the wild.
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.
I don't quite understand the scheme here. What is the purpose of the "user" label? (Future protocol expansions where something besides "users" can be identified?). Also why not order the labels with the underscores more like existing schemes for example, TLSA:
_443._tcp.mailhardener.com TLSA ...
So I'd expect something like:
_bitcoin-payment.matt.mattcorallo.com TXT ...
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.
(see also https://github.com/james-stevens/wallet-ids-in-dns)
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.
@pinheadmz
I'm not DNS expert, but the Wikipedia article about wildcard DNS entries says, "A wildcard DNS record is specified by using a
*
as the leftmost label (part) of a domain name, e.g.*.example.com
." I think @TheBlueMatt wants to allow wildcards for the username portion for LN (see the proposed BIP21 parameteromlookup
(onion message lookup) in this commit). If only the leftmost label can be a wildcard, then_bitcoin-payment.*.mattcorallo.com
wouldn't be allowed.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.
Is this really worth supporting (instead of restricting to ASCII characters)? This forces all clients to potentially support those encodings, which is a likely source of issues...
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.
Maybe not, I mean punycode itself is pretty easy, but dealing with homograph attacks is...quite nontrivial. So, indeed, I'm down to just remove all the non-ASCII support.
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.
When do you encounter multiple matching TXT records?
Is this due to querying twice and getting different results due to TTL?
Or a malformed TXT record: "bitcoin:........;bitcoin:......"
Or just two independent TXT records with the same key and different values?
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.
Two TXT records.
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.
If I had to guess at the rationale, this is meant to protect against the particular kind of configuration error where you intend to update the record, but mistakenly end up adding another instead. This protects against bitcoins getting sent to lost or compromised addresses.
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.
Honestly I didn't have a specific issue in mind more a general feeling that issues could crop up, this is a good example of one such issue.
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.
Recently, a vulnerability related to DNSSEC design, KeyTrap (CVE-2023-50387), was disclosed.
https://www.athene-center.de/en/keytrap
Is this requirement not affected by this vulnerability? Isn't it necessary to consider mitigation measures that existing resolvers take?
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.
KeyTrap observed that validation of DNSSEC signatures can be superlinear in the number of signatures/keys. The mitigations that ~all validators have deployed addresses that, and should be used here. However, "be up to date with the latest security fixes" is a general thing and doesn't really need to be specified here :)
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.
By only rotating the onchain address after receiving a payment, anyone with the human readable name can continually poll your address(es) and monitor the onchain transactions you receive.
It requires a more active attack on privacy, but still very easy.
I still think it is worthwhile to include onchain, it's nice to support any BIP21 type. But could discourage onchain in favour of silent payments / BOLT12.
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.
That's definitely the goal.
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.
I don't understand what you mean here, can you detail?
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.
Drawback (b) is "revealing sender IP addresses to recipients or other intermediaries as a side-effect of payment". I added a few additional words here, let me know if that clarified it.
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.
This is much clearer, thanks!
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.
File not found
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.
I guess that's because Matt used the link that will work once that bLIP is merged, since we expect bLIPs to be merged more quickly than BIPs :)
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.
Not merged yet https://github.com/lightning/blips/pull/32/files
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.
As mentioned above the page is 404, but also in mediawiki format, links are encoded
[[url|name]]
instead of[[name|url]]
.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.
I'll make sure it gets merged before this does :)