Add option -alerts to opt out of alert system#6274
Conversation
src/main.cpp
Outdated
There was a problem hiding this comment.
Nit: The check for fAlerts would be faster I guess? Why not place switch the checks? If we are talking about a DoS situation perhaps that could make things less ressource hungry?
There was a problem hiding this comment.
OK, yes, agreed (though in practice, if the short string comparison gives a DoS risk we'll have worse problems... so this is not really worth worrying about)
|
ut ACK |
|
|
|
@paveljanik Yea - good catch, but I'm ok with that. This is effectively a way of saying that you don't acknowledge that the |
|
@laanwj I'd prefer it didn't treat "alert" as an unknown message, but instead logged it if debug=alert is set |
|
ut ACK Very odd - I thought this facility to disable alerts already existed. Apparently not. I was probably thinking of -alertnotify The patch is fine as-is, though I can see the value of debug=alert versus debug=net too. |
|
To log the alert in any sensible way, it'd have to be parsed (possibly even verified). That's exactly what I'm trying to avoid. |
|
Untested ACK |
Make it possible to opt-out of the centralized alert system by providing an option `-noalerts` or `-alerts=0`. The default remains unchanged. This is a gentler form of bitcoin#6260, in which I went a bit overboard by removing the alert system completely. I intend to add this to the GUI options in another pull after this.
laanwj
force-pushed
the
2015_06_disable_alert
branch
from
aa00660 to
02a6702
Compare
02a6702 Add option `-alerts` to opt out of alert system (Wladimir J. van der Laan)
Make it possible to opt-out of the centralized alert system by providing an option `-noalerts` or `-alerts=0`. The default remains unchanged. This is a gentler form of #6260, in which I went a bit overboard by removing the alert system completely. I intend to add this to the GUI options in another pull after this. Conflicts: src/init.cpp src/main.cpp Github-Pull: #6274 Rebased-From: 02a6702
Make it possible to opt-out of the centralized alert system by providing an option `-noalerts` or `-alerts=0`. The default remains unchanged. This is a gentler form of #6260, in which I went a bit overboard by removing the alert system completely. I intend to add this to the GUI options in another pull after this. Github-Pull: #6274 Rebased-From: 02a6702
|
Hello, question: Since the following will have disabled alerts, |
|
utACK 02a6702 |
@ABISprotocol There won't be any other alerts than the final alert, so there is no reason to enable alerts as a user, other than for academic purposes. See https://bitcoin.org/en/alert/2016-11-01-alert-retirement Please note that internal alerts, partition detection warnings and the -alertnotify option features remain according to #7692. |
|
Regardless of that, please don't use year old pull request for help questions, which should go to bitcoin stackexchange instead. |
|
|
||
|
|
||
| else if (strCommand == "alert") | ||
| else if (fAlerts && strCommand == "alert") |
There was a problem hiding this comment.
This will cause an "Unknown command" message to be logged if fAlerts is false, better to exit is strCommand == "alert" yet fAlerts is false (since "alert" is not an unknown command).
There was a problem hiding this comment.
Thanks for the code review, more than a year after the fact.
However this code has been ripped out already together with the alert logic, so it's no longer relevant.
|
Thanks for clarification. |
|
Hello, |
|
OK, see you... |
Make it possible to opt-out of the centralized alert system by providing an option `-noalerts` or `-alerts=0`. The default remains unchanged. This is a gentler form of bitcoin#6260, in which I went a bit overboard by removing the alert system completely. I intend to add this to the GUI options in another pull after this. Conflicts: src/init.cpp src/main.cpp Github-Pull: bitcoin#6274 Rebased-From: 02a6702 (cherry picked from commit be64204) # Conflicts: # src/main.cpp # src/main.h
11 participants
Make it possible to opt-out of the centralized alert system by providing an option
-noalertsor-alerts=0. The default remains unchanged.This is a gentler form of #6260, in which I went a bit overboard by removing the alert system completely.
I intend to add this to the GUI options in another pull.