[gui] Add proxy icon in statusbar #11491

mess110 commented at 9:24 PM on October 12, 2017: contributor

Relates to #7734

Please ignore the wrong alpha in the screenshot, I couldn't get the screenshot alpha right :(

I plan to extend this feature in future PRs to include:

custom Tor icon
clickable icon which opens network settings

Old proposals, dropped in favor of current

proxy_preview

luke-jr commented at 9:30 PM on October 12, 2017: member

Hmm, a fixed icon that is disabled/enabled doesn't extend well to a tri-state when Tor is added. But that can be deferred until adding Tor I guess.

fanquake added the label GUI on Oct 12, 2017

in src/Makefile.qt.include:281 in bd28377bb3 outdated

 277 | @@ -278,6 +278,8 @@ RES_ICONS = \
 278 |    qt/res/icons/network_disabled.png \
 279 |    qt/res/icons/open.png \
 280 |    qt/res/icons/overview.png \
 281 | +  qt/res/icons/proxy_enabled.png \

promag commented at 10:26 PM on October 12, 2017:

Sort.

in src/qt/bitcoingui.cpp:1099 in 597cd7bcb0 outdated

1091 | @@ -1084,6 +1092,30 @@ void BitcoinGUI::setEncryptionStatus(int status)
1092 |  }
1093 |  #endif // ENABLE_WALLET
1094 |  
1095 | +void BitcoinGUI::updateProxyIcon()
1096 | +{
1097 | +    bool proxyEnabled = false;
1098 | +    proxyType proxy;
1099 | +    UniValue networks(UniValue::VARR);

promag commented at 10:29 PM on October 12, 2017:

Unused, remove?

promag commented at 8:49 AM on October 13, 2017:

The #include above too.

in src/qt/bitcoingui.cpp:1113 in 597cd7bcb0 outdated

1108 | +            proxyEnabled = true;
1109 | +            break;
1110 | +        }
1111 | +    }
1112 | +
1113 | +    labelProxyIcon->show();

promag commented at 10:38 PM on October 12, 2017:

Already visible? Remove.

in src/qt/bitcoingui.cpp:1115 in 597cd7bcb0 outdated

1110 | +        }
1111 | +    }
1112 | +
1113 | +    labelProxyIcon->show();
1114 | +    labelProxyIcon->setPixmap(platformStyle->SingleColorIcon(proxyEnabled ? ":/icons/proxy_enabled" : ":/icons/proxy_disabled").pixmap(STATUSBAR_ICONSIZE,STATUSBAR_ICONSIZE));
1115 | +    labelProxyIcon->setToolTip(proxyEnabled ? tr("Proxy is <b>enabled</b>: %1").arg(QString::fromStdString(proxy.proxy.ToStringIPPort())) : tr("Proxy is <b>disabled</b>"));

promag commented at 10:41 PM on October 12, 2017:

Why the cosmetic in the tooltips?

promag commented at 8:40 AM on October 13, 2017:

Nit: If enabled tooltip = Proxy: %1 else Proxy disabled.

mess110 commented at 4:11 PM on October 13, 2017:

I think the bold helps readability. Can remove if needed.

in src/qt/bitcoingui.cpp:1097 in 597cd7bcb0 outdated

