[Qt] show network/chain errors in the GUI #7579

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2016/02/gui_alert changing 1 files +5 −3

jonasschnelli commented at 3:51 PM on February 22, 2016: contributor

Alternative solution to #7461.

This will show fork- and upgrade-warnings in the UI over the Notificator class as well as in the overview screen alert area. Warnings can be combined (simple HTML <hr> separator). Warnings will be removed (if condition no longer exists) over the periodic PartitionCheck() function (updated once a day).
jonasschnelli added the label GUI on Feb 22, 2016
jonasschnelli commented at 4:15 PM on February 22, 2016: contributor

Example: <img width="1407" alt="bildschirmfoto 2016-02-22 um 17 15 25" src="https://cloud.githubusercontent.com/assets/178464/13224239/e65793b8-d987-11e5-8e7f-e691ffd7d634.png">
btcdrak commented at 4:46 PM on February 22, 2016: contributor

Nice work. Concept ACK.
jonasschnelli added the label Needs backport on Feb 23, 2016
jonasschnelli commented at 8:07 AM on February 24, 2016: contributor

Binaries if someone wants to test this: https://bitcoin.jonasschnelli.ch/pulls/7579/
laanwj commented at 7:26 PM on March 7, 2016: member

Nice. Concept ACK.

Warnings will be removed (if condition no longer exists) over the periodic PartitionCheck() function (updated once a day).

That's essential. Most of those chain conditions are temporary conditions, so they should go away as soon as they're no longer detected.
MarcoFalke commented at 11:39 AM on March 14, 2016: member

Concpet ACK 44de50b
MarcoFalke commented at 4:52 PM on April 24, 2016: member

Needs rebase.
sipa commented at 1:58 PM on June 3, 2016: member

Still needs rebase.
jonasschnelli force-pushed on Jun 10, 2016
jonasschnelli force-pushed on Jun 10, 2016
jonasschnelli force-pushed on Jun 10, 2016
jonasschnelli commented at 8:32 AM on June 10, 2016: contributor

Rebase. I guess we should also take this into 0.13.

in src/main.cpp:None in 1020788e46 outdated

4369 | @@ -4364,18 +4370,19 @@ std::string GetWarnings(const std::string& strFor)
4370 |      // Misc warnings like out of disk space and clock is wrong
4371 |      if (strMiscWarning != "")
4372 |      {
4373 | -        strStatusBar = strGUI = strMiscWarning;
4374 | +        strStatusBar = strMiscWarning;
4375 | +        strGUI += strGUI.empty() ? "" : uiAlertSeperator + strMiscWarning;

MarcoFalke commented at 8:58 AM on June 10, 2016:

Nit: That was confusing to read. Mind to add curly brackets around the ternary operator?

jonasschnelli commented at 9:12 AM on June 10, 2016:

Hmm.. I don't see your solution with curly brackets, can you post an example? IMO this line is readable. If not, we could with to an if/else.

sipa commented at 9:26 AM on June 10, 2016:

I think he means strGUI += (strGUI.empty() ? "" : uiAlertSeparator) + strMiscWarning;@.

MarcoFalke commented at 9:46 AM on June 10, 2016:

Jup, I meant round brackets. (Not curly) 😣

No need for an extra if statement. You can put it in two lines if one is too long:

diff --git a/src/main.cpp b/src/main.cpp
index 505fd3a..e041472 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -4373,3 +4373,4 @@ std::string GetWarnings(const std::string& strFor)
         strStatusBar = strMiscWarning;
-        strGUI += strGUI.empty() ? "" : uiAlertSeperator + strMiscWarning;
+        strGUI += (strGUI.empty() ? "" : uiAlertSeperator) +
+                  strMiscWarning;
     }

jonasschnelli force-pushed on Jun 10, 2016
laanwj commented at 12:54 PM on June 21, 2016: member

This is still blocked on solving the spurious false positive issue for 0.13: #7568, #7488
MarcoFalke commented at 5:12 PM on July 18, 2016: member

This is still blocked on solving the spurious false positive issue for 0.13: #7568, #7488

So what is the current state of this pull right now?
[Qt] show network/chain errors in the GUI 2f32c82b3d
jonasschnelli force-pushed on Jul 20, 2016
jonasschnelli commented at 12:43 PM on July 20, 2016: contributor

Rebased. Seeks a quick retest.
jtimon commented at 7:33 PM on July 20, 2016: contributor

utACK 2f32c82
sipa commented at 12:54 PM on August 25, 2016: member

I think this is ready for merge?
jonasschnelli merged this on Aug 25, 2016
jonasschnelli closed this on Aug 25, 2016
jonasschnelli referenced this in commit 0606f95b1e on Aug 25, 2016
MarcoFalke added this to the milestone 0.13.1 on Aug 26, 2016
MarcoFalke removed the label Needs backport on Sep 9, 2016

in src/main.cpp:None in 2f32c82b3d

4667 |  
4668 |      if (fLargeWorkForkFound)
4669 |      {
4670 |          strStatusBar = strRPC = "Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.";
4671 | -        strGUI = _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.");
4672 | +        strGUI += strGUI.empty() ? "" : uiAlertSeperator + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.");

rodasmith commented at 4:38 PM on September 10, 2016:

Is this missing parentheses?

strGUI += (strGUI.empty() ? "" : uiAlertSeperator) + _("Warning: The network does not appear to fully agree! Some miners appear to be experiencing issues.");

If strGUI is empty, the non-parenthesized line only appends "", losing the fLargeWorkForkFound warning. Same with the fLargeWorkInvalidChainFound warning on line 4675.

rodasmith commented at 10:48 PM on September 10, 2016:

PR: fix op order to append first alert #8697

codablock referenced this in commit 10e238f86c on Sep 19, 2017
codablock referenced this in commit 8ea2f65f75 on Jan 9, 2018
codablock referenced this in commit 3fca7e2044 on Jan 9, 2018
andvgal referenced this in commit 2c1751d5d8 on Jan 6, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Milestone
0.13.1

Linked (view graph)

#7461 Make internal (core) errors show up in the Qt client.#8679 [0.13] Various backports