-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Update secp256k1 subtree to libsecp256k1 version 0.2.0 #26691
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
Conversation
21ffe4b Merge bitcoin-core/secp256k1#1055: Prepare initial release e025ccd release: prepare for initial release 0.2.0 6d1784a build: add missing files to EXTRA_DIST 8c949f5 Merge bitcoin-core/secp256k1#1173: Don't use compute credits for now 13bf1b6 changelog: make order of change types match keepachangelog.com b1f992a doc: improve release process 7e5b226 Don't use compute credits for now ad39e2d build: change package version to 0.1.0-dev 5c789dc Merge bitcoin-core/secp256k1#1168: Replace deprecated context flags with NONE in benchmarks and tests d6dc0f4 tests: Switch to NONE contexts in module tests 0c8a5ca tests: Switch to NONE contexts in tests.c 86540e9 tests: add test for deprecated flags and rm them from run_context caa0ad6 group: add gej_eq_var 37ba744 tests: Switch to NONE contexts in exhaustive and ctime tests 8d7a9a8 benchmarks: Switch to NONE contexts 90618e9 doc: move CHANGELOG from doc/ to root directory e3f8477 Merge bitcoin-core/secp256k1#1126: API cleanup with respect to contexts 4386a23 examples: Switch to NONE contexts 7289b51 docs: Use doxygen style if and only if comment is user-facing e7d0185 docs: Get rid of "initialized for signing" terminology 0612636 docs: Tidy and improve docs about contexts and randomization e02d686 selftest: Expose in public API e383fbf selftest: Rename internal function to make name available for API d2c6d48 tests: Use new name of static context 53796d2 contexts: Rename static context 72fedf8 docs: Improve docs for static context 316ac76 contexts: Deprecate all context flags except SECP256K1_CONTEXT_NONE 477f02c Merge bitcoin-core/secp256k1#1165: gitignore: Add *.sage.py files autogenerated by sage [skip ci] 092be61 gitignore: Add *.sage.py files autogenerated by sage 1a553ee docs: Change signature "validation" to "verification" ee7341f docs: Never require a verification context 751c435 Merge bitcoin-core/secp256k1#1152: Update macOS image for CI 2286f80 Merge bitcoin-core/secp256k1#993: Enable non-experimental modules by default e40fd27 Merge bitcoin-core/secp256k1#1156: Followups to int128_struct arithmetic 99bd335 Make int128 overflow test use secp256k1_[ui]128_mul a8494b0 Use compute credits for macOS jobs 3afce0a Avoid signed overflow in MSVC AMR64 secp256k1_mul128 c0ae48c Update macOS image for CI 9b5f589 Heuristically decide whether to use int128_struct 63ff064 int128: Add test override for testing __(u)mulh on MSVC X64 f2b7e88 Add int128 randomized tests 6138d73 Merge bitcoin-core/secp256k1#1155: Add MSan CI jobs ddf2b29 Merge bitcoin-core/secp256k1#1000: Synthetic int128 type. 86e3b38 Merge bitcoin-core/secp256k1#1149: Remove usage of CHECK from non-test file 00a42b9 Add MSan CI job 44916ae Merge bitcoin-core/secp256k1#1147: ci: print env to allow reproducing the job outside of CI c2ee917 Merge bitcoin-core/secp256k1#1146: ci: prevent "-v/--version: not found" irrelevant error e13fae4 Merge bitcoin-core/secp256k1#1150: ci: always cat test_env.log a340d95 ci: add int128_struct tests dceaa1f int128: Tidy #includes of int128.h and int128_impl.h 2914bcc Simulated int128 type. 6a965b6 Remove usage of CHECK from non-test file 5c9f1a5 ci: always cat all logs_snippets 49ae843 ci: mostly prevent "-v/--version: not found" irrelevant error 4e54c03 ci: print env to allow reproducing the job outside of CI a43e982 Merge bitcoin-core/secp256k1#1144: Cleanup `.gitignore` file f5039cb Cleanup `.gitignore` file 798727a Revert "Add test logs to gitignore" 41e8704 build: Enable some modules by default 694ce8f Merge bitcoin-core/secp256k1#1131: readme: Misc improvements 88b0089 readme: Fix line break 78f5296 readme: Sell "no runtime dependencies" ef48f08 readme: Add IRC channel 9f8a13d Merge bitcoin-core/secp256k1#1128: configure: Remove pkgconfig macros again (reintroduced by mismerge) cabe085 configure: Remove pkgconfig macros again (reintroduced by mismerge) 3efeb9d Merge bitcoin-core/secp256k1#1121: config: Set preprocessor defaults for ECMULT_* config values 6a873cc Merge bitcoin-core/secp256k1#1122: tests: Randomize the context with probability 15/16 instead of 1/4 17065f4 tests: Randomize the context with probability 15/16 instead of 1/4 c27ae45 config: Remove basic-config.h da6514a config: Introduce DEBUG_CONFIG macro for debug output of config 63a3565 Merge bitcoin-core/secp256k1#1120: ecmult_gen: Skip RNG when creating blinding if no seed is available d0cf55e config: Set preprocessor defaults for ECMULT_* config values 55f8bc9 ecmult_gen: Improve comments about projective blinding 7a86955 ecmult_gen: Simplify code (no observable change) 4cc0b1b ecmult_gen: Skip RNG when creating blinding if no seed is available af65d30 Merge bitcoin-core/secp256k1#1116: build: Fix #include "..." paths to get rid of further -I arguments 40a3473 build: Fix #include "..." paths to get rid of further -I arguments 43756da Merge bitcoin-core/secp256k1#1115: Fix sepc256k1 -> secp256k1 typo in group.h 069aba8 Fix sepc256k1 -> secp256k1 typo in group.h accadc9 Merge bitcoin-core/secp256k1#1114: `_scratch_destroy`: move `VERIFY_CHECK` after invalid scrach space check cd47033 Merge bitcoin-core/secp256k1#1084: ci: Add MSVC builds 1827c9b scratch_destroy: move VERIFY_CHECK after invalid scrach space check 49e2acd configure: Improve rationale for WERROR_CFLAGS 8dc4b03 ci: Add a C++ job that compiles the public headers without -fpermissive 51f296a ci: Run persistent wineserver to speed up wine 3fb3269 ci: Add 32-bit MinGW64 build 9efc2e5 ci: Add MSVC builds 2be6ba0 configure: Convince autotools to work with MSVC's archiver lib.exe bd81f41 schnorrsig bench: Suppress a stupid warning in MSVC 09f3d71 configure: Add a few CFLAGS for MSVC 3b4f3d0 build: Reject C++ compilers in the preprocessor 1cc0941 configure: Don't abort if the compiler does not define __STDC__ cca8cbb configure: Output message when checking for valgrind 1a6be57 bench: Make benchmarks compile on MSVC git-subtree-dir: src/secp256k1 git-subtree-split: 21ffe4b
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
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.
Concept ACK
The third commit says "Invoke secp256k1_selftest() during initialization." but that happens in a different commit. edit: by the way, if we can guarantee that the |
* Use SECP256K1_CONTEXT_NONE when creating signing context, as SECP256K1_CONTEXT_SIGN is deprecated and unnecessary. * Use secp256k1_static_context where applicable.
Fixed. |
I compiled 2022917 on a Linux and an Intel mac, compiled and successfully ran the tests. |
Anything left to do here? |
For other reviewers, including myself, this should be useful for testing a subtree update: https://github.com/bitcoin/bitcoin/blob/master/test/lint/README.md#git-subtree-checksh |
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.
commit message for 4462cb0
misnames secp256k1_context_static
as secp256k1_static_context
@@ -14,7 +14,7 @@ | |||
</ItemGroup> | |||
<ItemDefinitionGroup> | |||
<ClCompile> | |||
<PreprocessorDefinitions>ENABLE_MODULE_ECDH;ENABLE_MODULE_RECOVERY;ENABLE_MODULE_EXTRAKEYS;ENABLE_MODULE_SCHNORRSIG;%(PreprocessorDefinitions)</PreprocessorDefinitions> |
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.
was ecdh enabled for any reason?
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.
Bitcoin Core doesn't use the libsecp256k1 ecdh module, so it should be disabled. I don't know why it was enabled here before.
ACK 2022917 |
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 2022917
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
…on 0.2.0 notes: - excludes changes made to `SignSchnorr`, `XOnlyPubKey`, kernel context and absent {fuzz,bench} tests
backport: merge bitcoin#27479, bitcoin#27230, bitcoin#25251, partial bitcoin#22934, bitcoin#23383, bitcoin#24792, bitcoin#26691, bitcoin#27445 (secp256k1 update)
Now that libsecp256k1 has a release (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-December/021271.html), update the subtree to match it.
The changes themselves are not very impactful for Bitcoin Core, but include: