BIP374: add test vectors for secp256k1 generator point #1751

1. 
   stratospher commented at 6:23 am on January 24, 2025: contributor

   Add successful test vectors for:

   • secp256k1’s generator point
   • optional message
2. stratospher force-pushed on Jan 24, 2025
3. BIP374: add test vectors for secp256k1 generator point

   - added 2 more successful test vectors.
     now there are 7 test vectors[test vectors 0..6].
   - test vectors 5, 6 have G=GENERATOR

   41e0f34f76
4. BIP374: add test vector for optional message

   - added 1 more successful test vectors.
     now there are 8 test vectors[test vectors 0..7].
   - test vector 5 has optional message
   - test vectors 5, 6, 7 have G=GENERATOR

   5f42eb64d4
5. stratospher force-pushed on Jan 28, 2025
6. jonatack added the label Proposed BIP modification on Jan 29, 2025
7. jonatack added the label Pending acceptance on Jan 29, 2025
8. in bip-0374/gen_test_vectors.py:59 in 5f42eb64d4

   55@@ -57,6 +56,7 @@ def gen_all_generate_proof_vectors(f):
   56     for i in range(NUM_SUCCESS_TEST_VECTORS):
   57         G, a, A, b, B, C, auxrand, msg, proof = TEST_VECTOR_DATA[i]
   58         assert proof is not None and len(proof) == 64
   59+        if msg is None: msg = b""
   

   ---

   ---

   

   jonatack commented at 8:08 pm on January 29, 2025:

   formatting nit if you repush, for each of these:

   0        if msg is None:
   1            msg = b""
   

9. 
   jonatack commented at 8:09 pm on January 29, 2025: member

   First glance LGTM, generated and ran the test vectors
10. 
    jonatack commented at 8:09 pm on January 29, 2025: member

    Pinging authors @andrewtoth @theStack @RubenSomsen for feedback
11. in bip-0374/gen_test_vectors.py:2 in 41e0f34f76 outdated

    0@@ -1,5 +1,5 @@
    1 #!/usr/bin/env python3
    2-"""Generate the BIP-DLEQ test vectors (limited to secp256k1 generator right now)."""
    3+"""Generate the BIP-0374 test vectors."""
    

    ---

    ---

    

    theStack commented at 4:32 pm on January 31, 2025:

    nit: if you have to retouch, could also substitute s/BIP-DLEQ/BIP-0374/ in run_test_vectors.py
12. theStack approved
13. 
    theStack commented at 4:34 pm on January 31, 2025: contributor

    lgtm ACK 5f42eb64d4df8920220332a62128afd2f6403be1

    Something to solve for a different PR, but it seems there is currently a general mismatch in the BIP that wasn’t noticed so far: the algorithms take G as input parameter with the intention to “let this algorithm be used for other curves”, but on the other hand the reference implementation and the test vectors are currently still only available for the secp256k1 curve. To my understanding, supporting different curves could mean completely different curve parameters (e.g. different constants a, b, p, n etc., and even different serialization size of scalars and points that are not compatible with the BIP), and not only choosing a different generator point on a curve with the same equation. Is it even possible to write the BIP in such a generic way? Other BIPs often include the warning “We note that adapting this proposal to other elliptic curves is not straightforward and can result in an insecure scheme.” (e.g. in BIP327, which is referred to in this BIP), likely for a good reason. If the intention was to indeed only support secp256k1 with different G values (if that makes sense and has use cases), the phrases should adapted.

14. 
    andrewtoth commented at 3:58 pm on February 1, 2025: contributor

    supporting different curves could mean completely different curve parameters (e.g. different constants a, b, p, n etc., and even different serialization size of scalars and points that are not compatible with the BIP)

    Indeed, from what I see the constant n is used in the BIP directly, p is used indirectly for cbytes(P)->xbytes(P)->x(P), and the serialization sizes could be larger than 32 bytes. I think these are the only items in the BIP that would have to be changed for other curves. It seems like adapting this would be straightforward though, unlike BIP327. We could simply say p is the field size and n is the order, and xbytes should use a length greater than 32 if required by the curve.

15. andrewtoth approved
16. 
    andrewtoth commented at 4:13 pm on February 1, 2025: contributor

    ACK 5f42eb64d4df8920220332a62128afd2f6403be1

    All tests passed on my machine. Good to have these test vectors, thanks!

17. jonatack removed the label Pending acceptance on Feb 1, 2025
18. jonatack merged this on Feb 1, 2025
19. jonatack closed this on Feb 1, 2025

    ---

20. 
    jonatack commented at 4:35 pm on February 1, 2025: member

    Feel free to open a follow-up for the feedback.

Contributors
stratospher jonatack theStack andrewtoth

Labels
Proposed BIP modification