- rework the proxy handling in init to cover more cases and work more thoroughly
- extend RPC call getnetwork info to see if IPv6 and Tor proxy is the default proxy
See #2575 for discussion!
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4871_2711cda5b8c54efb2164d7bb3593b3db50f8e15c/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
Rebased and removed an obsolete c_str().
360 | obj.push_back(Pair("name", GetNetworkName(network)));
361 | obj.push_back(Pair("limited", IsLimited(network)));
362 | obj.push_back(Pair("reachable", IsReachable(network)));
363 | obj.push_back(Pair("proxy", proxy.IsValid() ? proxy.ToStringIPPort() : string()));
364 | + if (network != NET_IPV4)
365 | + obj.push_back(Pair("proxy-from-default", proxy == proxyDefault));
As said before - I'm still not convinced that proxy-from-default is useful information. Clients requesting from RPC just want to know the currently effective state, it shouldn't matter how it came that way. For troubleshooting command line argument interaction there's always debug.log.
Alright, removing this will finally make the pull get merged then?
The main problem here is that we don't have testcases with proxy, so this needs to be manually tested. Can you list what you tested?
Needs rebase
Rebased, needs review / testing.
To be clear what this is blocked on: We need (RPC) tests for the proxy functionality, before we can merge changes like this.
- rework the proxy handling in init to cover more cases and work more
thoroughly
377 | obj.push_back(Pair("limited", IsLimited(network)));
378 | obj.push_back(Pair("reachable", IsReachable(network)));
379 | obj.push_back(Pair("proxy", proxy.IsValid() ? proxy.proxy.ToStringIPPort() : string()));
380 | obj.push_back(Pair("proxy_randomize_credentials", proxy.randomize_credentials));
381 | + if (network != NET_IPV4)
382 | + obj.push_back(Pair("proxy-from-default", proxy.proxy == proxyDefault.proxy));
Let's remove proxy-from-default, it's not useful information and will always be true anyway according to the above code.
I'll remove it :).
I'm terribly sorry sorry for holding this up so long, but I'm not convinced that the new logic is any better.
The logical flow here sounds to me:
-proxy, apply to all nets, as well as the name proxy-onion, apply to NET_TOR. This overrides the default set in the first step, if anyAll errors during parsing of either setting should be fatal, to avoid that the user is running with different settings than they expect.
This is nearer to the original code than yours. I certainly think the code could be simplified and made more clear, what do you think about:
bool proxyRandomize = GetBoolArg("-proxyrandomize", true);
// -proxy sets a proxy for all outgoing network traffic
// -noproxy (or -proxy=0) as well as the empty string can be used to not set a proxy, this is the default
std::string proxyArg = GetArg("-proxy", "");
if (proxyArg != "" && proxyArg != "0") {
proxyType addrProxy = proxyType(CService(proxyArg, 9050), proxyRandomize);
if (!addrProxy.IsValid())
return InitError(strprintf(_("Invalid -proxy address: '%s'"), proxyArg));
SetProxy(NET_IPV4, addrProxy);
SetProxy(NET_IPV6, addrProxy);
SetProxy(NET_TOR, addrProxy);
SetNameProxy(addrProxy);
SetReachable(NET_TOR); // by default, -proxy sets onion as reachable, unless -noonion later
}
// -onion can override normal proxy for .onion addresses
// -noonion (or -onion=0) disables connecting to .onion entirely
// An empty string is used to just not override the onion proxy
std::string onionArg = GetArg("-onion", "");
if (onionArg != "") {
if (onionArg == "0") { // Handle -noonion/-onion=0
SetReachable(NET_TOR, false); // set onions as unreachable
} else {
proxyType addrOnion;
addrOnion = proxyType(CService(onionArg, 9050), proxyRandomize);
if (!addrOnion.IsValid())
return InitError(strprintf(_("Invalid -onion address: '%s'"), onionArg));
SetProxy(NET_TOR, addrOnion);
SetReachable(NET_TOR);
}
}
This provides a straightforward flow, gets rid of .count() (which is what you needed for the GUI, as it makes it possible to override an earlier provided proxy option to nothing), as well as comments the different cases.