BIP-374: treat challenge hash result e as scalar

[theStack]

The purpose of this change is primarily to improve clarity for implementers (e.g. in libsecp [1]) and for consistency with existing BIPs like e.g. BIP-340 and BIP-327, where the challenge hash result is reduced modulo the curve order as well:

bips/bip-0340.mediawiki

Line 156 in 857a7de

* Let ''e = int(hash<sub>BIP0340/challenge</sub>(bytes(R) || bytes(P) || m)) mod n''.

bips/bip-0327.mediawiki

Line 434 in 857a7de

* Let ''e = int(hash<sub>BIP0340/challenge</sub>(xbytes(R) || xbytes(Q) || m)) mod n''

It seems not strictly needed from a pure specification point of view, but I think it makes sense to treat e as scalar throughout, given that we perform scalar operations at the proof generation side for calculating $s = (k + e \cdot a) \bmod n$, so implementers would need to do a conversion (and hence decide whether to wrap-around or not) anyways.

Under the assumption that reaching a challenge hash with e >= n is negligible, I think this change can be treated as logically equivalent and thus backwards compatible (but maybe there is more subtlety to it that I don't see); note that the newly introduced test vectors would also fail without the new early rejection branch, but at a later point due to not fulfilling the verification equation.

Thanks to @macgyver13 for making me aware that e is currently not specified to be in the scalar range.

[1] see PRs bitcoin-core/secp256k1#1651 and bitcoin-core/secp256k1#1802

[theStack]

pinging the BIP-374 co-authors and some "friendly cryptographers from next-door" :-) @andrewtoth @jonasnick @real-or-random @RubenSomsen @sipa

[theStack]

leaving a note for reviewers that we could also take better use of secp256k1lab's types in the reference implementation (e.g. directly return a Scalar here instead of working with ints), but that's out of scope for this PR

[murchandamus]

Hey, @theStack, what’s the next step here? Is this ready for merge, would you like to wait longer for the other authors to chime in, or is there something else that we should wait for?

[macgyver13]

ACK fc874dd

Ran both gen_test_vectors.py and run_test_vectors.py locally. Generation produces expected csv output with new proof vectors 15 and 16, matches the committed version. The runner correctly rejected invalid proofs. BIP text changes look good to me.

python run_test_vectors.py
...
Test vector #15: (Proof with e equal to curve order)
 * Passed proof verification test.
Test vector #16: (Proof with s equal to curve order)
 * Passed proof verification test.
   
All test vectors passed.