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
7067ee54b4 tests: add tests for `secp256k1_{read,write}_be64` (Sebastian Falbesoner)
740528caad scalar: use newly introduced `secp256k1_{read,write}_be64` helpers (4x64 impl.) (Sebastian Falbesoner)
Pull request description:
This is a simple follow-up to #1339, as suggested in comment https://github.com/bitcoin-core/secp256k1/pull/1339#issuecomment-1587508040.
ACKs for top commit:
stratospher:
ACK 7067ee5.
real-or-random:
utACK 7067ee54b4
Tree-SHA512: f9bc2ab610099948ffac1e6bb3c822bd90b81a7110ab74cec03175e2c92ed27694a15f9cdaa7c4f1b460fe459f61c3d1d102c99592169f127fdd7539a1a0c154
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.
887183e7de scalar: use `secp256k1_{read,write}_be32` helpers (4x64 impl.) (Sebastian Falbesoner)
52b84238de scalar: use `secp256k1_{read,write}_be32` helpers (8x32 impl.) (Sebastian Falbesoner)
Pull request description:
This refactoring PR takes use of the `secp256k1_{read,write}_be32` helpers (introduced in PR #1093, commit 8d89b9e6e5) in the scalar <-> byte array conversion functions, for both the 8x32 and 4x64 implementations. (An alternative for the latter would be to introduce special helpers for reading/writing uint64_t in big endian `secp256k1_{read,write}_be64`).
Verified via `objdump -D libsecp256k1.a` that `secp256k1_scalar_set_b32` for 4x64 compiles to the same code on master and the PR (`secp256k1_scalar_get_b32` is apparently always inlined) on amd64 with clang 13.0.0.
ACKs for top commit:
sipa:
utACK 887183e7de
Tree-SHA512: 915cb4624c6da0530dce4ec3ac48e88dd735386302cd2e15759e3c30102d81186f382ffe71493ddd0538069f1b558db543d9bb900dfdb69acb60effedc33f705
f3644287b1 docs: correct `pubkey` param descriptions for `secp256k1_keypair_{xonly_,}pub` (Sebastian Falbesoner)
Pull request description:
ACKs for top commit:
real-or-random:
ACK f3644287b1 because it's consistent with the other docs
jonasnick:
ACK f3644287b1
Tree-SHA512: cc4db4637301335ea9d23ac43bb3a78de54af79a5262dba2013945f87d80670c7ae1e106101a59c04225eb077e9a9e0ecc9d9d3bfe2d11cdc90f098ebd479f49
From an API perspective, the functions `secp256k1_keypair_pub` and
`secp256k1_keypair_xonly_pub` always succeed (i.e. return the value 1),
so the other cases in the `pubkey` parameter descriptions never happen
and can hence be removed.
Note that the "1 always" return value description was previously done in
commit b8f8b99f0f (PR #1089), which also
explains why invalid inputs for the affected functions are in practice
only possible in violation of the type system.
db29bf220c ci: Remove quirk that runs dummy command after wineserver (Tim Ruffing)
c7db4942b3 ci: Fix error D8037 in `cl.exe` (Hennadii Stepanov)
7dae115861 Revert "ci: Move wine prefix to /tmp to avoid error D8037 in cl.exe" (Hennadii Stepanov)
Pull request description:
Since the 2146cbfaf0, the `msvc-wine` effectively initializes the WINE prefix when running the `install.sh` script. See [`install.sh`#L143](2146cbfaf0/install.sh (L143)):
```sh
WINEDEBUG=-all wine64 wineboot &>/dev/null
```
Our following `wine64 wineboot --init` just messes up with the prefix.
This PR fixes this issue.
Also https://github.com/bitcoin-core/secp256k1/pull/1327 has been reverted as apparently it does not work. And https://github.com/bitcoin-core/secp256k1/pull/1320 has been combined into this one.
ACKs for top commit:
real-or-random:
ACK db29bf220c
Tree-SHA512: 59e61bde0060f67501f93da8b4e193f2bfcda85d849c16bb017e38af7aa9e3b569fe2fd4aa5cdb658c3b2345cc42fad98323e329b519389b2e881ecfd403d147
605e07e365 fix input range comment for `secp256k1_fe_add_int` (Sebastian Falbesoner)
Pull request description:
This seems to be a typo that was introduced with commit 4371f98346 (PR #1066).
ACKs for top commit:
sipa:
ACK 605e07e365
real-or-random:
ACK 605e07e365
Tree-SHA512: 7ee99cf7140c698d1146072734ba986de7328f78b2c076ee445067ef64a6a335c8669f1e733e10f5e14f98b566c799cc4c51b3eb0f036cd178b3c93476c6df2e
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
27504d5c94 ci: Move wine prefix to /tmp to avoid error D8037 in cl.exe (Tim Ruffing)
Pull request description:
Don't ask me why this makes a difference. It may be some permission problem even though everything in Cirrus CI runs as root anyway. In any case, I'll probably get mad if I investigate this further.
Fixes#1326.
ACKs for top commit:
hebasto:
ACK 27504d5c94, tested in my personal Cirrus account.
Tree-SHA512: 08bb1734827579b59c705a44ee8fad6d504031eb5659c2743649be95fb048794b95ac0869a994bfa732f7f0714b4d12674c325637fe079b2266f18a3c14bbec0
Don't ask me why this makes a difference. It may be some permission
problem even though everything in Cirrus CI runs as root anyway. In
any case, I'll probably get mad if I investigate this further.
Fixes#1326.
6433175ffe Do not invoke fe_is_zero on failed set_b32_limit (Pieter Wuille)
Pull request description:
Noticed in the CI output of #1313 (https://cirrus-ci.com/task/5117786435878912)
The code violates the field element contract that states that a field element that comes out of a failed `secp256k1_fe_set_b32_limit` call cannot be used before overwriting it. This is not an issue in practice, as such failure can only occur with negligible probability, but the experimental compiler in that CI setting is technically correct in detecting this possibility.
Fix it by setting it to 1 based on a `secp256k1_fe_normalizes_to_zero` test rather than a `secp256k1_fe_is_zero` one (which does not require normalization).
ACKs for top commit:
stratospher:
ACK 6433175
real-or-random:
utACK 6433175ffe
Tree-SHA512: 49da4535181c4607c1f4d23d1fd7cd65e7751c7cfa68643f1da77f3ec7961754fc8553bb415137fd61d86c805fe69f5adf97c05b9dc4d3bf357ae7c6409cc51a
5768b50229 build: Enable -DVERIFY for precomputation binaries (Tim Ruffing)
Pull request description:
because... why not?!
I realized that this can't hurt when working on #1313.
ACKs for top commit:
sipa:
ACK 5768b50229
Tree-SHA512: 2412cb93097f5c7904cfded6816bc5cdc69d958b4023ddaffd6e7575615ac5bfcd3a7cfc9ce2c0b0e6526a6f000dd84ecd32909d9d207a3644aadb5d34905911
31b4bbee1e Make fe_cmov take max of magnitudes (Pieter Wuille)
Pull request description:
This addresses part of #1001.
The magnitude and normalization of the output of `secp256k1_fe_cmov` should not depend on the runtime value of `flag`.
ACKs for top commit:
real-or-random:
utACK 31b4bbee1e
stratospher:
ACK 31b4bbe.
Tree-SHA512: 08bef9f63797cb8a1f3ea63c716c09aaa267dfee285b74ef5fbb47d614569d2787ec73d21bce080214872dfe70246f73cea42ad3c24e6baccecabe3312f71433
3ad1027a40 Revert "Remove unused scratch space from API" (Jonas Nick)
Pull request description:
This reverts commit 712e7f8722.
Removing the scratch space from the API may break bindings to the library.
ACKs for top commit:
sipa:
ACK 3ad1027a40
real-or-random:
ACK 3ad1027a40
Tree-SHA512: ad394c0a2f83fe3a5f400c0e8f2b9bf40037ce4141d4414e6345918f5e6003c61da02a538425a49bdeb5700f5ecb713bd58f5752c0715fb1fcc4950099fdc0e6
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
