-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
OpenSSL 1.1 - tests* BIGNUM - BN_init -> BN_new #7086
Comments
If this isn't actually being used in the library, the test may be better suited for libsecp256k1 and tested there. |
Agree with @laanwj. These were added to ensure consensus behavior against the pre-0.10 bignum implementation. At this point we'd likely take the new behavior anyway if an inconsistency was discovered. |
From what I've heard, secp256k1 already does tons of much better tests :) As @theuni says the test is there to compare our scriptnum values' behavior with OpenSSL. This mattered for 0.10, but indeed, at this point if OpenSSL starts to diverge, we'd take our behavior. If we still want to do a cross-version test like this usefully we should compare against, say, 0.10's post-OpenSSL CScriptNum implementation. |
@dcousens these aren't related to libsec256k1 at all. They're tests of the number implementation used in script, which long ago used OpenSSL but does no more. I agree these could be removed, today the only value they provide is additional chances of catching our implementation changing between versions. If they did turn up a disagreement with the current code now and OpenSSL, we'd ignore it: I don't believe the openssl based code is in widespread use on the network anymore. This would be better handled by copying an implementation (like wumpus says, 0.10's) into the tests. |
Compare against the scriptnum from Bitcoin Core 0.10 instead of OpenSSL. Closes bitcoin#7086.
Sorry @laanwj but this is the first time I've had a request like that and don't know the standard way of testing those pull requests.
but of course that wouldn't be it since those PR's are waiting to be merged into Master. |
Start with master then:
Then you can test with the two pulls merged. When you're done, you can switch away from your new branch and delete it
|
Nope
init.cpp
crypto.h
libcrypto.pc
(at first I messed up because I copied/pasted your commands without noticing you typed 7096 instead of 7095, not that it matters because 7083 should have been the fix regarding OpenSSL_version). |
@vindicatorr Can you paste I am running openssl/openssl@3bbd1d6 and faf12bc. So I get:
|
@MarcoFalke
openssl
|
Strange. It looks like it is linking against a different (older) OpenSSL than is described by the headers (one without OpenSSL_version). |
For further completeness of my process:
The same process I've done with OpenSSL 1.0.2 (and 1.1 with --disable-tests and edited init.cpp to comment version line) |
And the error output with V=1
|
How did you setup OpenSSL? |
And "openssl version" outputs the 1.1
EDIT: and not that it should matter, but you pulled yours from github and I pulled mine straight from git://git.openssl.org/openssl.git |
@MarcoFalke I'm just perplexed. I deleted /usr/local/ssl, rebooted, reinstalled openssl, rebooted, rebuilt bitcoin and I'm still getting that linker error. |
@vindicatorr Are there other openSSL libraries installed on your system? My guess is that it tries linking against those. Note that for linking you need a |
Not sure why I have openssl stuff in /usr/local/lib and ssl/lib, but it may be due to some build attempt I made when trying to get another program to build (I remember, whatever program it was, saying the version information wasn't found or something, which I confirmed with readelf). Still, those other paths shouldn't even be a consideration for the compiler since I specified pkg_config and it even showed the local/ssl/lib linker path in the g++ output. |
The problem is that If you move |
@laanwj I was just about to say you got it right as the build, check and install all completed successfully.
fudge. Thought we had it.
Sure looks like libcrypto is being used in the right place. |
OpenSSL_version is in libssl, not in libcrypto, and from that list it appears you're not linking libssl? |
I needed to rebuild with V=1 and found those 8 instances where lssl was referenced.
|
AHHHHHH! Noticing something new. While searching online, I saw something else. |
Step-1 complete (openssl install): Now for bitcoin. |
Success. build, check, install, version info, help is all good and the service is back up and running. |
https://wiki.openssl.org/index.php/Manual:BN_new(3)
https://www.openssl.org/docs/man1.0.2/crypto/bn.html
void BN_init(BIGNUM *);
https://www.openssl.org/docs/manmaster/crypto/bn.html
BIGNUM *BN_new(void);
The text was updated successfully, but these errors were encountered: