build: explicitly disable libsecp256k1 openssl based tests #23314
Conversation
These tests will fail if built against OpenSSL 3. They have been removed upstream, bitcoin-core/secp256k1#983, however rather than backport a subtree update, we can just disable the tests. See also: bitcoin#23314
|
No objection to updating to the current master branch. Do be aware that due to bitcoin-core/secp256k1#956 a table that was previously built at runtime, is now precomputed as part of the source code. That's 2.4 MB of C code being added to the repository, and 0.5 MiB added to the binary size. I don't expect that to be a problem, but if the binary size increase is an issue, it can be reduced with a configure flag (with a mild validation performance reduction). |
|
If you are going to backport a |
It wasn't going be to a backport, as I would rather make the subtree update here, but we can always include that commit here, and then do a subtree update after.
It's not clear how soon that bump would be, and it could well be too late for 23.0. |
|
Is there anything in the bump that we need, assuming the test issue is fixed with a configure flag? Especially, since you are planning another bump after bitcoin-core/secp256k1#988 (soon?). |
Same here.
I don't think so. |
|
Also, there was discussion about a 1.0 release of libsecp256k1, so I am wondering if we should wait for that as well |
These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048.
2f1c6a4 to
d752454
Compare
|
Ok. I've changed this to just disable the tests for now, as I would like to get this merged and backported into |
|
cr ACK d752454 Didn't test on Alpine or elsewhere. |
|
Code review ACK d752454 |
Concept ACK |
This would be a nice goal in order to create pressure on us to do a release. But my more pessimistic internal voice tells me that bitcoin-core/secp256k1#988 will be in before we have a release. Anyway, we should seriously do a release! |
These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048. Github-Pull: bitcoin#23314 Rebased-From: d752454
|
Backported in #23315. |
…ased tests e959b46 build: explicitly disable libsecp256k1 openssl based tests (fanquake) Pull request description: Backport of #23314 These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See #23048. Top commit has no ACKs. Tree-SHA512: ab3213dc82e7a64a005ce237710009bb447dee2702c4c02245e70df62063a00add73c4e80e9c619ce57345d4a2808fd4dc08e2e02a319b0f3d9285b8b0056599
These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048. Github-Pull: bitcoin#23314 Rebased-From: d752454
314195c Remove unnecessary cast in CKey::SignSchnorr (Pieter Wuille) a1f76cd Remove --disable-openssl-tests for libsecp256k1 configure (Pieter Wuille) 86dbc4d Squashed 'src/secp256k1/' changes from be8d9c2..0559fc6 (Pieter Wuille) Pull request description: The motivation for this bump is getting rid of a cast in `CKey::SignSchnorr`; the `aux_rand` argument isn't modified by the `secp256k1_schnorrsig_sign` function, but was marked as non-`const` anyway. This is fixed now (bitcoin-core/secp256k1#966), and the cast is removed in this PR. There are a few other relevant changes: * (bitcoin-core/secp256k1#956): replaces a runtime-computed table with a precomputed one; this adds arouns 1 MiB to the binary size, but is a step towards significantly simplifying the API. If 1 MiB is too much, it can be reduced by 2 or 4 (or more) for a slight verification performance reduction. * (bitcoin-core/secp256k1#983): removes (test/bench only) OpenSSL support entirely, removing the need to pass `--disable-openssl-tests` (see #23314). * (bitcoin-core/secp256k1#810): mild performance increase for 64-bit non-x86 platforms. * (bitcoin-core/secp256k1#1002): Make aux_rnd32==NULL behave identical to 0x0000..00 (which impacts BIP341/BIP342 signing in Bitcoin Core, making it more strictly BIP340 compliant, though not in a manner that affects security). ACKs for top commit: fanquake: ACK 314195c - this includes a nice simplification to the lilbsecp build system (and thus our build system), and fixes issues like #22854. Did a Guix build on x86 (above), as well as a build on arm64 (except for the arm64 host): Tree-SHA512: 0e048390fc148fbbdf5b98d9cce8c71067564e7d69d97b68347808a9bc45a04f4fc653c392c880d79d5d8b9cf282195520955581ac4f1595f6a948080cf5949d
…enssl based tests e959b46 build: explicitly disable libsecp256k1 openssl based tests (fanquake) Pull request description: Backport of bitcoin#23314 These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048. Top commit has no ACKs. Tree-SHA512: ab3213dc82e7a64a005ce237710009bb447dee2702c4c02245e70df62063a00add73c4e80e9c619ce57345d4a2808fd4dc08e2e02a319b0f3d9285b8b0056599
…enssl based tests (#4941) e959b46 build: explicitly disable libsecp256k1 openssl based tests (fanquake) Pull request description: Backport of bitcoin#23314 These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048. Top commit has no ACKs. Tree-SHA512: ab3213dc82e7a64a005ce237710009bb447dee2702c4c02245e70df62063a00add73c4e80e9c619ce57345d4a2808fd4dc08e2e02a319b0f3d9285b8b0056599 Co-authored-by: MarcoFalke <falke.marco@gmail.com>
These tests are failing when run against OpenSSL 3, and have been
removed upstream, bitcoin-core/secp256k1#983, so
disabled them for now to avoid
make checkfailures.Note that this will also remove warning output from our build, due to
the use of deprecated OpenSSL API functions. See #23048.