-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Signed-digit based ecmult_const algorithm #1184
Signed-digit based ecmult_const algorithm #1184
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa nice! The skew correction is really annoying to reason about...
I just don't know where to get all the review power from.
a0c7934
to
f16c500
Compare
Added a commit to remove |
@peterdettman Randomly tagging you, you may find this interesting. |
Very cool! I wonder if the explanation might be clearer if you said something like "without any further optimizations, k0 would be 2^n - 1 with n = 256; however we are going to combine this scheme with the GLV endomorphism and do a windowed rather than bitwise approach. So instead we solve for k0" rather than "for some constant k0". In the end I was able to understand what you were doing without much trouble, so what you've written is fine, but it was a bit intimidating to see k0, k1, k2 all introduced at once just as unknowns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f16c500
f16c500
to
b4ff4eb
Compare
@apoelstra I've changed the explanation to introduce the offset terms a bit more gently. I've also dropped the |
@sipa this looks way better, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK b4ff4eb
b4ff4eb
to
4de39ef
Compare
EDIT: no, doesn't work |
4de39ef
to
1c48861
Compare
Oops, my previous rebase accidentally reverted the changes I made to address @apoelstra's comment. Re-rebased. |
4436ce2
to
a3b1a90
Compare
Are you able to explain why that doesn't work? |
Nice idea, simple and clear.
|
a3b1a90
to
dd21e2c
Compare
I've switched to offset |
I think if _scalar_split_lambda just output 129-bit two's complement values (and then sign-extend to ECMULT_CONST_BITS), then they would be directly usable in the comb, no offset needed for s1, s2. Something for the future, perhaps. |
Rebased. |
f773744
to
c8eb787
Compare
Rebased and addressed the comments above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK mod these nits
also utACK on jonas' fixups (except a typo and one further comment here: jonasnick@cdf545a )
I believe I've done a thorough review, but I really have only nits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK c8eb787.
was able to follow the useful comments and code! also observed a 2% speedup on my machine.
Co-authored-by: Jonas Nick <[email protected]> Co-authored-by: Tim Ruffing <[email protected]>
* add test case for a=infinity The corresponding ecmult_const branch was not tested before this commit. * add test for edge cases
c8eb787
to
355bbdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 355bbdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 355bbdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 355bbdf
This was removed upstream, bitcoin-core/secp256k1#1184, and is causing build failures downstream: ```bash clang++ -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=address -fsanitize-address-use-after-scope -fsanitize=fuzzer-no-link -stdlib=libc++ -DCRYPTOFUZZ_NO_OPENSSL -I /src/boost_1_74_0/ -DCRYPTOFUZZ_SECP256K1 -DCRYPTOFUZZ_TREZOR_FIRMWARE -DCRYPTOFUZZ_BOTAN -DCRYPTOFUZZ_BOTAN_IS_ORACLE -DCRYPTOFUZZ_BITCOIN -Wall -Wextra -std=c++17 -I include/ -I . -I fuzzing-headers/include -DFUZZING_HEADERS_NO_IMPL bignum_fuzzer_importer.o botan_importer.o builtin_tests_importer.o components.o crypto.o datasource.o driver.o ecc_diff_fuzzer_exporter.o ecc_diff_fuzzer_importer.o entry.o executor.o expmod.o mutator.o mutatorpool.o numbers.o openssl_importer.o operation.o options.o repository.o tests.o util.o wycheproof.o z3.o modules/trezor/module.a modules/secp256k1/module.a modules/bitcoin/module.a modules/botan/module.a -fsanitize=fuzzer third_party/cpu_features/build/libcpu_features.a -o cryptofuzz /usr/bin/ld: /usr/bin/ld: DWARF error: invalid or unhandled FORM value: 0x25 modules/secp256k1/module.a(secp256k1_api.o): in function `cryptofuzz_secp256k1_scalar_shr_int': secp256k1_api.c:(.text.cryptofuzz_secp256k1_scalar_shr_int[cryptofuzz_secp256k1_scalar_shr_int]+0x27): undefined reference to `secp256k1_scalar_shr_int' /usr/bin/ld: secp256k1_api.c:(.text.cryptofuzz_secp256k1_scalar_shr_int[cryptofuzz_secp256k1_scalar_shr_int]+0x39): undefined reference to `secp256k1_scalar_shr_int' clang-15: error: linker command failed with exit code 1 (use -v to see invocation) make: *** [Makefile:33: cryptofuzz] Error 1 ERROR:__main__:Building fuzzers failed. ```
efe85c70a2 Merge bitcoin-core/secp256k1#1466: release cleanup: bump version after 0.4.1 4b2e06f460 release cleanup: bump version after 0.4.1 1ad5185 Merge bitcoin-core/secp256k1#1465: release: prepare for 0.4.1 672053d release: prepare for 0.4.1 1a81df8 Merge bitcoin-core/secp256k1#1380: Add ABI checking tool for release process 74a4d97 doc: Add ABI checking with `check-abi.sh` to the Release Process e7f830e Add `tools/check-abi.sh` 77af1da Merge bitcoin-core/secp256k1#1455: doc: improve secp256k1_fe_set_b32_mod doc 3928b7c doc: improve secp256k1_fe_set_b32_mod doc 5e9a4d7 Merge bitcoin-core/secp256k1#990: Add comment on length checks when parsing ECDSA sigs 4197d66 Merge bitcoin-core/secp256k1#1431: Add CONTRIBUTING.md 0e5ea62 CONTRIBUTING: add some coding and style conventions e2c9888 Merge bitcoin-core/secp256k1#1451: changelog: add entry for "field: Remove x86_64 asm" d2e36a2 changelog: add entry for "field: Remove x86_64 asm" 1a432cb README: update first sentence 0922a04 docs: move coverage report instructions to CONTRIBUTING 76880e4 Add CONTRIBUTING.md including scope and guidelines for new code d3e29db Merge bitcoin-core/secp256k1#1450: Add group.h ge/gej equality functions 04af0ba Replace ge_equals_ge[,j] calls with group.h equality calls 60525f6 Add unit tests for group.h equality functions a47cd97 Add group.h ge/gej equality functions 10e6d29 Merge bitcoin-core/secp256k1#1446: field: Remove x86_64 asm 07687e8 Merge bitcoin-core/secp256k1#1393: Implement new policy for VERIFY_CHECK and #ifdef VERIFY (issue bitcoin#1381) bb46723 remove VERIFY_SETUP define a3a3e11 remove unneeded VERIFY_SETUP uses in ECMULT_CONST_TABLE_GET_GE macro a0fb68a introduce and use SECP256K1_SCALAR_VERIFY macro cf25c86 introduce and use SECP256K1_{FE,GE,GEJ}_VERIFY macros 5d89bc0 remove superfluous `#ifdef VERIFY`/`#endif` preprocessor conditions c2688f8 redefine VERIFY_CHECK to empty in production (non-VERIFY) mode 5814d84 Merge bitcoin-core/secp256k1#1438: correct assertion for secp256k1_fe_mul_inner c1b4966 Merge bitcoin-core/secp256k1#1445: bench: add --help option to bench_internal f07cead build: Don't call assembly an optimization 2f0762f field: Remove x86_64 asm 1ddd76a bench: add --help option to bench_internal e721039 Merge bitcoin-core/secp256k1#1441: asm: add .note.GNU-stack section for non-exec stack ea47c82 Merge bitcoin-core/secp256k1#1442: Return temporaries to being unsigned in secp256k1_fe_sqr_inner dcdda31 Tighten secp256k1_fe_mul_inner's VERIFY_BITS checks 1027135 Return temporaries to being unsigned in secp256k1_fe_sqr_inner 33dc7e4 asm: add .note.GNU-stack section for non-exec stack c891c5c Merge bitcoin-core/secp256k1#1437: ci: Ignore internal errors of snapshot compilers 8185e72 ci: Ignore internal errors in snapshot compilers 40f50d0 Merge bitcoin-core/secp256k1#1184: Signed-digit based ecmult_const algorithm 8e2a5fe correct assertion for secp256k1_fe_mul_inner 355bbdf Add changelog entry for signed-digit ecmult_const algorithm 21f49d9 Remove unused secp256k1_scalar_shr_int 115fdc7 Remove unused secp256k1_wnaf_const aa9f3a3 ecmult_const: add/improve tests 4d16e90 Signed-digit based ecmult_const algorithm ba523be make SECP256K1_SCALAR_CONST reduce modulo exhaustive group order 2140da9 Add secp256k1_scalar_half for halving scalars (+ tests/benchmarks). 1f1bb78 Merge bitcoin-core/secp256k1#1430: README: remove CI badge 5dab0ba README: remove CI badge b314cf2 Merge bitcoin-core/secp256k1#1426: ci/cirrus: Add native ARM64 jobs fa4d6c7 ci/cirrus: Add native ARM64 persistent workers ee7aaf2 Merge bitcoin-core/secp256k1#1395: tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) ba9cb6f Merge bitcoin-core/secp256k1#1424: ci: Bump major versions for docker actions d9d80fd ci: Bump major versions for docker actions 4fd00f4 Merge bitcoin-core/secp256k1#1422: cmake: Install `libsecp256k1.pc` file 421d848 ci: Align Autotools/CMake `CI_INSTALL` directory names 9f005c6 cmake: Install `libsecp256k1.pc` file 2262d0e ci/cirrus: Bring back skeleton .cirrus.yml without jobs b10ddd2 Merge bitcoin-core/secp256k1#1416: doc: Align documented scripts with CI ones 49be5be Merge bitcoin-core/secp256k1#1390: tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID cbf3053 Merge bitcoin-core/secp256k1#1417: release cleanup: bump version after 0.4.0 9b118bc release cleanup: bump version after 0.4.0 7030364 tests: add CHECK_ERROR_VOID and use it in scratch tests f8d7ea6 tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID b0f7bfe doc: Do not mention soname in CHANGELOG.md "ABI Compatibility" section bd9d98d doc: Align documented scripts with CI ones a1d52e3 tests: remove unnecessary test in run_ec_pubkey_parse_test 875b0ad tests: remove unnecessary set_illegal_callback c45b7c4 refactor: introduce testutil.h (deduplicate `random_fe_`, `ge_equals_` helpers) dc55141 tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) e02f313 Add comment on length checks when parsing ECDSA sigs git-subtree-dir: src/secp256k1 git-subtree-split: efe85c70a2e357e3605a8901a9662295bae1001f
efe85c70a2 Merge bitcoin-core/secp256k1#1466: release cleanup: bump version after 0.4.1 4b2e06f460 release cleanup: bump version after 0.4.1 1ad5185 Merge bitcoin-core/secp256k1#1465: release: prepare for 0.4.1 672053d release: prepare for 0.4.1 1a81df8 Merge bitcoin-core/secp256k1#1380: Add ABI checking tool for release process 74a4d97 doc: Add ABI checking with `check-abi.sh` to the Release Process e7f830e Add `tools/check-abi.sh` 77af1da Merge bitcoin-core/secp256k1#1455: doc: improve secp256k1_fe_set_b32_mod doc 3928b7c doc: improve secp256k1_fe_set_b32_mod doc 5e9a4d7 Merge bitcoin-core/secp256k1#990: Add comment on length checks when parsing ECDSA sigs 4197d66 Merge bitcoin-core/secp256k1#1431: Add CONTRIBUTING.md 0e5ea62 CONTRIBUTING: add some coding and style conventions e2c9888 Merge bitcoin-core/secp256k1#1451: changelog: add entry for "field: Remove x86_64 asm" d2e36a2 changelog: add entry for "field: Remove x86_64 asm" 1a432cb README: update first sentence 0922a04 docs: move coverage report instructions to CONTRIBUTING 76880e4 Add CONTRIBUTING.md including scope and guidelines for new code d3e29db Merge bitcoin-core/secp256k1#1450: Add group.h ge/gej equality functions 04af0ba Replace ge_equals_ge[,j] calls with group.h equality calls 60525f6 Add unit tests for group.h equality functions a47cd97 Add group.h ge/gej equality functions 10e6d29 Merge bitcoin-core/secp256k1#1446: field: Remove x86_64 asm 07687e8 Merge bitcoin-core/secp256k1#1393: Implement new policy for VERIFY_CHECK and #ifdef VERIFY (issue bitcoin#1381) bb46723 remove VERIFY_SETUP define a3a3e11 remove unneeded VERIFY_SETUP uses in ECMULT_CONST_TABLE_GET_GE macro a0fb68a introduce and use SECP256K1_SCALAR_VERIFY macro cf25c86 introduce and use SECP256K1_{FE,GE,GEJ}_VERIFY macros 5d89bc0 remove superfluous `#ifdef VERIFY`/`#endif` preprocessor conditions c2688f8 redefine VERIFY_CHECK to empty in production (non-VERIFY) mode 5814d84 Merge bitcoin-core/secp256k1#1438: correct assertion for secp256k1_fe_mul_inner c1b4966 Merge bitcoin-core/secp256k1#1445: bench: add --help option to bench_internal f07cead build: Don't call assembly an optimization 2f0762f field: Remove x86_64 asm 1ddd76a bench: add --help option to bench_internal e721039 Merge bitcoin-core/secp256k1#1441: asm: add .note.GNU-stack section for non-exec stack ea47c82 Merge bitcoin-core/secp256k1#1442: Return temporaries to being unsigned in secp256k1_fe_sqr_inner dcdda31 Tighten secp256k1_fe_mul_inner's VERIFY_BITS checks 1027135 Return temporaries to being unsigned in secp256k1_fe_sqr_inner 33dc7e4 asm: add .note.GNU-stack section for non-exec stack c891c5c Merge bitcoin-core/secp256k1#1437: ci: Ignore internal errors of snapshot compilers 8185e72 ci: Ignore internal errors in snapshot compilers 40f50d0 Merge bitcoin-core/secp256k1#1184: Signed-digit based ecmult_const algorithm 8e2a5fe correct assertion for secp256k1_fe_mul_inner 355bbdf Add changelog entry for signed-digit ecmult_const algorithm 21f49d9 Remove unused secp256k1_scalar_shr_int 115fdc7 Remove unused secp256k1_wnaf_const aa9f3a3 ecmult_const: add/improve tests 4d16e90 Signed-digit based ecmult_const algorithm ba523be make SECP256K1_SCALAR_CONST reduce modulo exhaustive group order 2140da9 Add secp256k1_scalar_half for halving scalars (+ tests/benchmarks). 1f1bb78 Merge bitcoin-core/secp256k1#1430: README: remove CI badge 5dab0ba README: remove CI badge b314cf2 Merge bitcoin-core/secp256k1#1426: ci/cirrus: Add native ARM64 jobs fa4d6c7 ci/cirrus: Add native ARM64 persistent workers ee7aaf2 Merge bitcoin-core/secp256k1#1395: tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) ba9cb6f Merge bitcoin-core/secp256k1#1424: ci: Bump major versions for docker actions d9d80fd ci: Bump major versions for docker actions 4fd00f4 Merge bitcoin-core/secp256k1#1422: cmake: Install `libsecp256k1.pc` file 421d848 ci: Align Autotools/CMake `CI_INSTALL` directory names 9f005c6 cmake: Install `libsecp256k1.pc` file 2262d0e ci/cirrus: Bring back skeleton .cirrus.yml without jobs b10ddd2 Merge bitcoin-core/secp256k1#1416: doc: Align documented scripts with CI ones 49be5be Merge bitcoin-core/secp256k1#1390: tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID cbf3053 Merge bitcoin-core/secp256k1#1417: release cleanup: bump version after 0.4.0 9b118bc release cleanup: bump version after 0.4.0 7030364 tests: add CHECK_ERROR_VOID and use it in scratch tests f8d7ea6 tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID b0f7bfe doc: Do not mention soname in CHANGELOG.md "ABI Compatibility" section bd9d98d doc: Align documented scripts with CI ones a1d52e3 tests: remove unnecessary test in run_ec_pubkey_parse_test 875b0ad tests: remove unnecessary set_illegal_callback c45b7c4 refactor: introduce testutil.h (deduplicate `random_fe_`, `ge_equals_` helpers) dc55141 tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) e02f313 Add comment on length checks when parsing ECDSA sigs git-subtree-dir: src/secp256k1 git-subtree-split: efe85c70a2e357e3605a8901a9662295bae1001f
efe85c70a2 Merge bitcoin-core/secp256k1#1466: release cleanup: bump version after 0.4.1 4b2e06f460 release cleanup: bump version after 0.4.1 1ad5185cd4 Merge bitcoin-core/secp256k1#1465: release: prepare for 0.4.1 672053d801 release: prepare for 0.4.1 1a81df826e Merge bitcoin-core/secp256k1#1380: Add ABI checking tool for release process 74a4d974d5 doc: Add ABI checking with `check-abi.sh` to the Release Process e7f830e32c Add `tools/check-abi.sh` 77af1da9f6 Merge bitcoin-core/secp256k1#1455: doc: improve secp256k1_fe_set_b32_mod doc 3928b7c383 doc: improve secp256k1_fe_set_b32_mod doc 5e9a4d7aec Merge bitcoin-core/secp256k1#990: Add comment on length checks when parsing ECDSA sigs 4197d667ec Merge bitcoin-core/secp256k1#1431: Add CONTRIBUTING.md 0e5ea62207 CONTRIBUTING: add some coding and style conventions e2c9888eee Merge bitcoin-core/secp256k1#1451: changelog: add entry for "field: Remove x86_64 asm" d2e36a2b81 changelog: add entry for "field: Remove x86_64 asm" 1a432cb982 README: update first sentence 0922a047fb docs: move coverage report instructions to CONTRIBUTING 76880e4015 Add CONTRIBUTING.md including scope and guidelines for new code d3e29db8bb Merge bitcoin-core/secp256k1#1450: Add group.h ge/gej equality functions 04af0ba162 Replace ge_equals_ge[,j] calls with group.h equality calls 60525f6c14 Add unit tests for group.h equality functions a47cd97d51 Add group.h ge/gej equality functions 10e6d29b60 Merge bitcoin-core/secp256k1#1446: field: Remove x86_64 asm 07687e811d Merge bitcoin-core/secp256k1#1393: Implement new policy for VERIFY_CHECK and #ifdef VERIFY (issue #1381) bb4672342e remove VERIFY_SETUP define a3a3e11acd remove unneeded VERIFY_SETUP uses in ECMULT_CONST_TABLE_GET_GE macro a0fb68a2e7 introduce and use SECP256K1_SCALAR_VERIFY macro cf25c86d05 introduce and use SECP256K1_{FE,GE,GEJ}_VERIFY macros 5d89bc031b remove superfluous `#ifdef VERIFY`/`#endif` preprocessor conditions c2688f8de9 redefine VERIFY_CHECK to empty in production (non-VERIFY) mode 5814d8485c Merge bitcoin-core/secp256k1#1438: correct assertion for secp256k1_fe_mul_inner c1b4966410 Merge bitcoin-core/secp256k1#1445: bench: add --help option to bench_internal f07cead0ca build: Don't call assembly an optimization 2f0762fa8f field: Remove x86_64 asm 1ddd76af0a bench: add --help option to bench_internal e72103932d Merge bitcoin-core/secp256k1#1441: asm: add .note.GNU-stack section for non-exec stack ea47c82e01 Merge bitcoin-core/secp256k1#1442: Return temporaries to being unsigned in secp256k1_fe_sqr_inner dcdda31f2c Tighten secp256k1_fe_mul_inner's VERIFY_BITS checks 10271356c8 Return temporaries to being unsigned in secp256k1_fe_sqr_inner 33dc7e4d3e asm: add .note.GNU-stack section for non-exec stack c891c5c2f4 Merge bitcoin-core/secp256k1#1437: ci: Ignore internal errors of snapshot compilers 8185e72d29 ci: Ignore internal errors in snapshot compilers 40f50d0fbd Merge bitcoin-core/secp256k1#1184: Signed-digit based ecmult_const algorithm 8e2a5fe908 correct assertion for secp256k1_fe_mul_inner 355bbdf38a Add changelog entry for signed-digit ecmult_const algorithm 21f49d9bec Remove unused secp256k1_scalar_shr_int 115fdc7232 Remove unused secp256k1_wnaf_const aa9f3a3c00 ecmult_const: add/improve tests 4d16e90111 Signed-digit based ecmult_const algorithm ba523be067 make SECP256K1_SCALAR_CONST reduce modulo exhaustive group order 2140da9cd5 Add secp256k1_scalar_half for halving scalars (+ tests/benchmarks). 1f1bb78b7f Merge bitcoin-core/secp256k1#1430: README: remove CI badge 5dab0baa80 README: remove CI badge b314cf2833 Merge bitcoin-core/secp256k1#1426: ci/cirrus: Add native ARM64 jobs fa4d6c76b6 ci/cirrus: Add native ARM64 persistent workers ee7aaf213e Merge bitcoin-core/secp256k1#1395: tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) ba9cb6f378 Merge bitcoin-core/secp256k1#1424: ci: Bump major versions for docker actions d9d80fd155 ci: Bump major versions for docker actions 4fd00f4bfe Merge bitcoin-core/secp256k1#1422: cmake: Install `libsecp256k1.pc` file 421d84855a ci: Align Autotools/CMake `CI_INSTALL` directory names 9f005c60d6 cmake: Install `libsecp256k1.pc` file 2262d0eaab ci/cirrus: Bring back skeleton .cirrus.yml without jobs b10ddd2bd2 Merge bitcoin-core/secp256k1#1416: doc: Align documented scripts with CI ones 49be5be9e8 Merge bitcoin-core/secp256k1#1390: tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID cbf3053ff1 Merge bitcoin-core/secp256k1#1417: release cleanup: bump version after 0.4.0 9b118bc7fb release cleanup: bump version after 0.4.0 70303643cf tests: add CHECK_ERROR_VOID and use it in scratch tests f8d7ea68df tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID b0f7bfedc9 doc: Do not mention soname in CHANGELOG.md "ABI Compatibility" section bd9d98d353 doc: Align documented scripts with CI ones a1d52e3e12 tests: remove unnecessary test in run_ec_pubkey_parse_test 875b0ada25 tests: remove unnecessary set_illegal_callback c45b7c4fbb refactor: introduce testutil.h (deduplicate `random_fe_`, `ge_equals_` helpers) dc5514144f tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) e02f313b1f Add comment on length checks when parsing ECDSA sigs git-subtree-dir: src/secp256k1 git-subtree-split: efe85c70a2e357e3605a8901a9662295bae1001f
e3a885d Merge bitcoin-core/secp256k1#1522: release: prepare for 0.5.0 c0e4ec3 release: prepare for 0.5.0 bb528cf Merge bitcoin-core/secp256k1#1518: Add secp256k1_pubkey_sort 7d2591c Add secp256k1_pubkey_sort da51507 Merge bitcoin-core/secp256k1#1058: Signed-digit multi-comb ecmult_gen algorithm 4c341f8 Add changelog entry for SDMC a043940 Permit COMB_BITS < 256 for exhaustive tests 39b2f2a Add test case for ecmult_gen recoded = {-1,0,1} 644e86d Reintroduce projective blinding 07810d9 Reduce side channels from single-bit reads a0d32b5 Optimization: use Nx32 representation for recoded bits e03dcc4 Make secp256k1_scalar_get_bits support 32-bit reads 5005abe Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t 6247f48 Optimization: avoid unnecessary doublings in precomputation 15d0cca Optimization: first table lookup needs no point addition 7a33db3 Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset ed2a056 Provide 3 configurations accessible through ./configure 5f7be9f Always generate tables for current (blocks,teeth) config fde1dfc Signed-digit multi-comb ecmult_gen algorithm 486518b Make exhaustive tests's scalar_inverse(&x,&x) work ab45c3e Initial gej blinding -> final ge blinding aa00a6b Introduce CEIL_DIV macro and use it d831168 Merge bitcoin-core/secp256k1#1515: ci: Note affected clangs in comment on ASLR quirk a85e223 ci: Note affected clangs in comment on ASLR quirk 4b77fec Merge bitcoin-core/secp256k1#1512: msan: notate more variable assignments from assembly code f7f0184 msan: notate more variable assignments from assembly code a613391 change inconsistent array param to pointer 05bfab6 Merge bitcoin-core/secp256k1#1507: ci: Add workaround for ASLR bug in sanitizers a5e8ab2 ci: Add sanitizer env variables to debug output 84a93de ci: Add workaround for ASLR bug in sanitizers 427e86b Merge bitcoin-core/secp256k1#1490: tests: improve fe_sqr test (issue #1472) 2028069 doc: clarify input requirements for secp256k1_fe_mul 11420a7 tests: improve fe_sqr test cdc9a62 Merge bitcoin-core/secp256k1#1489: tests: add missing fe comparison checks for inverse field test cases d926510 Merge bitcoin-core/secp256k1#1496: msan: notate variable assignments from assembly code 31ba404 msan: notate variable assignments from assembly code e7ea32e msan: Add SECP256K1_CHECKMEM_MSAN_DEFINE which applies to memory sanitizer and not valgrind e7bdddd refactor: rename `check_fe_equal` -> `fe_equal` 00111c9 tests: add missing fe comparison checks for inverse field test cases 0653a25 Merge bitcoin-core/secp256k1#1486: ci: Update cache action 94a14d5 ci: Update cache action 2483627 Merge bitcoin-core/secp256k1#1483: cmake: Recommend native CMake commands in README 5ad3aa3 Merge bitcoin-core/secp256k1#1484: tests: Drop redundant _scalar_check_overflow calls 51df2d9 tests: Drop redundant _scalar_check_overflow calls 3777e3f cmake: Recommend native CMake commands in README e4af41c Merge bitcoin-core/secp256k1#1249: cmake: Add `SECP256K1_LATE_CFLAGS` configure option 3bf4d68 Merge bitcoin-core/secp256k1#1482: build: Clean up handling of module dependencies e682267 build: Error if required module explicitly off 89ec583 build: Clean up handling of module dependencies 4437886 Merge bitcoin-core/secp256k1#1468: v0.4.1 release aftermath a9db9f2 Merge bitcoin-core/secp256k1#1480: Get rid of untested sizeof(secp256k1_ge_storage) == 64 code path 74b7c3b Merge bitcoin-core/secp256k1#1476: include: make docs more consistent b37fdb2 check-abi: Minor UI improvements ad5f589 check-abi: Default to HEAD for new version 9fb7e2f release process: Style and formatting nits ba5d72d assumptions: Use new STATIC_ASSERT macro e53c2d9 Require that sizeof(secp256k1_ge_storage) == 64 d0ba2ab util: Add STATIC_ASSERT macro da7bc1b include: in doc, remove article in front of "pointer" aa3dd52 include: make doc about ctx more consistent e3f6900 include: remove obvious "cannot be NULL" doc d373bf6 Merge bitcoin-core/secp256k1#1474: tests: restore scalar_mul test 79e0945 Merge bitcoin-core/secp256k1#1473: Fix typos 3dbfb48 tests: restore scalar_mul test d77170a Fix typos e7053d0 release process: Add email step 429d21d release process: Run sanity checks on release PR efe85c7 Merge bitcoin-core/secp256k1#1466: release cleanup: bump version after 0.4.1 4b2e06f release cleanup: bump version after 0.4.1 1ad5185 Merge bitcoin-core/secp256k1#1465: release: prepare for 0.4.1 672053d release: prepare for 0.4.1 1a81df8 Merge bitcoin-core/secp256k1#1380: Add ABI checking tool for release process 74a4d97 doc: Add ABI checking with `check-abi.sh` to the Release Process e7f830e Add `tools/check-abi.sh` 77af1da Merge bitcoin-core/secp256k1#1455: doc: improve secp256k1_fe_set_b32_mod doc 3928b7c doc: improve secp256k1_fe_set_b32_mod doc 5e9a4d7 Merge bitcoin-core/secp256k1#990: Add comment on length checks when parsing ECDSA sigs 4197d66 Merge bitcoin-core/secp256k1#1431: Add CONTRIBUTING.md 0e5ea62 CONTRIBUTING: add some coding and style conventions e2c9888 Merge bitcoin-core/secp256k1#1451: changelog: add entry for "field: Remove x86_64 asm" d2e36a2 changelog: add entry for "field: Remove x86_64 asm" 1a432cb README: update first sentence 0922a04 docs: move coverage report instructions to CONTRIBUTING 76880e4 Add CONTRIBUTING.md including scope and guidelines for new code d3e29db Merge bitcoin-core/secp256k1#1450: Add group.h ge/gej equality functions 04af0ba Replace ge_equals_ge[,j] calls with group.h equality calls 60525f6 Add unit tests for group.h equality functions a47cd97 Add group.h ge/gej equality functions 10e6d29 Merge bitcoin-core/secp256k1#1446: field: Remove x86_64 asm 07687e8 Merge bitcoin-core/secp256k1#1393: Implement new policy for VERIFY_CHECK and #ifdef VERIFY (issue #1381) bb46723 remove VERIFY_SETUP define a3a3e11 remove unneeded VERIFY_SETUP uses in ECMULT_CONST_TABLE_GET_GE macro a0fb68a introduce and use SECP256K1_SCALAR_VERIFY macro cf25c86 introduce and use SECP256K1_{FE,GE,GEJ}_VERIFY macros 5d89bc0 remove superfluous `#ifdef VERIFY`/`#endif` preprocessor conditions c2688f8 redefine VERIFY_CHECK to empty in production (non-VERIFY) mode 5814d84 Merge bitcoin-core/secp256k1#1438: correct assertion for secp256k1_fe_mul_inner c1b4966 Merge bitcoin-core/secp256k1#1445: bench: add --help option to bench_internal f07cead build: Don't call assembly an optimization 2f0762f field: Remove x86_64 asm 1ddd76a bench: add --help option to bench_internal e721039 Merge bitcoin-core/secp256k1#1441: asm: add .note.GNU-stack section for non-exec stack ea47c82 Merge bitcoin-core/secp256k1#1442: Return temporaries to being unsigned in secp256k1_fe_sqr_inner dcdda31 Tighten secp256k1_fe_mul_inner's VERIFY_BITS checks 1027135 Return temporaries to being unsigned in secp256k1_fe_sqr_inner 33dc7e4 asm: add .note.GNU-stack section for non-exec stack c891c5c Merge bitcoin-core/secp256k1#1437: ci: Ignore internal errors of snapshot compilers 8185e72 ci: Ignore internal errors in snapshot compilers 40f50d0 Merge bitcoin-core/secp256k1#1184: Signed-digit based ecmult_const algorithm 8e2a5fe correct assertion for secp256k1_fe_mul_inner 355bbdf Add changelog entry for signed-digit ecmult_const algorithm 21f49d9 Remove unused secp256k1_scalar_shr_int 115fdc7 Remove unused secp256k1_wnaf_const aa9f3a3 ecmult_const: add/improve tests 4d16e90 Signed-digit based ecmult_const algorithm ba523be make SECP256K1_SCALAR_CONST reduce modulo exhaustive group order 2140da9 Add secp256k1_scalar_half for halving scalars (+ tests/benchmarks). 1f1bb78 Merge bitcoin-core/secp256k1#1430: README: remove CI badge 5dab0ba README: remove CI badge b314cf2 Merge bitcoin-core/secp256k1#1426: ci/cirrus: Add native ARM64 jobs fa4d6c7 ci/cirrus: Add native ARM64 persistent workers ee7aaf2 Merge bitcoin-core/secp256k1#1395: tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) ba9cb6f Merge bitcoin-core/secp256k1#1424: ci: Bump major versions for docker actions d9d80fd ci: Bump major versions for docker actions 4fd00f4 Merge bitcoin-core/secp256k1#1422: cmake: Install `libsecp256k1.pc` file 421d848 ci: Align Autotools/CMake `CI_INSTALL` directory names 9f005c6 cmake: Install `libsecp256k1.pc` file 2262d0e ci/cirrus: Bring back skeleton .cirrus.yml without jobs b10ddd2 Merge bitcoin-core/secp256k1#1416: doc: Align documented scripts with CI ones 49be5be Merge bitcoin-core/secp256k1#1390: tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID cbf3053 Merge bitcoin-core/secp256k1#1417: release cleanup: bump version after 0.4.0 9b118bc release cleanup: bump version after 0.4.0 199d27c Merge bitcoin-core/secp256k1#1415: release: Prepare for 0.4.0 7030364 tests: add CHECK_ERROR_VOID and use it in scratch tests f8d7ea6 tests: Replace counting_illegal_callbacks with CHECK_ILLEGAL_VOID 1633980 release: Prepare for 0.4.0 d9a8506 changelog: Catch up in preparation of release b0f7bfe doc: Do not mention soname in CHANGELOG.md "ABI Compatibility" section bd9d98d doc: Align documented scripts with CI ones 0b4640a Merge bitcoin-core/secp256k1#1413: ci: Add `release` job 8659a01 ci: Add `release` job f9b3889 ci: Update `actions/checkout` version a1d52e3 tests: remove unnecessary test in run_ec_pubkey_parse_test 875b0ad tests: remove unnecessary set_illegal_callback 727bec5 Merge bitcoin-core/secp256k1#1414: ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot 2635068 ci/gha: Let MSan continue checking after errors in all jobs e78c7b6 ci/Dockerfile: Reduce size of Docker image further 2f0d3bb ci/Dockerfile: Warn if `ulimit -n` is too high when running Docker 4b8a647 ci/gha: Add ARM64 QEMU jobs for clang and clang-snapshot 6ebe7d2 ci/Dockerfile: Always use versioned clang packages 65c79fe Merge bitcoin-core/secp256k1#1412: ci: Switch macOS from Ventura to Monterey and add Valgrind c223d7e ci: Switch macOS from Ventura to Monterey and add Valgrind ea26b71 Merge bitcoin-core/secp256k1#1411: ci: Make repetitive command the default one cce0456 ci: Make repetitive command the default one 317a4c4 ci: Move `git config ...` to `run-in-docker-action` 4d7fe60 Merge bitcoin-core/secp256k1#1409: ci: Move remained task from Cirrus to GitHub Actions 676ed8f ci: Move "C++ (public headers)" from Cirrus to GitHub Actions 61fc3a2 ci: Move "C++ -fpermissive..." from Cirrus to GitHub Actions d51fb0a ci: Move "MSan" from Cirrus to GitHub Actions c22ac27 ci: Move sanitizers task from Cirrus to GitHub Actions 26a9899 Merge bitcoin-core/secp256k1#1410: ci: Use concurrency for pull requests only ee1be62 ci: Use concurrency for pull requests only 6ee1455 Merge bitcoin-core/secp256k1#1406: ci, gha: Move more non-x86_64 tasks from Cirrus CI to GitHub Actions fc3dea2 ci: Move "ppc64le: Linux..." from Cirrus to GitHub Actions 7782dc8 ci: Move "ARM64: Linux..." from Cirrus to GitHub Actions 0a16de6 ci: Move "ARM32: Linux..." from Cirrus to GitHub Actions ea33914 ci: Move "s390x (big-endian): Linux..." from Cirrus to GitHub Actions 880be8a ci: Move "i686: Linux (Debian stable)" from Cirrus to GiHub Actions 2e6cf9b Merge bitcoin-core/secp256k1#1396: ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job 5373693 Merge bitcoin-core/secp256k1#1405: ci: Drop no longer needed workaround ef9fe95 ci: Drop no longer needed workaround e10878f ci, gha: Drop `driver-opts.network` input for `setup-buildx-action` 4ad4914 ci, gha: Add `retry_builder` Docker image builder 6617a62 ci: Remove "x86_64: Linux (Debian stable)" task from Cirrus CI 03c9e65 ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job ad3e65d ci: Remove GCC build files and sage to reduce size of Docker image 6b9507a Merge bitcoin-core/secp256k1#1398: ci, gha: Add Windows jobs based on Linux image 87d35f3 ci: Rename `cirrus.sh` to more general `ci.sh` d6281dd ci: Remove Windows tasks from Cirrus CI 2b6f9cd ci, gha: Add Windows jobs based on Linux image 48b1d93 Merge bitcoin-core/secp256k1#1403: ci, gha: Ensure only a single workflow processes `github.ref` at a time 0ba2b94 Merge bitcoin-core/secp256k1#1373: Add invariant checking for scalars c45b7c4 refactor: introduce testutil.h (deduplicate `random_fe_`, `ge_equals_` helpers) dc55141 tests: simplify `random_fe_non_zero` (remove loop limit and unneeded normalize) 060e32c Merge bitcoin-core/secp256k1#1401: ci, gha: Run all MSVC tests on Windows natively de657c2 Merge bitcoin-core/secp256k1#1062: Removes `_fe_equal_var`, and unwanted `_fe_normalize_weak` calls (in tests) bcffeb1 Merge bitcoin-core/secp256k1#1404: ci: Remove "arm64: macOS Ventura" task from Cirrus CI c2f6435 ci: Add comment about switching macOS to M1 on GHA later 4a24fae ci: Remove "arm64: macOS Ventura" task from Cirrus CI b0886fd ci, gha: Ensure only a single workflow processes `github.ref` at a time 3d05c86 Merge bitcoin-core/secp256k1#1394: ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions d78bec7 ci: Remove Windows MSVC tasks from Cirrus CI 3545dc2 ci, gha: Run all MSVC tests on Windows natively 5d8fa82 Merge bitcoin-core/secp256k1#1274: test: Silent noisy clang warnings about Valgrind code on macOS x86_64 8e54a34 ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions b327abf Merge bitcoin-core/secp256k1#1402: ci: Use Homebrew's gcc in native macOS task d62db57 ci: Use Homebrew's gcc in native macOS task 54058d1 field: remove `secp256k1_fe_equal_var` bb4efd6 tests: remove unwanted `secp256k1_fe_normalize_weak` call eedd781 Merge bitcoin-core/secp256k1#1348: tighten group magnitude limits, save normalize_weak calls in group add methods (revival of #1032) b2f6712 Merge bitcoin-core/secp256k1#1400: ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift 9c91ea4 ci: Enable ellswift module where it's missing db32a24 ctimetests: Use new SECP256K1_CHECKMEM macros also for ellswift ce765a5 Merge bitcoin-core/secp256k1#1399: ci, gha: Run "SageMath prover" job on GitHub Actions 8408dfd Revert "ci: Run sage prover on CI" c8d9914 ci, gha: Run "SageMath prover" job on GitHub Actions 8d2960c Merge bitcoin-core/secp256k1#1397: ci: Remove "Windows (VS 2022)" task from Cirrus CI f1774e5 ci, gha: Make MSVC job presentation more explicit 5ee039b ci: Remove "Windows (VS 2022)" task from Cirrus CI 96294c0 Merge bitcoin-core/secp256k1#1389: ci: Run "Windows (VS 2022)" job on GitHub Actions a2f7ccd ci: Run "Windows (VS 2022)" job on GitHub Actions 374e2b5 Merge bitcoin-core/secp256k1#1290: cmake: Set `ENVIRONMENT` property for examples on Windows 1b13415 Merge bitcoin-core/secp256k1#1391: refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2) a1bd497 refactor: take use of `secp256k1_scalar_{zero,one}` constants (part 2) b7c685e Save _normalize_weak calls in group add methods c83afa6 Tighten group magnitude limits 26392da Merge bitcoin-core/secp256k1#1386: ci: print $ELLSWIFT in cirrus.sh d23da6d use secp256k1_scalar_verify checks 4692478 ci: print $ELLSWIFT in cirrus.sh c7d0454 add verification for scalars c734c64 Merge bitcoin-core/secp256k1#1384: build: enable ellswift module via SECP_CONFIG_DEFINES ad15215 update max scalar in scalar_cmov_test and fix schnorrsig_verify exhaustive test 78ca880 build: enable ellswift module via SECP_CONFIG_DEFINES 0e00fc7 Merge bitcoin-core/secp256k1#1383: util: remove unused checked_realloc b097a46 util: remove unused checked_realloc 2bd5f3e Merge bitcoin-core/secp256k1#1382: refactor: Drop unused cast 4f8c5bd refactor: Drop unused cast 173e8d0 Implement current magnitude assumptions 49afd2f Take use of _fe_verify_magnitude in field_impl.h 4e9661f Add _fe_verify_magnitude (no-op unless VERIFY is enabled) 690b0fc add missing group element invariant checks 175db31 ci: Drop no longer needed `PATH` variable update on Windows 116d2ab cmake: Set `ENVIRONMENT` property for examples on Windows cef3739 cmake, refactor: Use helper function instead of interface library 747ada3 test: Silent noisy clang warnings about Valgrind code on macOS x86_64 42f8c51 cmake: Add `SECP256K1_LATE_CFLAGS` configure option e02f313 Add comment on length checks when parsing ECDSA sigs git-subtree-dir: src/secp256k1 git-subtree-split: e3a885d42a7800c1ccebad94ad1e2b82c4df5c65
Using some insights learned from #1058, this replaces the fixed-wnaf ecmult_const algorithm with a signed-digit based one. Conceptually both algorithms are very similar, in that they boil down to summing precomputed odd multiples of the input points. Practically however, the new algorithm is simpler because it's just using scalar operations, rather than relying on wnaf machinery with skew terms to guarantee odd multipliers.
The idea is that we can compute$q \cdot A$ as follows:
secp256k1_scalar_lambda_split
.The comments in$f(q) = (q + (1+\lambda)(2^n - 2^{129} - 1))/2 \mod n$ , the result will equal $q \cdot A$ .
ecmult_const_impl.h
show that ifThis last step can be performed in groups of multiple bits at once, by looking up entries in a precomputed table of odd multiples of$A$ and $\lambda A$ , and then multiplying by a power of two before proceeding to the next group.
The result is slightly faster (I measure ~2% speedup), but significantly simpler as it only uses scalar arithmetic to determine the table lookup values. The speedup is due to the fact that no skew corrections at the end are needed, and less overhead to determine table indices. The precomputed table sizes are also made independent from the
ecmult
ones, after observing that the optimal table size is bigger here (which also gives a small speedup).