Skip to content

Conversation

@jonasschnelli
Copy link
Contributor

Alternative solution for #7586

Removes openssl information log dump during startup as well as from the QT debug window.
Openssl is no longer consensus critical, though, it still in use for random number generation, AES encryption and BIP70 (payment protocol, GUI only).

@paveljanik
Copy link
Contributor

I think that logging SSL library used for generating random numbers is still worth it. We do not need it in the GUI though, I agree.

@maflcko
Copy link
Member

maflcko commented Feb 26, 2016

Great to see this removed from the GUI.

utACK 5ecfa36. (building right now)

Tested ACK 5ecfa36

(I am ok to keep it in the debug.log, but I don't have a strong opinion here)

@AliceWonderMiscreations
Copy link
Contributor

I also think it should be in the debug log but I do not think it serves a purpose in the Qt Information dialog.

Way off topic but in my dialog, the Build date appears to be wrong, reporting 17 Feb which is before 0.12.0 was released. Wonder if that is a bug in my chroot build environment.

@maflcko
Copy link
Member

maflcko commented Feb 27, 2016

I also think it should be in the debug log

Imo it's just not worth the effort to maintain. (We've had more than 3 pull request about "fixing" this in the last month). There are other trivial ways to determine the version of the library you are using

Way off topic but in my dialog, the Build date appears to be wrong, reporting 17 Feb which is before 0.12.0 was released. Wonder if that is a bug in my chroot build environment.

The build date for releases is the date when the last commit was commited:

$ git log  --format=%cD 188ca9c305d3dd0fb462b9d6a44048b1d99a05f3 -1
Wed, 17 Feb 2016 09:40:03 +0100

@paveljanik
Copy link
Contributor

@MarcoFalke The sentence "There are other trivial ways to determine the version of the library you are using" unfortunately applies only to users with deep technical knowledge. Our users mix different versions of SSL libraries, at compile time and at runtime. The fixes applied to this part of our code only handle such strange instances. It is very useful to know what was used at runtime, really.

@AliceWonderMiscreations
Copy link
Contributor

Thanks @MarcoFalke so it isn't really the "build" date as I think of "build" date (compile)
-=-
sorry for OT

@laanwj
Copy link
Member

laanwj commented Feb 29, 2016

I like this solution.

The build date should probably go away as well, it's confusing at least for the executables built in gitian, as the date/time is overridden to create deterministic executables. I don't think it adds very much in any case.

Imo it's just not worth the effort to maintain. (We've had more than 3 pull request about "fixing" this in the last month).

Yes, that's kind of annoying, and usually means a deeper solution is desirable.
OTOH we probably got it right now, finally. Removing it from just the GUI is fine with me too.

@laanwj laanwj added the GUI label Feb 29, 2016
@jonasschnelli
Copy link
Contributor Author

Agree with removing the build-date (in another PR), an alternative solution would be to only show the build date in non release builds (!CLIENT_VERSION_IS_RELEASE).

I think we should merge this PR and remove openssl from the logs and the GUI. IMO there is not much value in logging the openssl version regarding to PRNG, and, eventually we'll end up with a custom PRNG implementation (#5885) anyways.

@luke-jr
Copy link
Member

luke-jr commented Feb 29, 2016

It may actually be useful to log the version/hash of every library being used, but probably no reason to single out OpenSSL.

@laanwj
Copy link
Member

laanwj commented Mar 1, 2016

@luke-jr Well we log BerkeleyDB version as well. But at least there's a good reason for that, to troubleshoot wallet incompatibility.

@laanwj laanwj merged commit 5ecfa36 into bitcoin:master Mar 3, 2016
laanwj added a commit that referenced this pull request Mar 3, 2016
5ecfa36 Remove openssl info from init/log and from Qt debug window (Jonas Schnelli)
laanwj pushed a commit that referenced this pull request Mar 24, 2016
Conflicts:
	src/init.cpp

Github-Merge: #7605
Rebased-From: 5ecfa36
thokon00 pushed a commit to faircoin/faircoin that referenced this pull request Jun 28, 2016
Conflicts:
	src/init.cpp

Github-Merge: bitcoin#7605
Rebased-From: 5ecfa36
Warrows pushed a commit to Warrows/PIVX that referenced this pull request Aug 28, 2019
Kokary pushed a commit to Kokary/wagerr that referenced this pull request May 26, 2020
Kokary pushed a commit to Kokary/wagerr that referenced this pull request Nov 13, 2020
Kokary pushed a commit to Kokary/wagerr that referenced this pull request Nov 17, 2020
Kokary pushed a commit to Kokary/wagerr that referenced this pull request Nov 17, 2020
KolbyML pushed a commit to KolbyML/bitcoin that referenced this pull request Nov 21, 2020
Kokary pushed a commit to wagerr/wagerr that referenced this pull request Nov 24, 2020
Cryptarchist pushed a commit to wagerr/wagerr that referenced this pull request Nov 30, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants