Upstream PRs 1174, 1154, 1178, 1177, 1171, 1158, 1183, 1185, 1186, 1188, 1187 #246
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
…r 0.2.0 02ebc29 release cleanup: bump version after 0.2.0 (Jonas Nick) b6b360e doc: improve message of cleanup commit (Jonas Nick) Pull request description: ACKs for top commit: sipa: ACK 02ebc29 Tree-SHA512: b887e31a531f7d21025558ed0a64ff5f68dee6feff8288478f7eb023189ceb20e5ca8baf0434ebd2ee49488d35d7aebc1b837888ff8c6e6420e6b86cc2f99cb1
This change eases the use of alternate build systems by moving the variables in `src/libsecp256k1-config.h` to compiler macros for each invocation, preventing duplication of these variables for each build system. Co-authored-by: Ali Sherief <[email protected]>
…nset variables as an error 7a74688 ci: add missing CFLAGS & CPPFLAGS variable to print_environment (Jonas Nick) c2e0fda ci: set -u in cirrus.sh to treat unset variables as an error (Jonas Nick) Pull request description: This PR is supposed to prevent accidental misuse of cirrus.sh. Maybe there is a way to check if `CC`, `AR` and `NM` are set within the loop that deals with the other variables, but so far I did not come up with one (that's POSIX shell compliant). ACKs for top commit: real-or-random: ACK 7a74688 hebasto: re-ACK 7a74688 Tree-SHA512: 91e42b3f1192fbf86e6fb43942713e78b2bee977ddd95256ea7448f84324369399d31ec4eedd47af595bf994bbc9396e26bb5c93bdb7f58c4310b5d3d5d66731
9c5a4d2 Do not define unused `HAVE_VALGRIND` macro (Hennadii Stepanov) ad8647f Drop no longer relevant files from `.gitignore` (Hennadii Stepanov) b627ba7 Remove dependency on `src/libsecp256k1-config.h` (Hennadii Stepanov) Pull request description: Cherry-picked the first commit from #1142 and addressed a [comment](bitcoin-core/secp256k1#1142 (comment)). ACKs for top commit: sipa: utACK 9c5a4d2 real-or-random: utACK 9c5a4d2 Tree-SHA512: c6f268261fc5edee855a7e69fdf9f6c5f4b859eb1e078e3c44c3ee4c9c445738af3de9fc2fbcca90db9b9e38681da8217faaeb0735201052b16ea397a7817db9
c30b889 Clarify that the ABI-incompatible versions are earlier (Pieter Wuille) 881fc33 Consistency in naming of modules (Pieter Wuille) 9ecf814 Reduce font size in changelog (Pieter Wuille) 2dc133a Add more changelog entries (Pieter Wuille) ac233e1 Add links to diffs to changelog (Pieter Wuille) cee8223 Mention semantic versioning in changelog (Pieter Wuille) Pull request description: ACKs for top commit: real-or-random: ACK c30b889 jonasnick: ACK c30b889 Tree-SHA512: 0f753eae0ea4d65035bfbcd81b90169111ea030cf7196dd072fb1ccc8aac1437768031f3fcef431584028da68b66873204e16e03bcde4a6ae96b08ab7f97a480
…CHECK_VOID which returns (void) a49e094 docs: Fix typo (Tim Ruffing) 2551cda tests: Fix code formatting (Tim Ruffing) c635c1b Change ARG_CHECK_NO_RETURN to ARG_CHECK_VOID which returns (void) (Tim Ruffing) cf66f23 refactor: Add helper function secp256k1_context_is_proper() (Tim Ruffing) Pull request description: ACKs for top commit: sipa: utACK a49e094 jonasnick: ACK a49e094 Tree-SHA512: 0fd4ee88510f2de0de96378ae69ce6e610a446000bb78597026c5924803e1ce5a4f76303fc6446233a6129f9c42dce1b1549f93bef935131101e47b5a69cdf2f
d216475 test secp256k1_i128_to_i64 (Russell O'Connor) 4bc4290 Add a secp256k1_i128_to_u64 function. (Russell O'Connor) Pull request description: I wanted to experiment with what would be required to split up `secp256k1_i128_to_i64` between those cases when a signed 64 bit value is being demoted, versus an unsigned 64 bit value is being extracted from the lower bits, and this is the result. I'm not sure this is a useful PR, so feel free to close it. However, since it is already written, I figured it is worth at least discussing. ACKs for top commit: sipa: utACK d216475 real-or-random: ACK d216475 Tree-SHA512: 41dbb1d33b3078bee8e71a838cfad6f1859c0bba602ae061259add8e9e8ea5aa482daa41de79dbd7433ddbef4a0bc52757f3c45d63acc9c0eb05aa3ca891b922
…o bench compilation c0a555b Bugfix: pass SECP_CONFIG_DEFINES to bench compilation (Pieter Wuille) Pull request description: ACKs for top commit: real-or-random: utACK c0a555b apoelstra: ACK c0a555b Tree-SHA512: 4ec6ca4c012166beb6c5bdd1b2ed939554415e03545c176cf281000145c4000a460e231d5da26f617a81b048cd0fa3f8f16b61a207aed9479fdd854483e35ded
User applications shouldn't need or rely on `SECP_CONFIG_DEFINES`.
…amples 2f9ca28 Drop `SECP_CONFIG_DEFINES` from examples (Hennadii Stepanov) Pull request description: User applications shouldn't need or rely on `SECP_CONFIG_DEFINES`. See bitcoin-core/secp256k1#1178 (comment). ACKs for top commit: sipa: utACK 2f9ca28 real-or-random: utACK 2f9ca28 Tree-SHA512: c8e81e6842b31e7f4ebcbb18d5962f7d7308f024025d6225330a7ec099739278bb43ad98243698c5802bcc49bf7e247ab7cae7f40008fbba87f0d0e46cbe1e85
39e8f0e refactor: Separate run_context_tests into static vs proper contexts (Tim Ruffing) a4a0937 tests: Clean up and improve run_context_tests() further (Tim Ruffing) fc90bb5 refactor: Tidy up main() (Tim Ruffing) f32a36f tests: Don't use global context for context tests (Tim Ruffing) ce4f936 tests: Tidy run_context_tests() by extracting functions (Tim Ruffing) 18e0db3 tests: Don't recreate global context in scratch space test (Tim Ruffing) b198061 tests: Use global copy of secp256k1_context_static instead of clone (Tim Ruffing) Pull request description: This is an improved version of some of the tidying/refactoring in #1170. I think it's enough to deserve a separate PR. Once this is merged, I'll get back to the actual goal of #1170 (namely, forbidding cloning and randomizing static contexts.) This PR is a general clean up of the context tests. A notable change is that this avoids a code smell where `run_context_tests()` would use the global `ctx` variable like a local one (i.e., create a context in it and destroy it afterwards). After this PR, the global `ctx` is properly initialized for all the other tests, and they can decide whether they want to use it or not. Same for a global `sttc`, which is a memcpy of the static context (we need a writable copy in order to be able to set callbacks). Note that this touches code which is also affected by #1167 but I refrained from trying to solve this issue. The goal of this PR is simply not to worsen the situation w.r.t. #1167. We should really introduce a macro to solve #1167 but that's another PR. ACKs for top commit: sipa: utACK 39e8f0e apoelstra: ACK 39e8f0e Tree-SHA512: a22471758111061a062b126a52a0de24a1a311d1a0332a4ef006882379a4f3f2b00e53089e3c374bf47c4051bb10bbc6a9fdbcf6d0cd4eca15b5703590395fba
… like tests but without VERIFY 2037600 tests: Add noverify_tests which is like tests but without VERIFY (Tim Ruffing) Pull request description: mentioned in bitcoin-core/secp256k1#1037 (comment) Let's see how this affects CI time ACKs for top commit: sipa: ACK 2037600 apoelstra: ACK 2037600 Tree-SHA512: fab1ce1499d418671d3d0ecfddf15d75b7c2bbfbfb4be958a95730491244185a906c7133aba4d0bec56ee6c721cb525750eef4cafc12f386484af931e34b0e8e
…in tests 9a93f48 refactor: Rename STTC to STATIC_CTX in tests (Tim Ruffing) 3385a26 refactor: Rename global variables to uppercase in tests (Tim Ruffing) Pull request description: On top of #1186 . I feel that this is an improvement, but it touches a lot of lines and so it deserves a separate discussion. ACKs for top commit: sipa: ACK 9a93f48 Tree-SHA512: b6dad2ffff2267034bf8cefdd3ef7ea11e9bcb8142d64b460ca61e0d3ab8de22fb3ee994dea0fb32feee3864d07395c070abffab318690d09d104294895300c4
| CHECK(ecount == 5); | ||
|
|
||
| secp256k1_context_destroy(sttc); |
There was a problem hiding this comment.
This should reset all the callbacks in the contexts (similar to run_ec_illegal_argument_tests in tests.c):
secp256k1_context_set_error_callback(CTX, NULL, NULL);
secp256k1_context_set_error_callback(STATIC_CTX, NULL, NULL);
secp256k1_context_set_illegal_callback(CTX, NULL, NULL);
secp256k1_context_set_illegal_callback(STATIC_CTX, NULL, NULL);
There was a problem hiding this comment.
I intentionally leftt this out because it's difficult to enforce and useless, but then I remembered bitcoin-core/secp256k1#1167.
There was a problem hiding this comment.
Weirdly, in the upstream code we often only reset STATIC_CTX, but not CTX (e.g. in schnorrsig and extrakeys).
There was a problem hiding this comment.
All the callbacks should be correctly reset now.
There was a problem hiding this comment.
Weirdly, in the upstream code we often only reset STATIC_CTX, but not CTX (e.g. in schnorrsig and extrakeys).
Hm, I don't think there's a deep reason for this. I assume these are simply code smells in the upstream tests. A solution of bitcoin-core/secp256k1#1167 will probably fix this on the way.
configure.ac
Outdated
There was a problem hiding this comment.
This should replace AC_DEFINE -> SECP_CONFIG_DEFINES also for the -zkp modules.
There was a problem hiding this comment.
Not sure if I updated AC_MSG_CHECKING([for __builtin_popcount]) and AC_MSG_CHECKING([for __builtin_clzll]) correctly. It results in "no" on my machine either way.
There was a problem hiding this comment.
Oh, that's a bug introduced in 79472c7 ... Linking fails because main is missing. I'll open a PR, but we should probably wait with merging other PRs until did the rebase here.
|
Hm, it would be nicer to make |
Probably best to do this inside EXPECT_ARG_CHECK_FAIL. |
jonasnick
force-pushed
the
temp-merge-1187
branch
from
c60d6da to
304fc88
Compare
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.
[bitcoin-core/secp256k1#1174]: release cleanup: bump version after 0.2.0
[bitcoin-core/secp256k1#1154]: ci: set -u in cirrus.sh to treat unset variables as an error
[bitcoin-core/secp256k1#1178]: Drop
src/libsecp256k1-config.h[bitcoin-core/secp256k1#1177]: Some improvements to the changelog
[bitcoin-core/secp256k1#1171]: Change ARG_CHECK_NO_RETURN to ARG_CHECK_VOID which returns (void)
[bitcoin-core/secp256k1#1158]: Add a secp256k1_i128_to_u64 function.
[bitcoin-core/secp256k1#1183]: Bugfix: pass SECP_CONFIG_DEFINES to bench compilation
[bitcoin-core/secp256k1#1185]: Drop
SECP_CONFIG_DEFINESfrom examples[bitcoin-core/secp256k1#1186]: tests: Tidy context tests
[bitcoin-core/secp256k1#1188]: tests: Add noverify_tests which is like tests but without VERIFY
[bitcoin-core/secp256k1#1187]: refactor: Rename global variables in tests
This PR can be recreated with
./contrib/sync-upstream.sh -b sync-upstream range cc3b8a4f.Tip: Use
git show --remerge-diffto show the changes manually added to the merge commit.