e82144edfb Fixup skew before global Z fixup (Peter Dettman)
40b624c90b Add tests for _gej_cmov (Peter Dettman)
8c13a9bfe1 ECDH skews by 0 or 1 (Peter Dettman)
1515099433 Simpler and faster ecdh skew fixup (Peter Dettman)
Pull request description:
This PR adds a `_gej_cmov` method, with accompanying tests, and uses it to simplify the skew fixup at the end of `_ecmult_const`.
In the existing code, `_wnaf_const` chooses a skew of either 1 or 2, and `_ecmult_const` needs a call to `_ge_set_gej` (which does an expensive field inversion internally) and some overly-complicated conversions to/from `_ge_storage` so that `_ge_storage_cmov` can be used to select what value to add for the fixup.
This PR uses a simpler scheme where `_wnaf_const` chooses a skew of 0 or 1 and no longer needs special handling for scalars with value negative one. A new `_gej_cmov` method is used at the end of `_ecmult_const` for const-time optional addition to adjust the final result for the skew. Finally, the skew fixup is moved to before the global-Z adjustment, and the precomputed table entries (for 1P, λ(1P)) are used for the skew fixup, saving a field multiply and ensuring the fixup is done on the same isomorphism as the ladder.
The resulting `_wnaf_const` and `_ecmult_const` are shorter and simpler, and the ECDH benchmark is around 5% faster (64bit, i7).
Edit: Updated description once the final scope was clear.
ACKs for top commit:
apoelstra:
ACK e82144ed
sipa:
ACK e82144edfb
real-or-random:
ACK e82144edfb
Tree-SHA512: 10d6770f4ef4f8d0c78abbf58d643f25f5daef68896643af0a3f7f877414e23356724b6f20af2027316a4353a35b8cb0a7851e057a3f6483897df02bf033a8a2
3d7cbafb5f tests: Fix test whose result is implementation-defined (Tim Ruffing)
Pull request description:
A compiler may add struct padding and fe_cmov is not guaranteed to
preserve it.
On the way, we restore the name of the function. It was mistakenly
renamed in 6173839c90 using
"search and replace".
ACKs for top commit:
robot-dreams:
ACK 3d7cbafb5f
sipa:
utACK 3d7cbafb5f
Tree-SHA512: f8bb643d4915e9ce9c4fe45b48a2878f6cf1f29e654be1c150cdf65c6959cf65f8491928cf098da5a01f1d488ba475914905ca96b232abed499eb6ed65e53fb8
77a19750b4 Use xoshiro256++ PRNG instead of RFC6979 in tests (Pieter Wuille)
5f2efe684e secp256k1_testrand_int(2**N) -> secp256k1_testrand_bits(N) (Pieter Wuille)
Pull request description:
Just some easy low-hanging fruit. It's complete overkill to use the RFC6979 RNG for our test randomness. Replace it with a modern non-cryptographic RNG with good properties. It's a few % speedup for me.
Given the internal naming of all these functions to be "testrand", I'm not concerned about the risk of someone using this for something that needs actual cryptographic randomness.
ACKs for top commit:
robot-dreams:
ACK 77a19750b4
real-or-random:
utACK 77a19750b4
Tree-SHA512: 2706f37689e037e84b5df25c98af924c0756e6d59f5f822b23aec5ba381b2d536e0848f134026e2568396427218f1c770f1bb07613d702efb23a84015dc9271d
3ed0d02bf7 doc: add CHANGELOG template (Jonas Nick)
6f42dc16c8 doc: add release_process.md (Jonas Nick)
0bd3e4243c build: set library version to 0.0.0 explicitly (Jonas Nick)
b4b02fd8c4 build: change libsecp version from 0.1 to 0.1.0-pre (Jonas Nick)
Pull request description:
This is an attempt at a simple release process. Fixes#856. To keep it simple, there is no concept of release candidates for now.
The release version is determined by semantic versioning of the API. Since it does not seem to be a lot of work, it is proper to also version the ABI with the libtool versioning system. This versioning scheme (semver API, libtool versioning ABI) follows the suggestion in the [autotools mythbusters](https://autotools.io/libtool/version.html).
Experimental modules are a bit of a headache, as expected. This release process suggests to treat any change in experimental modules as backwards compatible. That way, users of stable modules are not bothered by frequent non-backwards compatible releases. But a downside is that one must not use experimental modules in shared libraries (which should be mentioned in the README?). It would be nice if we could make the schnorrsig module stable in the not too distant future (see also #817).
ACKs for top commit:
apoelstra:
utACK 3ed0d02bf7
elichai:
ACK 3ed0d02bf7
sipa:
ACK 3ed0d02bf7
real-or-random:
ACK 3ed0d02bf7
Tree-SHA512: 25a04335a9579e16de48d378b93a9c6a248529f67f7c436680fa2d495192132743ce016c547aa9718cdcc7fe932de31dd7594f49052e8bd85572a84264f2dbee
05e049b73c ecmult: move `_ecmult_odd_multiples_table_globalz_windowa` (siv2r)
Pull request description:
Fixes#1035
**Changes:**
- move `secp256k1_ecmult_odd_multiples_table_globalz_windowa` function from ecmult to ecmult_const
- remove outdated comment
ACKs for top commit:
robot-dreams:
utACK 05e049b73c (`diff` between removed and added lines is exactly as expected)
real-or-random:
utACK 05e049b73c
Tree-SHA512: 3fad4e93c641b642e84f4bbafcb8083d3e63b0523009fe0edcb2c1ebe1571d822320451289c651403ed1dc033ec6a7a3e8c3c56ad93d81bb1590cf9ff15a3b34
A compiler may add struct padding and fe_cmov is not guaranteed to
preserve it.
On the way, we improve the identity check such that it covers the
VERIFY struct members.
b4ac1a1d5f ci: Run valgrind/memcheck tasks with 2 CPUs (Tim Ruffing)
e70acab601 ci: Use Cirrus "greedy" flag to use idle CPU time when available (Tim Ruffing)
d07e30176e ci: Update brew on macOS (Tim Ruffing)
22382f0ea0 ci: Test different ecmult window sizes (Tim Ruffing)
26a022a3a0 ci: Remove STATICPRECOMPUTATION (Tim Ruffing)
10461d8bd3 precompute_ecmult: Always compute all tables up to default WINDOW_G (Tim Ruffing)
Pull request description:
ACKs for top commit:
elichai:
utACK b4ac1a1d5f
jonasnick:
ACK b4ac1a1d5f
Tree-SHA512: b283d7b1c72cf87484de1fe98318298698fe9982dc33389eaca62e92318ab0074c183b9799add274f46358032491fee875e5ffb2a76a47f3b07520b850f4c85e
1287786c7a doc: Add comment to top of field_10x26_impl.h (Elliott Jin)
58da5bd589 doc: Fix upper bounds + cleanup in field_5x52_impl.h comment (Elliott Jin)
Pull request description:
When reviewing #816 I noticed the upper bounds in the comment at the top of `field_5x52_impl.h` were off by 1 (see `fe_verify`). This PR fixes the upper bounds and also cleans up the comment along the way.
ACKs for top commit:
real-or-random:
ACK 1287786c7a
Tree-SHA512: 4b7dadc92451ab1ceb5a547a3101ff37f3ffd0645490563f1f3442ea8d6219f100ed914289d22435c4172d190fa1ff52e37e4464132bb3f9bbcc338488227f7b
22d25c8e0a Add another ecmult_multi test (Pieter Wuille)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 22d25c8e0a
Tree-SHA512: e1394fa1708e65a66d4b324cca60dd49c67e37b23b7da2a3ff0db7a2a25c23976cb03b96a8c8584ee81aaec559feb84fb113dff2e2ebf89110ed466a4a6b158b
The preinstalled brew is very old and tries to download prebuilt bottles
from a server which is no longer available. Because that will fail, brew
falls back to building our dependencies (e.g., autotools) from source,
which takes very long.
This commit makes sure that brew is updated before we start the build.
We also need to remove the `--shallow` argument from `brew tap`. It
doesn't exist in recent brew versions.
515e7953ca Improve checks at top of _fe_negate methods (Peter Dettman)
Pull request description:
In theory we could have a single static assertion that would ensure all of these are always true (for any magnitude up to the limit), but I think this small redundancy is clearer.
ACKs for top commit:
sipa:
utACK 515e7953ca
real-or-random:
ACK 515e7953ca bounds hold by inspection and by robot-dreams's script
Tree-SHA512: c33e47e186b37ca0b4e8d23712f8e5ab0c113024a0229fc6ce63b8cbad21bddbecc0c50029721a1fb3376b2d1da678c1ddb69c5ae971d84ddb7993c755867da4
e05da9e480 Fix c++ build (Pieter Wuille)
c45386d994 Cleanup preprocessor indentation in precompute{,d}_ecmult{,_gen} (Pieter Wuille)
19d96e15f9 Split off .c file from precomputed_ecmult.h (Pieter Wuille)
1a6691adae Split off .c file from precomputed_ecmult_gen.h (Pieter Wuille)
bb36331412 Simplify precompute_ecmult_print_* (Pieter Wuille)
38cd84a0cb Compute ecmult tables at runtime for tests_exhaustive (Pieter Wuille)
e458ec26d6 Move ecmult table computation code to separate file (Pieter Wuille)
fc1bf9f15f Split ecmult table computation and printing (Pieter Wuille)
31feab053b Rename function secp256k1_ecmult_gen_{create_prec -> compute}_table (Pieter Wuille)
725370c3f2 Rename ecmult_gen_prec -> ecmult_gen_compute_table (Pieter Wuille)
075252c1b7 Rename ecmult_static_pre_g -> precomputed_ecmult (Pieter Wuille)
7cf47f72bc Rename ecmult_gen_static_prec_table -> precomputed_ecmult_gen (Pieter Wuille)
f95b8106d0 Rename gen_ecmult_static_pre_g -> precompute_ecmult (Pieter Wuille)
bae77685eb Rename gen_ecmult_gen_static_prec_table -> precompute_ecmult_gen (Pieter Wuille)
Pull request description:
This PR implements a number of changes to follow up after merging #988:
* Naming consistency:
* All precomputed table files now have name `precomputed_*.*`
* All source files related to the creation of the precomputed table files have name `precompute_*.*`.
* All source files related to the computation of tables (whether they go in precomputed files or not) have name `*_compute_table.*`.
* Make the tables for exhaustive tests be computed at runtime rather than compile time (this was already the case for ecmult_gen, but not ecmult). This is a preparation for the next point, as the alternative would be to have separate precomputed libraries for the exhaustive tests and other binaries.
* Moves the actual tables to separate `precomputed_*.c` files, which are compiled only once as part of a new `libsecp256k1_precomputed.la`, included where relevant. The corresponding `precomputed_*.h` file are normal source files.
Retry of #1041.
ACKs for top commit:
real-or-random:
ACK e05da9e480
jonasnick:
ACK e05da9e480
Tree-SHA512: 71eadd66e30e511b786e910755e0eda53330dfa163b37e33602c3392f7b893569f56d3ca9870e85cbb3de83880fc5aef61ac3d55d759d7395086a69023f13f03
7dfceceea6 build: Remove #undef hack for ASM in the precomputation programs (Tim Ruffing)
bb36fe9be0 ci: Test `make precomp` (Tim Ruffing)
d94a37a20c build: Remove CC_FOR_BUILD stuff (Tim Ruffing)
ad63bb4c29 build: Prebuild and distribute ecmult_gen table (Tim Ruffing)
ac49361ed0 prealloc: Get rid of manual memory management for prealloc contexts (Tim Ruffing)
6573c08f65 ecmult_gen: Tidy precomputed file and save space (Tim Ruffing)
5eba83f17c ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS (Tim Ruffing)
fdb33dd122 refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table (Tim Ruffing)
a4875e30a6 refactor: Move default callbacks to util.h (Tim Ruffing)
4c94c55bce doc: Remove obsolete hint for valgrind stack size (Tim Ruffing)
5106226991 exhaustive_tests: Fix with ecmult_gen table with custom generator (Tim Ruffing)
e1a76530db refactor: Make generator a parameter of ecmult_gen_create_prec_table (Tim Ruffing)
9ad09f6911 refactor: Rename program that generates static ecmult_gen table (Tim Ruffing)
8ae18f1ab3 refactor: Rename file that contains static ecmult_gen table (Tim Ruffing)
00d2fa116e ecmult_gen: Make code consistent with comment (Tim Ruffing)
3b0c2185ea ecmult_gen: Simplify ecmult_gen context after making table static (Tim Ruffing)
e43ba02cfc refactor: Decouple table generation and ecmult_gen context (Tim Ruffing)
22dc2c0a0d ecmult_gen: Move table creation to new file and force static prec (Tim Ruffing)
Pull request description:
This resolves#893, resolves#692 (and also resolvesbitcoin/bitcoin#22854).
- [x] Extract table generation to separate function in separate file (to be used by generation script and exhaustive tests)
- [x] Tidy up
- [x] Remove code that deals with non-static tables
- [x] Make functions that need ecmult_gen not depend on signing context
- [x] Rename stuff to make it fit the new structure and consistent with how we hande verification tables (#956)
- [x] Fix exhaustive tests
- [x] Make table generation function take generator as input
- [x] Overwrite the static tables with a table with custom generator in exhaustive tests
- [x] Overhaul script that generates table files
- [x] Make table generation function take PREC_BITS as input (I have some code already, just not yet in this branch)
- [x] Change generation script to generate three tables (for all three values of ECMULT_GEN_PREC_BITS)
- [x] Ship pre-built tables
- [x] Add pregenerated table file to repo
- [x] Remove generation of table file from build process (like in #956)
- [x] Remove left-over stuff (e.g., detecting a compiler running on the build machine) from build system
- [x] Final cleanups (copyright headers, commit, messages, etc.)
- [ ] (separate PR:) Make sure link-time optimization remove corresponding static tables (and code) when no signing/verifcation function is called
- [ ] (separate PR:) Compile precomputation as a separate object file and link it (https://github.com/bitcoin-core/secp256k1/pull/988#issuecomment-977813538)
- [ ] (separate PR:) Document the backwards-compatible API changes made in this PR and in #956.
- [ ] Maybe deprecate the static context
ACKs for top commit:
sipa:
ACK 7dfceceea6
robot-dreams:
ACK 7dfceceea6 (based on range-diff between 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1 and 7dfceceea6)
Tree-SHA512: 6efb3f36f05efe3b79bbd877881fe1409f71fd6488d24c811b2e77d9f053bed78670dd1dcbb42ad780458a51c4ffa36de9cd6567271b22041dc7a122ceb677c5
Russell O'Connor
Paul Miller
Pieter Wuille
Peter Dettman
Tim Ruffing
siv2r
Jonas Nick
Elliott Jin