1091 | @@ -1084,6 +1092,30 @@ void BitcoinGUI::setEncryptionStatus(int status)
1092 |  }
1093 |  #endif // ENABLE_WALLET
1094 |  
1095 | +void BitcoinGUI::updateProxyIcon()
1096 | +{
1097 | +    bool proxyEnabled = false;

promag commented at 8:41 AM on October 13, 2017:

proxy_enabled.

in src/qt/bitcoingui.cpp:1101 in 597cd7bcb0 outdated

1096 | +{
1097 | +    bool proxyEnabled = false;
1098 | +    proxyType proxy;
1099 | +    UniValue networks(UniValue::VARR);
1100 | +
1101 | +    for(int n=0; n<NET_MAX; ++n)

promag commented at 8:42 AM on October 13, 2017:

for (int n = 0; n < NET_MAX; ++n) {

in src/qt/bitcoingui.cpp:1104 in 597cd7bcb0 outdated

1099 | +    UniValue networks(UniValue::VARR);
1100 | +
1101 | +    for(int n=0; n<NET_MAX; ++n)
1102 | +    {
1103 | +        enum Network network = static_cast<enum Network>(n);
1104 | +        if(network == NET_UNROUTABLE || network == NET_INTERNAL)

promag commented at 8:42 AM on October 13, 2017:

Space after if. continue in this line.

in src/qt/bitcoingui.cpp:1107 in 597cd7bcb0 outdated

1102 | +    {
1103 | +        enum Network network = static_cast<enum Network>(n);
1104 | +        if(network == NET_UNROUTABLE || network == NET_INTERNAL)
1105 | +            continue;
1106 | +        GetProxy(network, proxy);
1107 | +        if(proxy.IsValid()) {

promag commented at 8:43 AM on October 13, 2017:

Space after if.

in src/qt/bitcoingui.cpp:1114 in 597cd7bcb0 outdated

1109 | +            break;
1110 | +        }
1111 | +    }
1112 | +
1113 | +    labelProxyIcon->show();
1114 | +    labelProxyIcon->setPixmap(platformStyle->SingleColorIcon(proxyEnabled ? ":/icons/proxy_enabled" : ":/icons/proxy_disabled").pixmap(STATUSBAR_ICONSIZE,STATUSBAR_ICONSIZE));

promag commented at 8:44 AM on October 13, 2017:

Space after ,.

Nit, split in 2 lines? Or something like:

if (proxy_enabled) {
   ...
} else {
   ...
}

mess110 commented at 4:11 PM on October 13, 2017:

Removed space, prefer it like this for now.

promag commented at 8:49 AM on October 13, 2017: member

Tested ACK, some comments though.

promag commented at 8:51 AM on October 13, 2017: member

@luke-jr We can have an icon for each state no?

meshcollider commented at 9:16 AM on October 13, 2017: contributor

I'm unsure, but does this also need a mention in contrib/debian/copyright?

mess110 force-pushed on Oct 13, 2017

mess110 commented at 4:14 PM on October 13, 2017: contributor

@promag @MeshCollider Thanks for the reviews. If you think the remaining nits are very important, I can change them.

in src/qt/bitcoin.qrc:56 in 20b397987d outdated

  52 | @@ -53,6 +53,8 @@
  53 |          <file alias="hd_enabled">res/icons/hd_enabled.png</file>
  54 |          <file alias="hd_disabled">res/icons/hd_disabled.png</file>
  55 |          <file alias="network_disabled">res/icons/network_disabled.png</file>
  56 | +        <file alias="proxy_enabled">res/icons/proxy_enabled.png</file>

promag commented at 4:25 PM on October 13, 2017:

Nit, sort. :trollface:

in src/qt/bitcoingui.cpp:1099 in 20b397987d outdated

1090 | @@ -1084,6 +1091,26 @@ void BitcoinGUI::setEncryptionStatus(int status)
1091 |  }
1092 |  #endif // ENABLE_WALLET
1093 |  
1094 | +void BitcoinGUI::updateProxyIcon()
1095 | +{
1096 | +    bool proxy_enabled = false;
1097 | +    proxyType proxy;
1098 | +
1099 | +    for (int n = 0; n < NET_MAX; ++n) {

promag commented at 6:14 PM on October 13, 2017:

GetProxy returns IsValid so something like this should work:

for (int n = 0; !proxy_enabled && n < NET_MAX; ++n) {
    proxy_enabled = GetProxy((Network) n, proxy);
}

mess110 commented at 8:31 PM on October 13, 2017:

Nice, I didn't notice GetProxy checks for valid.

in src/qt/bitcoingui.h:194 in 20b397987d outdated

 189 | @@ -189,6 +190,9 @@ public Q_SLOTS:
 190 |      void incomingTransaction(const QString& date, int unit, const CAmount& amount, const QString& type, const QString& address, const QString& label);
 191 |  #endif // ENABLE_WALLET
 192 |  
 193 | +/** Set the proxy-enabled icon as shown in the UI. */
 194 | +void updateProxyIcon();

promag commented at 6:16 PM on October 13, 2017:

Missing indentation. No need to be a slot, just a private member.

promag commented at 6:17 PM on October 13, 2017: member

Should squash right?

mess110 force-pushed on Oct 13, 2017

in src/qt/bitcoingui.cpp:1100 in 8767cbbbb2 outdated

1095 | +{
1096 | +    bool proxy_enabled = false;
1097 | +    proxyType proxy;
1098 | +
1099 | +    for (int n = 0; !proxy_enabled && n < NET_MAX; ++n) {
1100 | +        proxy_enabled = GetProxy((Network) n, proxy);

jonasschnelli commented at 10:36 PM on October 13, 2017:

Instead of directly accessing core features, can you wrap that via clientmodel? There are serval tries to detach the GUI from the rest and those little things will cause a lot of work.

jonasschnelli commented at 10:37 PM on October 13, 2017: contributor

Concept ACK. Would prefer if we could use an icon from http://s-ings.com/typicons/.

ghost commented at 2:54 AM on October 14, 2017: none

First seeing the icon I was confused, meaning was not intuitive for me. The icon in the screenshot is so small, I hardly could imagine what the downgoing line is. Icon looked more like a star. But looking at the bigger icon on https://openclipart.org/detail/190624/load-balancer I could finally see that the icon visualizes "technically" correct. Since You said that You did several searches and ended up with this, I guess it's OK, the user can learn about the meaning and get used to it.

mess110 force-pushed on Oct 14, 2017

mess110 commented at 5:53 PM on October 14, 2017: contributor

@wodry guess I wasn't 100% clear. I attempted to construct a proxy icon from 3 copies of https://github.com/bitcoin/bitcoin/blob/master/src/qt/res/icons/debugwindow.png but it was too much to fit in the space and the icon ended up looking too different from the others. So I chose the icon you see because it fits and is public domain @jonasschnelli I checked the link you provided. Indeed it would be nice to include from the same set, but I couldn't find one that matches. Imo, the current icon is better. However, here are a few long shots:

flow merge
group
pinterest (as in Proxy - lol)
world

jonasschnelli commented at 4:17 AM on October 15, 2017: contributor

Tested a bit:

The icon seems hard to grasp what it is (already mentioned).
IMO the icon should only be visible when proxy is enabled (not faded out, completely invisible)
Clicking on the icon should open settings and showing the network/proxy tab

mess110 force-pushed on Oct 17, 2017

mess110 commented at 6:10 PM on October 17, 2017: contributor

The new icon is made from 3 device-desktop and 1 rotated chevron from http://s-ings.com/typicons/

After talking with @jonasschnelli, I will do the clickable icon in a different PR.

mess110 force-pushed on Oct 17, 2017

rebroad commented at 6:40 PM on October 17, 2017: contributor

I think it only makes sense to have this as an icon if the proxy is enableable/disableable from the GUI - otherwise, why? Why not have icons for various things, e.g. bloom filters enabled too then?

in src/qt/bitcoingui.h:13 in 15e1282602 outdated

   9 | @@ -10,6 +10,7 @@
  10 |  #endif
  11 |  
  12 |  #include "amount.h"
  13 | +#include <univalue.h>

promag commented at 9:49 PM on October 17, 2017:

Remove.

in src/qt/clientmodel.cpp:341 in 15e1282602 outdated

 336 | @@ -336,3 +337,20 @@ void ClientModel::unsubscribeFromCoreSignals()
 337 |      uiInterface.NotifyBlockTip.disconnect(boost::bind(BlockTipChanged, this, _1, _2, false));
 338 |      uiInterface.NotifyHeaderTip.disconnect(boost::bind(BlockTipChanged, this, _1, _2, true));
 339 |  }
 340 | +
 341 | +UniValue ClientModel::getProxyInfo() const

promag commented at 9:53 PM on October 17, 2017:

Suggestion to remove UniValue dependency:

bool ClientModel::GetProxyInfo(std::string& ip_port) const
{
    for (int n = 0; n < NET_MAX; ++n) {
        proxyType proxy;
        if (GetProxy((Network) n, proxy)) {
            ip_port = proxy.proxy.ToStringIPPort();
            return true;
        }
    }
    return false;
}

Haven't tested.

unknown commented at 5:13 AM on October 18, 2017:

I can not see how the new icon relates to proxy. Instead of showing "something with computer and cable", maybe simply show a "P" ?

promag commented at 10:04 PM on October 17, 2017: member

@rebroad UI evolve because of these changes, eventually recognising less is more. I also don't appreciate the icon there, but this is for sure something that can be improved. I would only show mutable state information in the status bar, and settings is not state.

IMO icon is unrecognisable in small size. Beside doesn't feel related to proxy at all (personal opinion).

mess110 force-pushed on Oct 24, 2017

in src/qt/bitcoingui.cpp:1103 in 19e68f7b61 outdated

1098 | +    if (proxy_enabled) {
1099 | +        QString ip_port_q = QString::fromStdString(ip_port);
1100 | +
1101 | +        labelProxyIcon->setPixmap(platformStyle->SingleColorIcon(":/icons/proxy").pixmap(STATUSBAR_ICONSIZE, STATUSBAR_ICONSIZE));
1102 | +        labelProxyIcon->setToolTip(tr("Proxy is <b>enabled</b>: %1").arg(ip_port_q));
1103 | +        labelProxyIcon->setEnabled(proxy_enabled);

promag commented at 3:18 PM on October 25, 2017:

Remove as it is already enabled?

in src/qt/bitcoingui.cpp:1142 in 19e68f7b61 outdated

1096 | +    bool proxy_enabled = clientModel->getProxyInfo(ip_port);
1097 | +
1098 | +    if (proxy_enabled) {
1099 | +        QString ip_port_q = QString::fromStdString(ip_port);
1100 | +
1101 | +        labelProxyIcon->setPixmap(platformStyle->SingleColorIcon(":/icons/proxy").pixmap(STATUSBAR_ICONSIZE, STATUSBAR_ICONSIZE));

promag commented at 3:18 PM on October 25, 2017:

If idea is to show/hide then the pixmap can be set when the label is created.

mess110 commented at 4:36 PM on October 25, 2017:

In the future, I plan to support a Tor icon, so I prefer to do this here

promag commented at 11:39 PM on October 25, 2017:

Even in that case it can't change in runtime right?

Sjors commented at 1:37 PM on February 8, 2018:

Although toggling proxy requires a restart, I prefer it if the UI doesn't assume that (to prevent a regression later). Not a show-stopper perse, but this code could confuse people.

Suggestion: add an if statement that checks if the icon has already been drawn. If so, call show().

in src/qt/clientmodel.cpp:273 in 19e68f7b61 outdated

 336 | @@ -336,3 +337,15 @@ void ClientModel::unsubscribeFromCoreSignals()
 337 |      uiInterface.NotifyBlockTip.disconnect(boost::bind(BlockTipChanged, this, _1, _2, false));
 338 |      uiInterface.NotifyHeaderTip.disconnect(boost::bind(BlockTipChanged, this, _1, _2, true));
 339 |  }
 340 | +
 341 | +bool ClientModel::getProxyInfo(std::string& ip_port) const

promag commented at 3:20 PM on October 25, 2017:

Nit, some may prefer std::pair<bool, std::string> ClientModel::getProxyInfo() const;, both work for me.

mess110 commented at 4:37 PM on October 25, 2017:

Gonna go with what is currently implemented

promag commented at 3:22 PM on October 25, 2017: member

IMO the icon could be improved. Not sure if it should be visible only when the proxy is enabled.

Edit: Feels better without the disabled proxy icon.

mess110 force-pushed on Oct 25, 2017

mess110 commented at 4:39 PM on October 25, 2017: contributor

@promag yea, I agree, the icon needs improving. Will come up with something better

Sjors commented at 11:49 AM on November 9, 2017: member

The icons at the bottom are already quite confusing. Some allow the user to take action (e.g. BTC), another does not allow action and never chances (HD). The network icon allows the user to take action, but unlike the BTC icon doesn't first show a dropdown menu, but instead disables your network without warning.

Instead of adding another icon, I suggest turning the connection icon into something that opens a dialog, where you inspect and change network settings (including disconnecting, using a proxy or tor). The shape or color of the icon could give a hint as to the current state (e.g. green for Tor), but once the user clicks on it they would get the full information. That way the icon doesn't need to be perfect.

luke-jr commented at 2:58 PM on November 10, 2017: member

I wonder if a small "P" (and later "T" for Tor) in the connection icon would do the trick...

mess110 force-pushed on Nov 11, 2017

mess110 commented at 10:50 AM on November 11, 2017: contributor

I updated the proposal to use an icon form http://s-ings.com/typicons/ as @jonasschnelli suggested (decided to try this instead of creating an svg with P in it - curious about feedback). The set also contains a t icon for future Tor PR. @Sjors yea, the icons can be a bit confusing. This is why in the first version of this PR, I created proxy enabled/disabled similar to how the network icon behaves. However, each icon has slightly different behavior and needs. I think the security benefit of seeing the proxy icon is worth it though.

luke-jr commented at 5:02 PM on November 11, 2017: member

This indicates proxy for me, even though I don't use a proxy (but I have Tor enabled with -onion).

luke-jr commented at 5:19 PM on November 11, 2017: member

I wonder if a small "P" (and later "T" for Tor) in the connection icon would do the trick...

Implemented this. It's too small. :(

laanwj commented at 12:06 PM on November 13, 2017: member

I think the security benefit of seeing the proxy icon is worth it though.

I tend to agree. Let's be pragmatic about ACKing this when the functionality works, aesthetics can always be improved later.

jonasschnelli commented at 11:37 PM on November 29, 2017: contributor

The current P icon is probably the "Pintrest" logo. Feels wrong. But agree, lets try to separate functionality and aesthetics. @mess110: maybe rebase, make a pure "P" icon (just a default font). This PR should not go under in icon discussions.

mess110 force-pushed on Nov 30, 2017

mess110 commented at 12:38 AM on November 30, 2017: contributor

@jonasschnelli agree and thanks. I rebased and added a proxy icon (as a simple P)

Should have some time over the weekend to do the "T" for Tor

jonasschnelli commented at 12:59 AM on November 30, 2017: contributor

The "T" (tor) indicator may be more complicated. A false indication can be fatal... and unsure how to reliable detect if we are connecting via TOR

luke-jr commented at 1:16 AM on November 30, 2017: member

This indicates proxy for me, even though I don't use a proxy (but I have Tor enabled with -onion).

Reminder this bug persists (but in theory can be easiest solved by having the Tor icon).

mess110 commented at 1:30 AM on November 30, 2017: contributor

The way I was thinking of detecting if Tor is enabled is if the n variable from https://github.com/bitcoin/bitcoin/pull/11491/files#diff-2c51f64a3430117d2f6c7cb55355be66R343 is equal to https://github.com/bitcoin/bitcoin/blob/master/src/netaddress.h#L24 (in this case 3).

Would that be enough?

If not, for this PR I can just check that all n are set which does not happen for -onion (thx @luke-jr for the tip)

lmlsna commented at 4:44 AM on December 7, 2017: contributor

A possible kludgey check if tor is actually up and running is to just send a dummy HTTP request directly to the tor socks proxy port. If tor is running, it will politely respond with a HTTP/1.0 501 Tor is not an HTTP Proxy, if it isn't, it won't.

This misses the possibility that the tor client is running but unable to connect, however that would result in a broken connection (as opposed to an accidental clearnet connection). However, I can't think of any way to verify tor is actually working other than either asking tor-arm (which might not be there) or requesting https://check.torproject.org/ through the proxy (like the TBB does).

mess110 force-pushed on Dec 18, 2017

mess110 commented at 9:07 PM on December 18, 2017: contributor

NET_MAX is 5. I checked to see what I get from GetProxy for different options:

	0	1	2	3	4
with no options	false	false	false	false	false
with -proxy	false	true	true	true	false
with gui default proxy	false	true	true	true	false
with gui separate proxy	false	false	false	true	false
with both gui options	false	true	true	true	false
with -onion	false	false	false	true	false
with -onion and all gui options	false	true	true	true	false
with -onion and gui default proxy	false	true	true	true	false

Looking at the table above, I can not reliably determine if the proxy icon should be shown by checking the return value of GetProxy. Any advice on how to proceed?

luke-jr commented at 10:06 AM on January 3, 2018: member

Note that the network ids aren't just arbitrary numbers - they have meanings (defined in netaddress.h).

Anyhow, "gui separate proxy" is a Tor-specific option. So we shouldn't show "proxy" for that either.

"Proxy" icon should probably only be shown if NET_IPV4 and NET_IPV6 have a GetProxy result.

The Tor icon might be more complicated. Ideally, clicking on it would show a popup with the hidden service address and QR Code, so mobile wallets can autoconfigure. But I guess that's out-of-scope for this PR.

mess110 force-pushed on Jan 15, 2018

mess110 commented at 8:51 PM on January 15, 2018: contributor

Rebased and made sure the proxy icon is shown if NET_IPV4 and NET_IPV6 have a result.

For the tooltip, I am showing ToStringIPPort for NET_IPV4. I couldn't get an ipv6 to work with the -proxy argument. Also, ToStringIPPort for NET_IPV6 returns an ipv4 format.

mess110 commented at 1:17 AM on February 8, 2018: contributor

@luke-jr @promag @MeshCollider @jonasschnelli @wodry @rebroad @Sjors @laanwj @lmlsna can you please review this patch? Hope it doesn't get too buried. Thank you.

in contrib/debian/copyright:80 in d4b6d92f53 outdated

  75 | @@ -76,8 +76,8 @@ Comment:
  76 |  
  77 |  Files: src/qt/res/icons/clock*.png
  78 |         src/qt/res/icons/eye_*.png
  79 | -       src/qt/res/icons/verify.png
  80 |         src/qt/res/icons/tx_in*.png
  81 | +       src/qt/res/icons/verify.png

Sjors commented at 1:27 PM on February 8, 2018:

Nit: why does git think something changed here?

mess110 commented at 9:01 PM on April 26, 2018:

I moved one element so it is alphabetical

promag commented at 7:24 PM on May 15, 2018:

nit, unrelated change. If you happen to push again consider removing this change.

Sjors commented at 1:42 PM on February 8, 2018: member

Tested through QT settings and -proxy launch flag.

The current silent failure (e.g. if you set a non existent proxy address) is suboptimal. Perhaps a future PR can put a red cross through the P icon to show there's a problem.

Adding a test would be nice.

mess110 force-pushed on Apr 26, 2018

laanwj commented at 12:10 PM on May 15, 2018: member

Tested ACK 35d50aa833cea53f9c6195987256351f4b1c710a

I don't think it's a good idea to use a "P" icon because the word for "Proxy" is going to be different between languages. It should be replaced with a symbol. But that can be changed later, it should not hold up the merge.

in src/qt/clientmodel.cpp:276 in 35d50aa833 outdated

 268 | @@ -268,3 +269,13 @@ void ClientModel::unsubscribeFromCoreSignals()
 269 |      m_handler_notify_block_tip->disconnect();
 270 |      m_handler_notify_header_tip->disconnect();
 271 |  }
 272 | +
 273 | +bool ClientModel::getProxyInfo(std::string& ip_port) const
 274 | +{
 275 | +    proxyType ipv4, ipv6;
 276 | +    if (GetProxy((Network) 1, ipv4) && GetProxy((Network) 2, ipv6)) {

ryanofsky commented at 6:43 PM on May 15, 2018:

Could call m_node.getProxy() here and drop netbase.h include above. This would be a little more future-proof with #10102.

mess110 commented at 8:12 PM on May 15, 2018:

netbase.h is still needed for proxyType, but I can change to m_node.getProxy

ryanofsky commented at 6:47 PM on May 15, 2018: member

utACK 35d50aa833cea53f9c6195987256351f4b1c710a

in src/qt/bitcoingui.cpp:86 in 35d50aa833 outdated

  82 | @@ -83,6 +83,7 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
  83 |      unitDisplayControl(0),
  84 |      labelWalletEncryptionIcon(0),
  85 |      labelWalletHDStatusIcon(0),
  86 | +    labelProxyIcon(0),

promag commented at 7:25 PM on May 15, 2018:

nit, could be initialized in the declaration as per developer notes.

in src/qt/bitcoingui.cpp:1140 in 35d50aa833 outdated

1137 | +    bool proxy_enabled = clientModel->getProxyInfo(ip_port);
1138 | +
1139 | +    if (proxy_enabled) {
1140 | +        QString ip_port_q = QString::fromStdString(ip_port);
1141 | +
1142 | +        if (labelProxyIcon->pixmap() == 0) {

promag commented at 7:28 PM on May 15, 2018:

nit, IMO the label pixmap could be initialized when the label is created above.

mess110 commented at 8:19 PM on May 15, 2018:

This is consistent with labelWalletHDStatusIcon, so I will prefer the approach taken

in src/qt/bitcoingui.cpp:1140 in 35d50aa833 outdated

1135 | +{
1136 | +    std::string ip_port;
1137 | +    bool proxy_enabled = clientModel->getProxyInfo(ip_port);
1138 | +
1139 | +    if (proxy_enabled) {
1140 | +        QString ip_port_q = QString::fromStdString(ip_port);

promag commented at 7:29 PM on May 15, 2018:

nit, could inline this below.

promag commented at 7:33 PM on May 15, 2018: member

utACK 35d50aa.

I have a couple of nits for you to consider or feel free to ignore them and keep the above ACKs.

mess110 force-pushed on May 15, 2018

mess110 commented at 8:23 PM on May 15, 2018: contributor

Thanks for the reviews.

I inlined the string creation and using m_node.getProxy

[gui] Add proxy icon in statusbar 73cd5b25b9

mess110 force-pushed on May 15, 2018

jonasschnelli commented at 8:07 AM on May 16, 2018: contributor

Tested ACK 73cd5b25b901e17d24bd2ebeb7fe334f6092d3d8 https://bitcoin.jonasschnelli.ch/build/611

jonasschnelli merged this on May 16, 2018

jonasschnelli closed this on May 16, 2018

jonasschnelli referenced this in commit 40c34a0a29 on May 16, 2018

mess110 deleted the branch on May 16, 2018

mess110 referenced this in commit 9fb4abbebf on May 16, 2018

mess110 referenced this in commit 015500c732 on May 16, 2018

mess110 referenced this in commit a73098955f on May 16, 2018

mess110 referenced this in commit 4884fe5168 on Jul 23, 2018

laanwj referenced this in commit 2a583406c0 on Aug 20, 2018

ptschip referenced this in commit bc8ad5ec1d on Oct 22, 2019

jasonbcox referenced this in commit 2d293e74c9 on Oct 25, 2019

UdjinM6 referenced this in commit 47818f9857 on Jun 18, 2021

TheArbitrator referenced this in commit ec9086dbfd on Jun 21, 2021

UdjinM6 referenced this in commit f610f33298 on Jun 24, 2021

UdjinM6 referenced this in commit b7fb0eb438 on Jun 29, 2021

UdjinM6 referenced this in commit 7ab4d09153 on Jun 29, 2021

Munkybooty referenced this in commit 1127b318ef on Jun 30, 2021

UdjinM6 referenced this in commit 71b7cd3fcd on Jul 2, 2021

UdjinM6 referenced this in commit 2cac1b70b0 on Jul 4, 2021

UdjinM6 referenced this in commit 5582a6796a on Jul 6, 2021

Munkybooty referenced this in commit 3cc0bd45e7 on Sep 8, 2021

DrahtBot locked this on Sep 8, 2021