forked from bitcoin-core/secp256k1
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Merge 0.3.2 in FROST #24
Merged
Merged
Conversation
This file contains 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
Also remove the interface it was attached to since it's no longer needed. This removal simplifies the next commit.
This PR lints tests_wycheproof_generate.py according to pylint. This is a follow-up to PR bitcoin-core#1245. Co-authored-by: Sean Andersen <[email protected]>
…st{_xonly} a575339 Remove bits argument from secp256k1_wnaf_const (always 256) (Pieter Wuille) Pull request description: There is little reason for having the number of bits in the scalar as a parameter, as I don't think there are any (current) use cases for non-256-bit scalars. ACKs for top commit: jonasnick: ACK a575339 real-or-random: utACK a575339 Tree-SHA512: 994b1f19b4c513f6d070ed259a5d6f221a0c2450271ec824c5eba1cd0ecace276de391c170285bfeae96aaf8f1e0f7fe6260966ded0336c75c522ab6c56d182c
ef49a11 build: allow static or shared but not both (Cory Fields) 36b0adf build: remove warning until it's reproducible (Cory Fields) Pull request description: Continuing from here: bitcoin-core#1224 (comment) Unfortunately it wasn't really possible to keep a clean diff here because of the nature of the change. I suggest reviewing the lib creation stuff in its entirety, sorry about that :\ Rather than allowing for shared and static libs to be built at the same time like autotools, this PR switches to the CMake convention of allowing only 1. A new `BUILD_SHARED_LIBS` option is added to match CMake convention, as well as a `SECP256K1_DISABLE_SHARED` option which overrides it. That way even projects which have `BUILD_SHARED_LIBS=1` can opt-into a static libsecp in particular. Details: Two object libraries are created: `secp256k1_asm` and `secp256k1_precomputed_objs`. Some tests/benchmarks use the object libraries directly, some link against the real lib: `secp256k1`. Because the objs don't know what they're going to be linked into, they need to be told how to deal with PIC. The `DEFINE_SYMBOL` property sets the `DLL_EXPORT` define as necessary (when building a shared lib) ACKs for top commit: hebasto: re-ACK ef49a11, only [suggested](bitcoin-core#1230 (review)) changes since my recent [review](bitcoin-core#1230 (review)). real-or-random: ACK ef49a11 Tree-SHA512: 8870de305176fdb677caff0fdfc6f8c59c0e906489cb72bc9980e551002812685e59e20d731f2a82e33628bdfbb7261eafd6f228038cad3ec83bd74335959600
35ada3b tests: lint wycheproof's python script (RandomLattice) Pull request description: This PR lints tests_wycheproof_generate.py according to bitcoin's python linting scripts. This is a follow-up to PR bitcoin-core#1245. ACKs for top commit: sipa: utACK 35ada3b real-or-random: utACK 35ada3b Tree-SHA512: ea405060d2e73ff3543626687de5bc5282be923b914bd5c8c53e65df8dca9bea0000c416603095efff29bc7ae43c2081454c4e506db0f6805443d023fbffaf4c
…CPPFLAGS` 1ecb94e build: Make `SECP_VALGRIND_CHECK` preserve `CPPFLAGS` (Hennadii Stepanov) Pull request description: It was overlooked in bitcoin-core#862 and bitcoin-core#1027. ACKs for top commit: real-or-random: utACK 1ecb94e Tree-SHA512: 263fc600ce9743e4aad767150f706bf7d4325dabb9c363ed57f08fe38faea94d7d1999804947cffeacbe698bb6d959ee6de3f6e50400050a390ecc0db957e426
Useful for embedding secp256k1 in a subproject.
47ac3d6 cmake: Make installation optional (Anna “CyberTailor”) Pull request description: Useful for embedding secp256k1 in a subproject. ACKs for top commit: theuni: ACK 47ac3d6. real-or-random: utACK 47ac3d6 hebasto: ACK 47ac3d6, tested on Ubuntu 23.04. Tree-SHA512: 12ac0ba9dc38adf45684055386280b669384b5a4e528a3f6f4470fd0b7f57d64dfed6a8bb9f0a84cacfcb72f509534d71676c5ba37b27297b1a96676eea44e6e
Available in CMake 3.11+.
`DESCRIPTION` is available in CMake 3.9+. `HOMEPAGE_URL` is available in CMake 3.12+.
Available in CMake 3.12+.
Available in CMake 3.9+.
Available in CMake 3.3+.
… from `include/secp256k1.h` 8e142ca Move `SECP256K1_INLINE` macro definition out from `include/secp256k1.h` (Hennadii Stepanov) 7744589 Remove `SECP256K1_INLINE` usage from examples (Hennadii Stepanov) Pull request description: From [IRC](https://gnusha.org/secp256k1/2023-01-31.log): > 06:29 \< hebasto\> What are reasons to define the `SECP256K1_INLINE` macro in user's `include/secp256k1.h` header, while it is used internally only? > 06:32 \< hebasto\> I mean, any other (or a new dedicated) header in `src` looks more appropriate, no? > 06:35 \< sipa\> I think it may just predate any "utility" internal headers. > 06:42 \< sipa\> I think it makes sense to move it to util.h Pros: - it is a step in direction to better organized headers (in context of bitcoin-core#924, bitcoin-core#1039) Cons: - code duplication for `SECP256K1_GNUC_PREREQ` macro ACKs for top commit: sipa: utACK 8e142ca real-or-random: utACK bitcoin-core@8e142ca Tree-SHA512: 180e0ba7c2ef242b765f20698b67d06c492b7b70866c21db27c18d8b2e85c3e11f86c6cb99ffa88bbd23891ce3ee8a24bc528f2c91167ec2fddc167463f78eac
To use, invoke `cmake` with argument `--preset dev-mode`. Solves one item in bitcoin-core#1235. One disadvantage over `./configure --enable-dev-mode` is that CMake does not provide a way to "hide" presets from users. That is, `cmake --list-presets` will list dev-mode, and it will also appear in `cmake-gui`, even though it's not selectable there due to bug https://gitlab.kitware.com/cmake/cmake/-/issues/23341. (So in our case, that's probably rather a feature than a bug.) We curently use version 3 presets which require CMake 3.21+. Unfortunately, CMake versions before 3.19 may ignore the `--preset` argument silently. So if the preset is not picked up, make sure you have a recent enough CMake version. More unfortunately, we can't even spell this warning out in CMakePresets.json because CMake does not support officially support comments in JSON, see - https://gitlab.kitware.com/cmake/cmake/-/issues/21858 - https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5853 . We could use a hack hinted at in https://gitlab.kitware.com/cmake/cmake/-/issues/21858#note_908543 but that's risky, because it could simply break for future versions, and we probably want to use presets not only for dev mode.
This file is specifically intended for *local* CMake templates (as opposed to CMakePresets.json).
…ng variants 5b32602 Split fe_set_b32 into reducing and normalizing variants (Pieter Wuille) Pull request description: Follow-up to bitcoin-core#1205. This splits the `secp256k1_fe_set_b32` function into two variants: * `secp256k1_fe_set_b32_mod`, which returns `void`, reduces modulo the curve order, and only promises weakly normalized output. * `secp256k1_fe_set_b32_limit`, which returns `int` indicating success/failure, and only promises valid output in case the input is in range (but guarantees it's strongly normalized in this case). This removes one of the few cases in the codebase where normalization status depends on runtime values, making it fixed at compile-time instead. ACKs for top commit: real-or-random: ACK 5b32602 jonasnick: ACK 5b32602 Tree-SHA512: 4b93502272638c6ecdef4d74afa629e7ee540c0a20b377dccedbe567857b56c4684fad3af4b4293ed7ba35fed4aa5d0beaacdd77a903f44f24e8d87305919b61
In the existing code, the compiler is allowed to allocate the RSI register for outputs m0, m1, or m2, which are written to before the input in RSI is read from. Fix this by marking them as early clobber. Reported by ehoffman2 in bitcoin-core#766
In the field 5x52 asm for x86_64, stack variables are provided as outputs. The existing inputs are all forcibly allocated to registers, so cannot coincide, but mark them as early clobber anyway to make this clearer.
… really supported c6bb29b build: Rename `64bit` to `x86_64` (Hennadii Stepanov) 0324645 autotools: Add `SECP_ARM32_ASM_CHECK` macro (Hennadii Stepanov) ed4ba23 cmake: Add `check_arm32_assembly` function (Hennadii Stepanov) e5cf4bf build: Rename `arm` to `arm32` (Hennadii Stepanov) Pull request description: Closes bitcoin-core#1034. Solves one item in bitcoin-core#1235. ACKs for top commit: real-or-random: ACK c6bb29b tested on x86_64 but not on ARM Tree-SHA512: c3615a18cfa30bb2cc53be18c09ccab08fc800b84444d8c6b333347b4db039a3981da61e7da5086dd9f4472838d7c031d554be9ddc7c435ba906852bba593982
8c9ae37 Add release note (Pieter Wuille) 350b4bd Mark stack variables as early clobber for technical correctness (Pieter Wuille) 0c729ba Bugfix: mark outputs as early clobber in scalar x86_64 asm (Pieter Wuille) Pull request description: ACKs for top commit: real-or-random: ACK 8c9ae37 jonasnick: ACK 8c9ae37 Tree-SHA512: 874d01f5540d14b5188aec25f6441dbc6631f8d3980416040a3e250f1aef75150068415e7a458a9a3fb0d7cbdeb97f5c7e089b187d6d3dd79aa6e45274c241b6
This reverts commit 712e7f8.
3ad1027 Revert "Remove unused scratch space from API" (Jonas Nick) Pull request description: This reverts commit 712e7f8. Removing the scratch space from the API may break bindings to the library. ACKs for top commit: sipa: ACK 3ad1027 real-or-random: ACK 3ad1027 Tree-SHA512: ad394c0a2f83fe3a5f400c0e8f2b9bf40037ce4141d4414e6345918f5e6003c61da02a538425a49bdeb5700f5ecb713bd58f5752c0715fb1fcc4950099fdc0e6
697e1cc changelog: Catch up (Tim Ruffing) 76b43f3 changelog: Add entry for bitcoin-core#1303 (Tim Ruffing) Pull request description: ACKs for top commit: sipa: ACK 697e1cc jonasnick: ACK 697e1cc Tree-SHA512: cfeb513effc69925bdedd3a298b1e2e5bf7709f68b453a5f157c584560b5400c3dc8b9ce87a775281cdea9db7f44e7e1337fbc93563f6efe350fe5defacbc4f6
Closes #24
matteonardelli
force-pushed
the
frost-merge-0.3.2
branch
from
November 27, 2023 13:09
6afa091
to
c2cffbd
Compare
muxator
reviewed
Dec 6, 2023
muxator
reviewed
Dec 6, 2023
muxator
reviewed
Dec 6, 2023
Comment on lines
-65
to
+64
secp256k1_ecmult_const(result, &pt_ge, sc, ECMULT_CONST_256_BIT_SIZE); | ||
secp256k1_ecmult_const(result, &pt_ge, sc); |
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.
The removal of ECMULT_CONST_256_BIT_SIZE
is similar to what upstream has done in:
muxator
force-pushed
the
frost-merge-0.3.2
branch
from
December 6, 2023 10:23
0da717f
to
f2a1472
Compare
muxator
force-pushed
the
frost-merge-0.3.2
branch
from
December 6, 2023 10:45
f2a1472
to
61f39c3
Compare
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.