Skip to content

Add some BIP 353 DNSSEC proof test vectors and links#1912

Merged
jonatack merged 2 commits intobitcoin:masterfrom
TheBlueMatt:2025-08-dnssec-proof-tests
Sep 24, 2025
Merged

Add some BIP 353 DNSSEC proof test vectors and links#1912
jonatack merged 2 commits intobitcoin:masterfrom
TheBlueMatt:2025-08-dnssec-proof-tests

Conversation

@TheBlueMatt
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Aug 8, 2025

@TheBlueMatt

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?

@TheBlueMatt
Copy link
Contributor Author

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.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to update this 👍

alvroble added a commit to alvroble/embit that referenced this pull request Aug 8, 2025
*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
@achow101
Copy link
Member

achow101 commented Aug 8, 2025

Would it be possible to host these proofs on your website instead, on a dedicated page, and then link to them in the BIP?

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.

@ghost
Copy link

ghost commented Aug 8, 2025

Test vectors can be included in separate files contained in the repo, many BIPs do that as well.

That would be significantly better.

Copy link
Member

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@murchandamus
Copy link
Member

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?

@jonatack jonatack added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Aug 18, 2025
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, verified correctness of the first example proof (data payload bitcoin is cool!)

The editorial nits can be included separately in our monthly fixup PR.

The status update was handled with #1979.

@jonatack jonatack removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Sep 23, 2025
murchandamus added a commit to murchandamus/bips that referenced this pull request Sep 23, 2025
- 327 should be Final instead of Active
- 353 should be Proposed, as Testvectors are still in the works (see
  bitcoin#1912)
murchandamus added a commit to murchandamus/bips that referenced this pull request Sep 23, 2025
- 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>`.
@TheBlueMatt TheBlueMatt force-pushed the 2025-08-dnssec-proof-tests branch from e45f544 to c02809a Compare September 23, 2025 22:01
@TheBlueMatt
Copy link
Contributor Author

Addressed pending feedback. I didn't bother moving the tests to a new file, they're all at the end so it seemed fine.

@jonatack
Copy link
Member

ACK c02809a

Independently verified the test vectors.

@jonatack jonatack merged commit c5cb824 into bitcoin:master Sep 24, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants