Add option `-alerts` to opt out of alert system #6274

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2015_06_disable_alert changing 3 files +8 −1

laanwj commented at 10:04 AM on June 12, 2015: member

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.
laanwj added the label RPC on Jun 12, 2015
in src/main.cpp:None in 6cf9c828cd outdated
```
4622 | @@ -4622,7 +4623,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
4623 |      }
4624 |  
4625 |  
4626 | -    else if (strCommand == "alert")
4627 | +    else if (strCommand == "alert" && fAlerts)
```
Diapolo commented at 10:14 AM on June 12, 2015:

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?

laanwj commented at 10:38 AM on June 12, 2015:

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)
jtimon commented at 10:22 AM on June 12, 2015: contributor

ut ACK
paveljanik commented at 10:42 AM on June 12, 2015: contributor

&& fAlerts means that every incoming "alert" will log "Unknown command" when alerts disabled. Do you want it this way?
laanwj commented at 10:45 AM on June 12, 2015: member

@paveljanik Yea - good catch, but I'm ok with that. This is effectively a way of saying that you don't acknowledge that the alert message is part of the P2P protocol. Also remember that "unknown message" will only appear with -debug=net. (but it's easy enough to avoid if people prefer that...)
rebroad commented at 11:57 AM on June 12, 2015: contributor

@laanwj I'd prefer it didn't treat "alert" as an unknown message, but instead logged it if debug=alert is set
jgarzik commented at 12:33 PM on June 12, 2015: contributor

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.
laanwj commented at 12:49 PM on June 12, 2015: member

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.
sipa commented at 1:16 PM on June 14, 2015: member

Untested ACK

Add option `-alerts` to opt out of alert system

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.

02a6702a82

laanwj force-pushed on Jun 15, 2015
laanwj merged this on Jun 15, 2015
laanwj closed this on Jun 15, 2015
laanwj referenced this in commit c1fb0e1075 on Jun 15, 2015
laanwj referenced this in commit be6420407b on Jun 15, 2015
laanwj referenced this in commit 4d9c7fe61d on Jun 15, 2015
ABISprotocol commented at 10:21 PM on November 23, 2016: none

Hello, question:

Since the following will have disabled alerts, Bitcoin Core 0.13.1, 0.13.0, 0.12.1 Bitcoin Core 0.10.3, 0.11.x, and 0.12.x can disable alerts with -alerts=0 Armory 0.94.1+ how can a user view alerts if they want to view them (assuming that they are disabled in the software, but want to view an alert somewhere else)?
dcousens commented at 11:53 PM on November 23, 2016: contributor

utACK 02a6702
MarcoFalke commented at 12:53 PM on November 26, 2016: member

how can a user view alerts if they want to view them @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.
MarcoFalke commented at 12:55 PM on November 26, 2016: member

Regardless of that, please don't use year old pull request for help questions, which should go to bitcoin stackexchange instead.
in src/main.cpp:None in 02a6702a82
```
4622 | @@ -4622,7 +4623,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
4623 |      }
4624 |  
4625 |  
4626 | -    else if (strCommand == "alert")
4627 | +    else if (fAlerts && strCommand == "alert")
```
rebroad commented at 4:05 AM on December 4, 2016:

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).

laanwj commented at 10:18 AM on December 5, 2016:

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.
ABISprotocol commented at 10:29 AM on December 20, 2016: none

Thanks for clarification.
rubenscholfield commented at 4:39 PM on January 12, 2017: none

Hello, Commander Scholfield here.
rubenscholfield commented at 4:50 PM on January 12, 2017: none

OK, see you...
reddink referenced this in commit 88eac01eb9 on May 27, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6260 Remove alert system