7d8d5c86df tests: refactor: take use of `secp256k1_ge_x_on_curve_var` (Sebastian Falbesoner)
Pull request description:
The recently merged ellswift PR (#1129) introduced a helper `secp256k1_ge_x_on_curve_var` to check if a given X coordinate is on the curve (i.e. the expression x^3 + 7 is square, see commit 79e5b2a8b8). This can be used for code deduplication in the `ecmult_const_mult_xonly` test.
(Found this instance via `$ git grep add_int.*SECP256K1_B`, I think it's the only one where the helper can be used.)
ACKs for top commit:
sipa:
utACK 7d8d5c86df
real-or-random:
utACK 7d8d5c86df
Tree-SHA512: aebff9b5ef2f6f6664ce89e4e1272cb55b6aac81cfb379652c4b7ab30dd1d7fd82a2c3b47c7b7429755ba28f011a3a9e2e6d3aa5c77d3b105d159104c24b89f3
The recently merged ellswift PR (#1129) introduced a helper
`secp256k1_ge_x_on_curve_var` to check if a given X coordinate is
valid (i.e. the expression x^3 + 7 is square, see commit
79e5b2a8b8). This can be used for code
deduplication in the `ecmult_const_mult_xonly` test.
5b7bf2e9d4 Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin-core/secp256k1/issues/1324.
As the `__shiftright128` [docs](https://learn.microsoft.com/en-us/cpp/intrinsics/shiftright128) state:
> The `Shift` value is always modulo 64...
it is not applicable for the `n >= 64` branch.
ACKs for top commit:
sipa:
utACK 5b7bf2e9d4
real-or-random:
ACK 5b7bf2e9d4 tested with MSVC x64
Tree-SHA512: bc4c245a9da83c783a0479e751a4bc2ec77a34b99189fcc4431033a5420c93b610f3b960d3f23c15bce2eb010beba665b3e84d468b3fdab3d5846d4f27016898
e449af6872 Drop no longer needed `#include "../include/secp256k1.h"` (Hennadii Stepanov)
Pull request description:
The removed header includes have not been needed since https://github.com/bitcoin-core/secp256k1/pull/1231.
Test suggestions:
1. Using Autottols-based build system:
```
./autogen.sh
./configure
make clean-precomp
make
```
2. Using CMake-based build system:
```
cmake -B build -DCMAKE_C_INCLUDE_WHAT_YOU_USE="include-what-you-use"
cmake --build build --target secp256k1_precomputed
```
ACKs for top commit:
sipa:
utACK e449af6872
real-or-random:
utACK e449af6872
Tree-SHA512: 5aed7a88e1e03fcc2306c43817712c0652ecf6145679dd17f4719376818d372f619e4180bdaee548f2e82aaccbe6a2ff4c37203121d939af545128c8c48b933e
The scheme implemented is described below, and largely follows the paper
"SwiftEC: Shallue–van de Woestijne Indifferentiable Function To Elliptic Curves",
by Chavez-Saab, Rodriguez-Henriquez, and Tibouchi
(https://eprint.iacr.org/2022/759).
A new 64-byte public key format is introduced, with the property that *every*
64-byte array is an encoding for a non-infinite curve point. Each curve point
has roughly 2^256 distinct encodings. This permits disguising public keys as
uniformly random bytes.
The new API functions:
* secp256k1_ellswift_encode: convert a normal public key to an ellswift 64-byte
public key, using additional entropy to pick among the many possible
encodings.
* secp256k1_ellswift_decode: convert an ellswift 64-byte public key to a normal
public key.
* secp256k1_ellswift_create: a faster and safer equivalent to calling
secp256k1_ec_pubkey_create + secp256k1_ellswift_encode.
* secp256k1_ellswift_xdh: x-only ECDH directly on ellswift 64-byte public keys,
where the key encodings are fed to the hash function.
The scheme itself is documented in secp256k1_ellswift.h.
f1652528be Normalize ge produced from secp256k1_pubkey_load (stratospher)
Pull request description:
The output `ge` in secp256k1_pubkey_load is normalized when `sizeof(secp256k1_ge_storage) = 64` but not when it's not 64. ARG_CHECK at the end of the function assumes normalization. So normalize ge in the other code path too.
context: [#1129(comment)](https://github.com/bitcoin-core/secp256k1/pull/1129/files#r1196167066)
ACKs for top commit:
sipa:
utACK f1652528be
real-or-random:
ACK f1652528be tested by changing the two `== 64` checks to `== 65`
Tree-SHA512: 0de1caad85ccdb42053f8e09576135257c88fda88455ef25e7640049c05a1e03d1e9bae1cd132d2e6fc327fd79929257a8b21fe1cc41c82374b6cd88e6744aa3
The output ge is normalized when sizeof(secp256k1_ge_storage) = 64
but not when it's not 64. ARG_CHECK at the end of the function
assumes normalization. So normalize ge in the other code path too.
ade5b36701 tests: add checks for scalar constants `secp256k1_scalar_{zero,one}` (Sebastian Falbesoner)
654246c635 refactor: take use of `secp256k1_scalar_{zero,one}` constants (Sebastian Falbesoner)
Pull request description:
Rather than allocating a (non-constant) scalar variable on the stack with the sole purpose of setting it to a constant value, the global constants `secp256k1_scalar_{zero,one}` (apparently introduced in 34a67c773b, PR #710) can be directly used instead for the values 0 or 1. There is very likely not even a difference in run-time, but it leads to simpler and less code which might be nice.
ACKs for top commit:
sipa:
utACK ade5b36701
real-or-random:
utACK ade5b36701
Tree-SHA512: 0ff05a449c153f7117a4a56efef04b2087c2330f4692f3390a0b1d95573785ac7ae3fe689ed0ec2ecc64b575d2489d6e341d32567e75a1a4b4d458c3ecd406a1
8c9ae37a5a Add release note (Pieter Wuille)
350b4bd6e6 Mark stack variables as early clobber for technical correctness (Pieter Wuille)
0c729ba70d Bugfix: mark outputs as early clobber in scalar x86_64 asm (Pieter Wuille)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 8c9ae37a5a
jonasnick:
ACK 8c9ae37a5a
Tree-SHA512: 874d01f5540d14b5188aec25f6441dbc6631f8d3980416040a3e250f1aef75150068415e7a458a9a3fb0d7cbdeb97f5c7e089b187d6d3dd79aa6e45274c241b6
c6bb29b303 build: Rename `64bit` to `x86_64` (Hennadii Stepanov)
03246457a8 autotools: Add `SECP_ARM32_ASM_CHECK` macro (Hennadii Stepanov)
ed4ba238e2 cmake: Add `check_arm32_assembly` function (Hennadii Stepanov)
e5cf4bf3ff build: Rename `arm` to `arm32` (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin-core/secp256k1/issues/1034.
Solves one item in https://github.com/bitcoin-core/secp256k1/issues/1235.
ACKs for top commit:
real-or-random:
ACK c6bb29b303 tested on x86_64 but not on ARM
Tree-SHA512: c3615a18cfa30bb2cc53be18c09ccab08fc800b84444d8c6b333347b4db039a3981da61e7da5086dd9f4472838d7c031d554be9ddc7c435ba906852bba593982
In the field 5x52 asm for x86_64, stack variables are provided as outputs.
The existing inputs are all forcibly allocated to registers, so cannot
coincide, but mark them as early clobber anyway to make this clearer.
In the existing code, the compiler is allowed to allocate the RSI register
for outputs m0, m1, or m2, which are written to before the input in RSI is
read from. Fix this by marking them as early clobber.
Reported by ehoffman2 in https://github.com/bitcoin-core/secp256k1/issues/766
cd54ac7c1c schnorrsig: Improve docs of schnorrsig_sign_custom (Tim Ruffing)
28687b0312 schnorrsig: Add BIP340 varlen test vectors (Tim Ruffing)
97a98bed1e schnorrsig: Refactor test vector code to allow varlen messages (Tim Ruffing)
Pull request description:
ACKs for top commit:
sipa:
ACK cd54ac7c1c. I didn't verify the included test vectors match the BIP.
jonasnick:
ACK cd54ac7c1c
Tree-SHA512: 268140e239b703aaf79825de2263675a8c31bef999f013ea532b0cd7b80f2d600d78f3872209a93774ba4dbc0a046108e87d151fc4604882c5636876026a0816
17fa21733a ct: Be cautious and use volatile trick in more "conditional" paths (Tim Ruffing)
5fb336f9ce ct: Use volatile trick in scalar_cond_negate (Tim Ruffing)
Pull request description:
ACKs for top commit:
sipa:
ACK 17fa21733a
jonasnick:
ACK 17fa21733a
Tree-SHA512: 4a0fbee7b1cce4f4647bff697c0e645d93aa8fb49777feef5eb1e1eadce2116bafdcc6175c066ee4fe4bf1340047311e2d7d2c48bb288867a837ecd6c8687121
712e7f8722 Remove unused scratch space from API (Jonas Nick)
Pull request description:
Not sure if we want the typedef and `secp256k1_scratch_space_{create,destroy}` but if we don't keep them then this PR will be a rather large diff.
ACKs for top commit:
sipa:
ACK 712e7f8722
real-or-random:
utACK 712e7f8722
Tree-SHA512: b3a8feb0fe4639d5e48b708ccbf355bca5da658a291f63899086d2bbeb6d0ab33e3dcd55d8984ec7fa803f757b7d02e71bcb7e7eeecaab52ffc70ae85dce8c44
- secp256k1_scalar_cadd_bit
- secp256k1_modinvXX_normalize_YY
- secp256k1_modinvXX_divsteps_ZZ
- ECMULT_CONST_TABLE_GET_GE
Even though those code loations are not problematic right now
(with current compilers).
97c63b9039 Avoid normalize conditional on VERIFY (Pieter Wuille)
Pull request description:
In the old code, `secp256k1_gej_rescale` requires a normalized input in VERIFY mode, but not otherwise. Its requirements shouldn't depend on this mode being enabled or not.
ACKs for top commit:
real-or-random:
utACK 97c63b9039 I've also verified that the loop in secp256k1_ecmult_strauss_wnaf holds up the invariant that the magnitude of Z is 1, even with the normalization removed
jonasnick:
ACK 97c63b9039
Tree-SHA512: 9598c133c6f4e488c74512089dabe0508529f20ca782be1c8fbeae9d7f132da9d570a061053acd3d245a9a187abf1f2581207441ce6aac8d0f8972cf357a349f
