secp256k1proto: deserialize x-only-pk with from_bytes_xonly (schnorr_verify)

[theStack]

If the passed x-only-pubkey overflows (x >= p), this method fails immediately rather than silently wrapping around and then likely failing later (either because x % p is not on the curve or because the actual signature verification fails [1]). For the user of schnorr_verify this shouldn't make any difference, as False is returned with both variants, so this is probably more of a cosmetic change.

[1] b'\xff'*32 is an example x-only-pubkey for the latter case

[real-or-random]

ACK

If the passed x-only-pubkey overflows (x >= p), this method fails immediately rather than silently wrapping around and then likely failing later (either because x % p is not on the curve or because the actual signature verification fails [1]). For the user of schnorr_verify this shouldn't make any difference, as False is returned with both variants, so this is probably more of a cosmetic change.

Hm yes, but any implementation of verification should be 100% compatible with the BIP because it's consensus critical. And I think it's easy to come up with a signature (by changing the signing algorithm) that would pass verification here, but not according to the BIP.

So this is a real bug. I think (or hope) that this should be covered by one of the BIP340 test vectors.

The underlying problem here is that lift_x in the BIP fails on overflow, but here it does not. The idea is that you can use ints wherever field elements are expected, and we'll call the FE constructor, which simply takes the mod.

I tend to think that this constructor should require a normalized representation, i.e., be checked. There could still be a FE.from_int_wrapping factory method. But explicit is better than implicit.

[real-or-random]

note: We may want to squash this into 843d165 when moving secp256k1proto into a separate repo (because the latter will create a new history anyway).