Skip to content

bip374: add new test vectors checking for boundaries - #2335

Open
ViniciusCestarii wants to merge 1 commit into
bitcoin:masterfrom
ViniciusCestarii:bip-374-new-test-vectors
Open

ViniciusCestarii wants to merge 1 commit into
bitcoin:masterfrom
ViniciusCestarii:bip-374-new-test-vectors

Conversation

@ViniciusCestarii

Copy link
Copy Markdown
Contributor

Add 3 new generate proof test vectors for testing boundaries:

Success: a = 1, a = N-1
Failure: a = N+1 (catches implementation that does mod n on secret instead of first rejecting)

@jonatack jonatack added Proposed BIP modification PR by non-owner to update BIP content Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels Oct 20, 2026
@jonatack

Copy link
Copy Markdown
Member

cc BIP authors @andrewtoth, @RubenSomsen, and @theStack

@fametrano fametrano left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK 60b4c75, tested: the new vectors agree with an independent implementation.

The PR adds success cases for a=1 and a=N-1 and a failure case for a=N+1 to the proof-generation vectors. With the existing a=0 and a=N failures, both ends of the valid range are now pinned from both sides.

I ran all 14 rows of the CSV through btclib-ecc's BIP374 code, with and without libsecp256k1. Every success proof matched byte for byte and verified. The four failure rows (a=0, a=N, a=N+1, B at infinity) were refused. Re-running gen_test_vectors.py reproduced both CSV files exactly. The verification file is unchanged and already covers e=N and s=N, so I see no boundary missing from this change. Script and logs on request.

@theStack

Copy link
Copy Markdown
Contributor

LGTM, can you add a small changelog entry with bumped PATCH version?

@ViniciusCestarii
ViniciusCestarii force-pushed the bip-374-new-test-vectors branch from 60b4c75 to afaa856 Compare October 26, 2026 10:17
@ViniciusCestarii

Copy link
Copy Markdown
Contributor Author

Forced-push afaa856 addressing #2335 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified Proposed BIP modification PR by non-owner to update BIP content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants