Backport Bitcoin Qt/Gui changes up to 0.14.x part 3 #1617

codablock · 2017-09-09T09:49:50Z

This is the third part of Qt/Gui related PRs being backported from Bitcoin to Dash.

I tried to stick with modal overlay related PRs, but also included some PRs which contain changes later touched by the overlay related PRs.

This PR also includes parts of the network activity toggle I mentioned here. I added a commit on top of that PR to revert the functionality of the network status icon, so that it keeps opening the peers list instead of toggling network state. This means, that for now it's only possible to toggle network status with the new RPC call setnetworkactive. Later we may add a context menu to the status icon which gives an option to toggle network status. At that point we should probably also replace the network disabled icon to something that fits to the style of the Dash network status.

bitcoin#9218 also introduced the ClickableLabel which was then used for the network toggle and the sync spinner icon. While reverting the changes to the network status icon, I decided to also use the ClickableLabel for the status icon (removes the QPushButton flat button + style handling, making future backporting easier).

With this backport PR merged, we have all the functionality of the modal overlay from Bitcoin 0.14. The remaining backports that I'll do are not related to the overlay, and thus can be postponed to later (after the next releast).

19f46f1 Qt: New network_disabled icon (Luke Dashjr) 54cf997 RPC/Net: Use boolean consistently for networkactive, and remove from getinfo (Luke Dashjr) b2b33d9 Overhaul network activity toggle (Jonas Schnelli) 32efa79 Qt: Add GUI feedback and control of network activity state. (Jon Lund Steffensen) e38993b RPC: Add "togglenetwork" method to toggle network activity temporarily (Jon Lund Steffensen) 7c9a98a Allow network activity to be temporarily suspended. (Jon Lund Steffensen)

Stay with the way Dash handled clicking on the status icon

UdjinM6

Places where we use ConnectNode directly (like ThreadMnbRequestConnections or mixing) should also check for fNetworkActive, otherwise setnetworkactive has limited effect. Ideally we shouldn't use ConnectNode directly at all and should use OpenNetworkConnection in some way instead but that's a whole another story I guess.

Pinging @OlegGirko for review/ideas.

Also, see inline comments

UdjinM6 · 2017-09-10T09:35:13Z

contrib/debian/copyright

@@ -51,7 +51,10 @@ Comment: Site: https://github.com/stephenhutchings/typicons.font

 Files: src/qt/res/icons/connect*.png
       src/qt/res/src/connect-*.svg
+       src/qt/res/icons/network_disabled.png


src/qt/res/icons/*/network_disabled.png

I noticed that all icons suffer from wrong entries in contrib/debian/copyright and thus decided to not touch this one as well. If you feel this should be fixed, I can put it onto my TODO and later create a PR that fixes it for all icons.

I'm not sure I understand - you moved icons in this exact PR in 0056497196ae6bfa0048d672f71d2a21146ef5d6 so why not fixing references at the very same time? We should be fixing issues, not introducing new ones ;) Other (unrelated) broken references should be fixed later in a separate PR ofc.

Ok, fixed this and added a TODO for the other icons.

UdjinM6 · 2017-09-10T09:41:51Z

src/qt/sendcoinsdialog.cpp

@@ -17,6 +17,7 @@
 #include "walletmodel.h"

 #include "base58.h"
+#include "chainparams.h"


Remainder of bitcoin#9588, which originally also updated parts in sendcoinsdialog.cpp. I omitted these changes for Dash as the whole fee handling in Dash seems to be different. I now added a commit on top to remove the include.

codablock · 2017-09-10T14:55:44Z

Regarding fNetworkActive, if you feel it's too dangerous to include this at this point in time, I can remove the commits completely and figure out the merge conflicts. I will then add a dedicated commit for only this functionality, including a proper fix for the missing UI (context menu) for it.

UdjinM6 · 2017-09-10T19:08:36Z

re fNetworkActive:
I have another idea - instead of tracking down every ConnectNode we can probably just do this

diff --git a/src/net.cpp b/src/net.cpp
index d90e1442d..d4a4222a0 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -346,6 +346,10 @@ bool CConnman::CheckIncomingNonce(uint64_t nonce)
 
 CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fConnectToMasternode)
 {
+    if (!fNetworkActive) {
+        return NULL;
+    }
+
     if (pszDest == NULL) {
         // we clean masternode connections in CMasternodeMan::ProcessMasternodeConnections()
         // so should be safe to skip this and connect to local Hot MN on CActiveMasternode::ManageState()

I did a quick test and it seems to be working as expected without any issues.

UdjinM6 · 2017-09-10T20:49:45Z

btw, we need bitcoin#9528 and bitcoin#11237 too I guess

Network active toggle was already based on "[RPC] Give RPC commands more information about the RPC request" We need to use the old UniValue style until that one is backported

ed998ea qt: Avoid OpenSSL certstore-related memory leak (Wladimir J. van der Laan) 5204598 qt: Avoid shutdownwindow-related memory leak (Wladimir J. van der Laan) e4f126a qt: Avoid splash-screen related memory leak (Wladimir J. van der Laan) 693384e qt: Prevent thread/memory leak on exiting RPCConsole (Wladimir J. van der Laan) 47db075 qt: Plug many memory leaks (Wladimir J. van der Laan)

042f9fa qt: Show progress overlay when clicking spinner icon (Wladimir J. van der Laan) 827d9a3 qt: Replace NetworkToggleStatusBarControl with generic ClickableLabel (Wladimir J. van der Laan)

1077577 Fix auto-deselection of peers (Andrew Chow) addfdeb Multiple Selection for peer and ban tables (Andrew Chow)

…ht places df17fe0 Bugfix: Qt/RPCConsole: Put column enum in the right places (Luke Dashjr)

…outToBeChanged f36349e qt: Remove on_toggleNetworkActiveButton_clicked from RPCConsole (Wladimir J. van der Laan) 297cc20 qt: layoutAboutToChange signal is called layoutAboutToBeChanged (Wladimir J. van der Laan)

… and peer-finding 40ec7c7 [Qt] Improve progress display during headers-sync and peer-finding (Jonas Schnelli)

fa4d478 qt: Use nPowTargetSpacing constant (MarcoFalke)

…ar, allow hiding 89a3723 [Qt] Show ModalOverlay by pressing the progress bar, disabled show() in sync mode (Jonas Schnelli)

fafeec3 [qt] sync-overlay: Don't show progress twice (MarcoFalke)

…idates for NotifyHeaderTip() 3154d6e [Qt] use NotifyHeaderTip's height and date for the progress update (Jonas Schnelli) 0a261b6 Use pindexBestHeader instead of setBlockIndexCandidates for NotifyHeaderTip() (Jonas Schnelli)

Don't allow to open it again by clicking on the progress bar and spinner icon. Currently the overlay does not show meaningful information about masternode sync and it gives the impression of being stuck after the block chain sync is done.

This was just a remainder of a backported PR which meant to change some calculation in this file which does not apply to Dash.

…atNiceTimeOffset(qint64) 988d300 [qt] Rename formateNiceTimeOffset(qint64) to formatNiceTimeOffset(qint64) (practicalswift)

c8d38ab Refactor tipUpdate as per style guide (MeshCollider) 3b69a08 Fix division by zero in time remaining (MeshCollider) Pull request description: Fixes bitcoin#10291, bitcoin#11265 progressDelta may be 0 (or even negative according to 11265), this checks for that and prints unknown if it is, because we cannot calculate an estimate for the time remaining (would be infinite or negative). Tree-SHA512: bc5708e5ed6e4670d008219558c5fbb25709bd99a32c98ec39bb74f94a0b7fa058f3d03389ccdd39e6723e6b5b48e34b13ceee7c051c2db631e51d8ec3e1d68c

codablock · 2017-09-11T06:16:00Z

I've added bitcoin#9528 and bitcoin#11237 to the PR. The first one was already on my local branch for the next batch and the second one is pretty new (not even in Bitcoin 0.15).

EDIT: And I've added the fNetworkActive snipped from above.

UdjinM6

👍
Slightly tested ACK for now, will test a bit more to make sure we didn't miss smth.

jonasschnelli and others added 2 commits September 9, 2017 09:39

Revert on-click behavior of network status icon to showing peers list

8a04c20

Stay with the way Dash handled clicking on the status icon

UdjinM6 requested changes Sep 10, 2017

View reviewed changes

codablock and others added 17 commits September 11, 2017 07:52

Add theme support for network disabled icon

064e6cd

Use UniValue until bitcoin PR bitcoin#8788 is backported

b4bd44b

Network active toggle was already based on "[RPC] Give RPC commands more information about the RPC request" We need to use the old UniValue style until that one is backported

Merge bitcoin#9218: qt: Show progress overlay when clicking spinner icon

6336ac1

042f9fa qt: Show progress overlay when clicking spinner icon (Wladimir J. van der Laan) 827d9a3 qt: Replace NetworkToggleStatusBarControl with generic ClickableLabel (Wladimir J. van der Laan)

Merge bitcoin#8874: Multiple Selection for peer and ban tables

23fb5b6

1077577 Fix auto-deselection of peers (Andrew Chow) addfdeb Multiple Selection for peer and ban tables (Andrew Chow)

Merge bitcoin#9266: Bugfix: Qt/RPCConsole: Put column enum in the rig…

67e317c

…ht places df17fe0 Bugfix: Qt/RPCConsole: Put column enum in the right places (Luke Dashjr)

Merge bitcoin#9461: [Qt] Improve progress display during headers-sync…

ac375a1

… and peer-finding 40ec7c7 [Qt] Improve progress display during headers-sync and peer-finding (Jonas Schnelli)

Merge bitcoin#9588: qt: Use nPowTargetSpacing constant

c90944e

fa4d478 qt: Use nPowTargetSpacing constant (MarcoFalke)

Merge bitcoin#9280: [Qt] Show ModalOverlay by pressing the progress b…

47ee86d

…ar, allow hiding 89a3723 [Qt] Show ModalOverlay by pressing the progress bar, disabled show() in sync mode (Jonas Schnelli)

Merge bitcoin#8906: [qt] sync-overlay: Don't show progress twice

a9300ba

fafeec3 [qt] sync-overlay: Don't show progress twice (MarcoFalke)

Don't include chainparams.h in sendcoinsdialog.cpp

4f29f0b

This was just a remainder of a backported PR which meant to change some calculation in this file which does not apply to Dash.

Also check for fNetworkActive in ConnectNode

c74dfec

Merge bitcoin#9528: [qt] Rename formateNiceTimeOffset(qint64) to form…

75fd0fc

…atNiceTimeOffset(qint64) 988d300 [qt] Rename formateNiceTimeOffset(qint64) to formatNiceTimeOffset(qint64) (practicalswift)

codablock force-pushed the backport_bitcoin_qt_3 branch from c561211 to 1f1a609 Compare September 11, 2017 06:16

UdjinM6 added this to the 12.2 milestone Sep 11, 2017

UdjinM6 approved these changes Sep 11, 2017

View reviewed changes

UdjinM6 merged commit 91d99fc into dashpay:v0.12.2.x Sep 11, 2017

codablock deleted the backport_bitcoin_qt_3 branch September 14, 2018 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport Bitcoin Qt/Gui changes up to 0.14.x part 3 #1617

Backport Bitcoin Qt/Gui changes up to 0.14.x part 3 #1617

codablock commented Sep 9, 2017 •

edited

Loading

UdjinM6 left a comment •

edited

Loading

UdjinM6 Sep 10, 2017

codablock Sep 10, 2017 •

edited

Loading

UdjinM6 Sep 10, 2017 •

edited

Loading

codablock Sep 11, 2017

UdjinM6 Sep 10, 2017

codablock Sep 10, 2017 •

edited

Loading

codablock commented Sep 10, 2017 •

edited

Loading

UdjinM6 commented Sep 10, 2017

UdjinM6 commented Sep 10, 2017 •

edited

Loading

codablock commented Sep 11, 2017 •

edited

Loading

UdjinM6 left a comment

Backport Bitcoin Qt/Gui changes up to 0.14.x part 3 #1617

Backport Bitcoin Qt/Gui changes up to 0.14.x part 3 #1617

Conversation

codablock commented Sep 9, 2017 • edited Loading

UdjinM6 left a comment • edited Loading

Choose a reason for hiding this comment

UdjinM6 Sep 10, 2017

Choose a reason for hiding this comment

codablock Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

UdjinM6 Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

codablock Sep 11, 2017

Choose a reason for hiding this comment

UdjinM6 Sep 10, 2017

Choose a reason for hiding this comment

codablock Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

codablock commented Sep 10, 2017 • edited Loading

UdjinM6 commented Sep 10, 2017

UdjinM6 commented Sep 10, 2017 • edited Loading

codablock commented Sep 11, 2017 • edited Loading

UdjinM6 left a comment

Choose a reason for hiding this comment

codablock commented Sep 9, 2017 •

edited

Loading

UdjinM6 left a comment •

edited

Loading

codablock Sep 10, 2017 •

edited

Loading

UdjinM6 Sep 10, 2017 •

edited

Loading

codablock Sep 10, 2017 •

edited

Loading

codablock commented Sep 10, 2017 •

edited

Loading

UdjinM6 commented Sep 10, 2017 •

edited

Loading

codablock commented Sep 11, 2017 •

edited

Loading