Add some BIP 353 DNSSEC proof test vectors and links#1912
Add some BIP 353 DNSSEC proof test vectors and links#1912jonatack merged 2 commits intobitcoin:masterfrom
Conversation
1c9f947 to
665eaf1
Compare
665eaf1 to
e45f544
Compare
|
This PR simply fills the BIP with proofs that take up nearly as much space as the actual BIP text. Would it be possible to host these proofs on your website instead, on a dedicated page, and then link to them in the BIP? |
|
It seems a bit weird to me to elide test cases just because they're marginally big. But I'll let the BIP editors decide here. |
jonatack
left a comment
There was a problem hiding this comment.
Looks mostly good, didn't test, a few minor edits.
Some BIPs put the test vectors in a separate file. Either way seems fine to me.
| * A DNSSEC proof generation and validation implementation can be found at https://git.bitcoin.ninja/index.cgi?p=dnssec-prover;a=summary | ||
| * A lightning-specific name to payment instruction resolver can be found at https://git.bitcoin.ninja/index.cgi?p=lightning-resolver;a=summary | ||
| * Reference implementations for parsing the URI contents can be found in [[bip-0021.mediawiki|BIP 21]]. | ||
| * Reference implementations for parsing the URI contents can be found in [[bip-0321.mediawiki|BIP 321]]. |
*Allow multiple TXT records in DNSSEC proof per BIP353 specification *Only one TXT record can start with "bitcoin:" (case insensitive) *Add validation from bitcoin/bips#1912
I don't think that is an appropriate way for BIPs to refer to test vectors. All of the details necessary to implement a BIP should be attached to the BIP in this repo and should not require implementers to go to some third party resource that may not exist in the future. Test vectors can be included in separate files contained in the repo, many BIPs do that as well. |
That would be significantly better. |
murchandamus
left a comment
There was a problem hiding this comment.
The amount of test vectors here does not seem excessive yet, so I’m also fine with it being in this file, but if there will be more data in the future, I would recommend starting auxiliary file as proposed by others here.
|
It seems to me that BIP 353 should perhaps also be advanced to Proposed or Active after I have been reading about multiple implementations of BIP 353. Would you like to incorporate that into this same PR? |
- 327 should be Final instead of Active - 353 should be Proposed, as Testvectors are still in the works (see bitcoin#1912)
- 327 should be Final instead of Active - 353 should be Proposed, as Testvectors are still in the works (see bitcoin#1912)
mediawiki doesn't render backticks, so we have to use `<code></code>`.
e45f544 to
c02809a
Compare
|
Addressed pending feedback. I didn't bother moving the tests to a new file, they're all at the end so it seemed fine. |
|
ACK c02809a Independently verified the test vectors. |
No description provided.