bench: add internal benchmark for secp256k1_fe_normalize_var#1855
Merged
real-or-random merged 1 commit intoJun 9, 2026
Merged
Conversation
furszy
reviewed
May 30, 2026
Contributor
|
ACK 8b066a4 Worth noting that this benchmarks the optimistic path? It makes sense to do so, because the worst-case path is very unlikely to be needed. |
theStack
force-pushed
the
add-field_normalize_var-bench
branch
from
8b066a4 to
240578e
Compare
Contributor
Author
Makes sense, thanks. Added a corresponding comment. |
vmta
added a commit
to umkoin/umkoin
that referenced
this pull request
Jun 9, 2026
0f4a7e6bf Merge bitcoin-core/secp256k1#1855: bench: add internal benchmark for `secp256k1_fe_normalize_var` 13db747f2 Merge bitcoin-core/secp256k1#1861: refactor: introduce `_ecmult_gen_ge` helper (preventing accidental gej leaks) 9e017e506 refactor: rename `_ecmult_gen` -> `_ecmult_gen_gej` for consistency a3296d5e2 refactor: introduce `_ecmult_gen_ge` helper (preventing accidental gej leaks) c63062380 Merge bitcoin-core/secp256k1#1852: Add exhaustive test for ECDH module 240578eef bench: add internal benchmark for `secp256k1_fe_normalize_var` 5698e66c6 Add exhaustive test for ECDH module a39093de1 Merge bitcoin-core/secp256k1#1851: doc: correct API docs for ECDSA signing out-params (s/array/signature object/) 8363a2d8d Merge bitcoin-core/secp256k1#1854: tests: compare full MuSig aggregate nonce af1fdd121 tests: compare full MuSig aggregate nonce 40a0d874a doc: correct API docs for ECDSA signing out-params (s/array/signature object/) b11340b3c Merge bitcoin-core/secp256k1#1849: musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter` 8479eafa5 musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter` c1a9e4fe6 Merge bitcoin-core/secp256k1#1848: ci: Bump GCC snapshot major version to 17 3cca6451a ci: Bump GCC snapshot major version to 17 ea174fe04 Merge bitcoin-core/secp256k1#1846: ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup 285cb788e ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup 7262adb4b Merge bitcoin-core/secp256k1#1841: gha: Bump deprecated GHA workflow dependencies c5cd9d6d9 gha: Bump deprecated GHA workflow dependencies 95b702de3 Merge bitcoin-core/secp256k1#1839: ecdsa: VERIFY_CHECK result of _fe_set_b32_limit 634215f3f Merge bitcoin-core/secp256k1#1837: tests: Fix function pointer initialization C89 error in ellswift tests 43fca0ff5 ecdsa: VERIFY_CHECK result of _fe_set_b32_limit b84635ed3 tests: Fix C89 function pointer initialization in ellswift tests ffc25a273 Merge bitcoin-core/secp256k1#1834: ecmult: Document and test ng=NULL in ecmult 3a403639d eckey: Call ecmult with NULL instead of zero scalar 7e68c0c88 ecmult: Document and test ng=NULL in ecmult 1aafe1513 Merge bitcoin-core/secp256k1#1777: Make SHA256 compression runtime pluggable b9cb1cbfd Merge bitcoin-core/secp256k1#1824: util: introduce and use `ARRAY_SIZE` macro 4d92a083b sha256: speed up writes using multi-block compression 0753f8b90 Add API to override SHA256 compression at runtime fdb6a91a5 Introduce hash context to support pluggable SHA256 compression c0a2aba08 Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal 10f546a2c Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish 8d0eda07e testrand: Remove testrand_finish 95e681584 Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions f48b1bfa5 hash: add midstate initializer and use it for tagged hashes 3019186a6 Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION 79e9f2523 ci: Fix leftover use of old ECMULTGENPRECISION dfe042feb Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" 76e92cfee Revert "ci, docker: Fix LLVM repository signature failure" ac561601b Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism c7a7f732b Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 921b9711e util: introduce and use `ARRAY_SIZE` macro b99a94c38 Add tests for bad scalar inputs in ellswift XDH 307b49f1b ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 322d0a435 Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step ed02466d3 ci: Load Docker image by ID from builder step c49c9be50 bench: Update help functions in bench and bench_internal 1d146ac3e Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue #1812) f47bbc07f test: add unit tests for secp256k1_scalar_check_overflow d071aa56d Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts 99ab4a105 Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation c5da3bde9 Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation 97de5120c Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests 4fb7ccf5d ci: Enforce base-10 evaluation 3ae72e786 ci: Disable Docker build summary generation 97b3c4784 refactor: remove unnecessary `malloc` result casts 57315a698 Merge bitcoin-core/secp256k1#1813: Remove trailing spaces and introduce `-Wtrailing-whitespace=any` compiler flag 86cae58d2 build: Add `-Wleading-whitespace=spaces` compiler flag fb229e760 build: Add `-Wtrailing-whitespace=any` compiler flag 13e3bee50 refactor: Remove trailing whitespace 453949ab2 Merge bitcoin-core/secp256k1#1816: ci: Rotate Docker cache keys cd49c57e4 Merge bitcoin-core/secp256k1#1814: release process: mention the `[Unreleased]` link clearly 2ccff6eb7 ci: Add weekly schedule 2f18567d2 ci: Rotate Docker cache keys every 4 weeks 0ffb1749a ci, docker: Fix LLVM repository signature failure 0267b6551 release process: mention the `[Unreleased]` link clearly 1605b02f7 Merge bitcoin-core/secp256k1#1775: Add CMake build directory patterns to `.gitignore` 14e56970c Merge bitcoin-core/secp256k1#1794: ecmult: Use size_t for array indices c7a52400d Merge bitcoin-core/secp256k1#1809: release cleanup: bump version after 0.7.1 ae7eb729c release cleanup: bump version after 0.7.1 1a53f4961 Merge bitcoin-core/secp256k1#1808: Prepare for 0.7.1 20a209f11 release: prepare for 0.7.1 c4b6a81a6 changelog: update in preparation for the v0.7.1 release ebb35882d Merge bitcoin-core/secp256k1#1796: bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS c09215f7a bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS 1bc74a22f test: show both Autotools and CMake usage for ctime_tests 471e3a130 Merge bitcoin-core/secp256k1#1800: sage: verify Eisenstein integer connection for GLV constants 8354618e0 cmake: Set `LABELS` property for tests 29f26ec3c cmake: Integrate DiscoverTests and normalize test names 29ac4d849 sage: verify Eisenstein integer connection for GLV constants f95b263f2 cmake: Add DiscoverTests module 4ac651144 cmake, refactor: Deduplicate test-related code 4721e077b Merge bitcoin-core/secp256k1#1793: doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult bd5ced1fe doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult 47eb70959 ecmult: Use size_t for array indices in _odd_multiplies_table bb1d199de ecmult: Use size_t for array indices into tables 2d9137ce9 Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules f9a944ff2 Merge bitcoin-core/secp256k1#1790: doc: include arg -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON for cmake 0406cfc4d doc: include arg -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1 for cmake 8d445730e Merge bitcoin-core/secp256k1#1783: Add VERIFY_CHECKs and documentation that flags must be 0 or 1 aa2a39c1a Merge bitcoin-core/secp256k1#1778: doc/bench: Added cmake build options to bench error messages 540fec8ae Merge bitcoin-core/secp256k1#1788: test: split monolithic ellswift test into independent cases d822b2902 test: split monolithic ellswift test into independent cases ae00c552d Add VERIFY_CHECKs that flags are 0 or 1 5c7518334 Merge bitcoin-core/secp256k1#1784: refactor: remove ret from secp256k1_ec_pubkey_serialize be5e4f02f Merge bitcoin-core/secp256k1#1779: Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL 3daab83a6 refactor: remove ret from secp256k1_ec_pubkey_serialize 8bcda186d test: Add non-NULL checks for "pointer of array" API functions 5a08c1bcd Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL 3b5b03f30 doc/bench: Added cmake build options to bench error messages e7f7083b5 Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants 748c0fdd6 Add CMake build directory patterns to `.gitignore` 7eb86bdb0 autotools: Rename `build-aux` to `autotools-aux` b6c2a3cd7 Merge bitcoin-core/secp256k1#1761: ecmult_multi: reduce strauss memory usage by 30% f5e815f43 remove secp256k1_eckey_pubkey_serialize function 0d3659c54 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) adb76f82e use new `_eckey_pubkey_serialize{33,65}` functions in public API fc7458ca3 introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions c8206b1ce Merge bitcoin-core/secp256k1#1771: ci: Use Python virtual environment in "x86_64-macos-native" job f252da7e6 ci: Use Python virtual environment in "x86_64-macos-native" job 115b135fe Merge bitcoin-core/secp256k1#1763: bench: Use `ALIGNMENT` macro instead of hardcoded value 2f73e5281 group: Avoid using infinity field directly in other modules 153eea20c bench: Use `ALIGNMENT` macro instead of hardcoded value 26166c4f5 ecmult_multi: reduce strauss memory usage by 30% 7a2fff85e Merge bitcoin-core/secp256k1#1758: ci: Drop workaround for Valgrind older than 3.20.0 43e7b115f Merge bitcoin-core/secp256k1#1759: ci: Switch to macOS 15 Sequoia Intel-based image 8bc50b72f ci: Switch to macOS 15 Sequoia Intel-based image c09519f0e ci: Drop workaround for Valgrind older than 3.20.0 d543c0d91 Merge bitcoin-core/secp256k1#1734: Introduce (mini) unit test framework f44c1ebd9 Merge bitcoin-core/secp256k1#1719: ci: DRY workflow using anchors a44a33938 Merge bitcoin-core/secp256k1#1750: ci: Use clang-snapshot in "MSan" job 15d014804 ci: Drop default for `inputs.command` in `run-in-docker-action` 1decc49a1 ci: Use YAML anchor and aliases for repeated "CI script" steps dff1bc107 ci, refactor: Generalize use of `matrix.configuration.env_vars` 4b644da19 ci: Use YAML anchor and aliases for repeated "Print logs" steps a889cd93d ci: Bump `actions/checkout` version 574c2f308 ci: Use YAML anchor and aliases for repeated "Checkout" steps 53585f93b ci: Use clang-snapshot in "MSan" job 6894c964f Fix Clang 21+ `-Wuninitialized-const-pointer` warning when using MSan 2b7337f63 Merge bitcoin-core/secp256k1#1756: ci: Fix image caching and apply other improvements f163c3589 ci: Set `DEBIAN_FRONTEND=noninteractive` 70ae177ca ci: Bump `docker/build-push-action` version b2a95a420 ci: Drop `tags` input for `docker/build-push-action` 122014edb ci: Add `scope` parameter to `cache-{to,from}` options 2f4546ce5 test: add --log option to display tests execution 95b9953ea test: Add option to display all available tests 953f7b008 test: support running specific tests/modules targets 0302c1a3d test: add --help for command-line options 9ec3bfe22 test: adapt modules to the new test infrastructure 48789dafc test: introduce (mini) unit test framework baa265429 Merge bitcoin-core/secp256k1#1727: docs: Clarify that callback can be called more than once 4d90585fe docs: Improve API docs of _context_set_illegal_callback 895f53d1c docs: Clarify that callback can be called more than once de6af6ae3 Merge bitcoin-core/secp256k1#1748: bench: improve context creation in ECDH benchmark 581788515 Merge bitcoin-core/secp256k1#1749: build: Fix warnings in x86_64 assembly check ab560078a build: Fix warnings in x86_64 assembly check 10dab907e Merge bitcoin-core/secp256k1#1741: doc: clarify API doc of `secp256k1_ecdsa_recover` return value dfe284ed2 bench: improve context creation in ECDH benchmark 7321bdf27 doc: clarify API doc of `secp256k1_ecdsa_recover` return value b47565430 Merge bitcoin-core/secp256k1#1745: test: introduce group order byte-array constant for deduplication 9cce70386 refactor: move 'gettime_i64()' to tests_common.h 0c91c5604 test: introduce group order byte-array constant for deduplication 88be4e8d8 Merge bitcoin-core/secp256k1#1735: musig: Invalidate secnonce in secp256k1_musig_partial_sign 399b582a5 Split memclear into two versions git-subtree-dir: src/secp256k1 git-subtree-split: 0f4a7e6bf9d971addb2b851df7cd2777fc62b212
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While addressing the review suggestion #1765 (comment) (b10c mirror link), I noticed that we don't have an internal benchmark for the variable-time variant of
_fe_normalizeyet, so this PR adds one. IIUC it's fine to repeatedly apply the operation on the same (already normalized at latest after the first loop iteration) field element for benchmarking purposes and don't put in an effort to reach the final reduction code path, considering how extremely unlikely it is to reach it in practice.Results on my machine: