Upstream PR 1058#1
Closed
DarkWindman wants to merge 21 commits intotemp-merge-1515from
Closed
Conversation
Instead of having the starting point of the ecmult_gen computation be offset, do it with the final point. This enables reasoning over the set of points reachable in intermediary computations, which can be leveraged by potential future optimization. Because the final point is in affine coordinates, its projective blinding is no longer possible. It will be reintroduced again in a different way, in a later commit. Also introduce some more comments and more descriptive names.
The old code overwrote the input at the start of the function, making a call like secp256k1_scalar_inverse(&x,&x) always fail.
This introduces the signed-digit multi-comb multiplication algorithm for constant-time G multiplications (ecmult_gen). It is based on section 3.3 of "Fast and compact elliptic-curve cryptography" by Mike Hamburg (see https://eprint.iacr.org/2012/309). Original implementation by Peter Dettman, with changes by Pieter Wuille to use scalars for recoding, and additional comments.
It is unnecessary to recompute this term needed by the SDMC algorithm for every multiplication; move it into the context scalar_offset value instead.
The old code would trigger UB when count=32.
The existing code needs to deal with the edge case that bit_pos >= 256, which would lead to an out-of-bounds read from secp256k1_scalar. Instead, recode the scalar into an array of uint32_t with enough zero padding at the end to alleviate the issue. This also simplifies the code, and is necessary for a security improvement in a follow-up commit. Original code by Peter Dettman, with modifications by Pieter Wuille.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
… algorithm 4c341f8 Add changelog entry for SDMC (Pieter Wuille) a043940 Permit COMB_BITS < 256 for exhaustive tests (Pieter Wuille) 39b2f2a Add test case for ecmult_gen recoded = {-1,0,1} (Pieter Wuille) 644e86d Reintroduce projective blinding (Pieter Wuille) 07810d9 Reduce side channels from single-bit reads (Peter Dettman) a0d32b5 Optimization: use Nx32 representation for recoded bits (Peter Dettman) e03dcc4 Make secp256k1_scalar_get_bits support 32-bit reads (Pieter Wuille) 5005abe Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t (Pieter Wuille) 6247f48 Optimization: avoid unnecessary doublings in precomputation (Peter Dettman) 15d0cca Optimization: first table lookup needs no point addition (Pieter Wuille) 7a33db3 Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset (Pieter Wuille) ed2a056 Provide 3 configurations accessible through ./configure (Pieter Wuille) 5f7be9f Always generate tables for current (blocks,teeth) config (Pieter Wuille) fde1dfc Signed-digit multi-comb ecmult_gen algorithm (Peter Dettman) 486518b Make exhaustive tests's scalar_inverse(&x,&x) work (Pieter Wuille) ab45c3e Initial gej blinding -> final ge blinding (Pieter Wuille) aa00a6b Introduce CEIL_DIV macro and use it (Tim Ruffing) Pull request description: ACKs for top commit: real-or-random: reACK 4c341f8 jonasnick: ACK 4c341f8 stratospher: ACK 4c341f8. Did [these benchmarks](bitcoin-core/secp256k1#1058 (comment)) and saw a 12.4% on gcc 13.2.0 and 11.5% on clang 15.0.0. Also summarised how the precomputed table generation works [here](https://github.com/stratospher/blogosphere/blob/main/sdmc.md) for future me :) Tree-SHA512: 9a11138e4fb98b98e85c82cd46ed78b29fbe63d6efe61654ef519a64b1e175d63395a8a931c1646f9df8c7daacd796d5fe2384899d5a13a2c7ed2ded696ceed5
DarkWindman
force-pushed
the
temp-merge-1515
branch
from
0a9fd83 to
f9cf003
Compare
DarkWindman
force-pushed
the
temp-merge-1058
branch
from
5688b07 to
eaf10ed
Compare
mllwchrry
pushed a commit
that referenced
this pull request
Mar 3, 2026
…to improve parallelism 8354618 cmake: Set `LABELS` property for tests (Hennadii Stepanov) 29f26ec cmake: Integrate DiscoverTests and normalize test names (Hennadii Stepanov) f95b263 cmake: Add DiscoverTests module (Hennadii Stepanov) 4ac6511 cmake, refactor: Deduplicate test-related code (Hennadii Stepanov) Pull request description: This PR implements the idea suggested in bitcoin-core/secp256k1#1734 (review) and is based on the work from bitcoin/bitcoin#33483. Here is an example of the `ctest` output: ``` $ ctest --test-dir build -j $(nproc) Test project /home/hebasto/dev/secp256k1/secp256k1/build Start 1: secp256k1.noverify_tests.selftest_tests Start 2: secp256k1.noverify_tests.all_proper_context_tests Start 3: secp256k1.noverify_tests.all_static_context_tests Start 4: secp256k1.noverify_tests.deprecated_context_flags_test <snip> 193/196 Test BlockstreamResearch#31: secp256k1.noverify_tests.ecmult_constants ......................... Passed 5.32 sec 194/196 Test BlockstreamResearch#184: secp256k1.tests.ellswift_xdh_correctness_tests .................... Passed 5.62 sec 195/196 Test BlockstreamResearch#191: secp256k1.exhaustive_tests ........................................ Passed 6.97 sec 196/196 Test BlockstreamResearch#126: secp256k1.tests.ecmult_constants .................................. Passed 9.60 sec 100% tests passed, 0 tests failed out of 196 Label Time Summary: secp256k1_example = 0.02 sec*proc (5 tests) secp256k1_exhaustive = 6.97 sec*proc (1 test) secp256k1_noverify_tests = 23.77 sec*proc (95 tests) secp256k1_tests = 43.67 sec*proc (95 tests) Total Test time (real) = 10.21 sec ``` For comparison, here is the output for the master branch on the same machine: ``` $ ctest --test-dir build -j $(nproc) Test project /home/hebasto/dev/secp256k1/secp256k1/build Start 1: secp256k1_noverify_tests Start 2: secp256k1_tests Start 3: secp256k1_exhaustive_tests Start 4: secp256k1_ecdsa_example Start 5: secp256k1_ecdh_example Start 6: secp256k1_schnorr_example Start 7: secp256k1_ellswift_example Start 8: secp256k1_musig_example 1/8 Test #4: secp256k1_ecdsa_example .......... Passed 0.00 sec 2/8 Test #5: secp256k1_ecdh_example ........... Passed 0.00 sec 3/8 Test #6: secp256k1_schnorr_example ........ Passed 0.00 sec 4/8 Test #7: secp256k1_ellswift_example ....... Passed 0.00 sec 5/8 Test #8: secp256k1_musig_example .......... Passed 0.00 sec 6/8 Test #3: secp256k1_exhaustive_tests ....... Passed 6.26 sec 7/8 Test #1: secp256k1_noverify_tests ......... Passed 14.31 sec 8/8 Test #2: secp256k1_tests .................. Passed 31.65 sec 100% tests passed, 0 tests failed out of 8 Total Test time (real) = 31.65 sec ``` --- **New Feature:** As the number of tests has grown, the _labels_ have been introduced to simplify test management. Now, one can run: ``` $ ctest --test-dir build -j $(nproc) -L example Test project /home/hebasto/dev/secp256k1/secp256k1/build Start 192: secp256k1.example.ecdsa Start 193: secp256k1.example.ecdh Start 194: secp256k1.example.schnorr Start 195: secp256k1.example.ellswift Start 196: secp256k1.example.musig 1/5 Test BlockstreamResearch#192: secp256k1.example.ecdsa .......... Passed 0.00 sec 2/5 Test BlockstreamResearch#193: secp256k1.example.ecdh ........... Passed 0.00 sec 3/5 Test BlockstreamResearch#194: secp256k1.example.schnorr ........ Passed 0.00 sec 4/5 Test BlockstreamResearch#195: secp256k1.example.ellswift ....... Passed 0.00 sec 5/5 Test BlockstreamResearch#196: secp256k1.example.musig .......... Passed 0.00 sec 100% tests passed, 0 tests failed out of 5 Label Time Summary: secp256k1_example = 0.01 sec*proc (5 tests) Total Test time (real) = 0.01 sec ``` or ``` $ ctest --test-dir build -j $(nproc) -LE tests Test project /home/hebasto/dev/secp256k1/secp256k1/build Start 192: secp256k1.example.ecdsa Start 193: secp256k1.example.ecdh Start 194: secp256k1.example.schnorr Start 195: secp256k1.example.ellswift Start 196: secp256k1.example.musig Start 191: secp256k1.exhaustive_tests 1/6 Test BlockstreamResearch#192: secp256k1.example.ecdsa .......... Passed 0.00 sec 2/6 Test BlockstreamResearch#193: secp256k1.example.ecdh ........... Passed 0.00 sec 3/6 Test BlockstreamResearch#194: secp256k1.example.schnorr ........ Passed 0.00 sec 4/6 Test BlockstreamResearch#195: secp256k1.example.ellswift ....... Passed 0.00 sec 5/6 Test BlockstreamResearch#196: secp256k1.example.musig .......... Passed 0.00 sec 6/6 Test BlockstreamResearch#191: secp256k1.exhaustive_tests ....... Passed 6.19 sec 100% tests passed, 0 tests failed out of 6 Label Time Summary: secp256k1_example = 0.01 sec*proc (5 tests) secp256k1_exhaustive = 6.19 sec*proc (1 test) Total Test time (real) = 6.20 sec ``` ACKs for top commit: purpleKarrot: ACK 8354618 furszy: Tested ACK 8354618 Tree-SHA512: 8c506ab08491aba4836b3058a8a09c929c6dd097c11e4e6f4deb20cf602285e73c3fd8a2c2040f7e92a058c7f8fc09752fa9de2ce80f7673adbdd505237ed262
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
6 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.
Merge bitcoin-core/secp256k1#1058: Signed-digit multi-comb ecmult_gen algorithm
Stacked on top of branch
temp-merge-1515.This PR can be recreated with
./contrib/sync-upstream-stacked.sh temp-merge-1515 select da515074e3ebc8abc85a4fff3a31d7694ecf897b.Tip: Use
git show --remerge-diffto show the changes manually added to the merge commit.' --base 'temp-merge-1515' --web