This pull adds per-network information to the getnetworkinfo RPC call:
Inspired by #2575.
(After these changes it is also allowed to use -onlynet=onion instead of -onlynet=tor, as I like it better to call the network onion)
Just an alias for onlynet=tor, but matches the new name
of the proxy option -onion= better.
Allows other parts of the program to query for reachable
status of a network. Similar to IsLimited(net).
Returns the network name for an Network enum.
Only item I see is "proxy" changing in JSON output -- do we care about backwards compat here?
ACK otherwise
getnetworkinfo is brand new, so not too many people will rely on the current interface yet and still use getinfo if they want 'the' proxy. I'm fine with adding it back for backwards compatibility, but IMO it gives the wrong idea that there is only one proxy.
370 | @@ -351,7 +371,13 @@ Value getnetworkinfo(const Array& params, bool fHelp) 371 | " \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n" 372 | " \"timeoffset\": xxxxx, (numeric) the time offset\n" 373 | " \"connections\": xxxxx, (numeric) the number of connections\n" 374 | - " \"proxy\": \"host:port\", (string, optional) the proxy used by the server\n" 375 | + " \"networks\": [ (object) information per network\n"
Nit: Misses a , after [ and it's an (array) not (object).
No, there should be no , after [.
Good catch on the array versus object though.
370 | @@ -351,7 +371,13 @@ Value getnetworkinfo(const Array& params, bool fHelp) 371 | " \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n" 372 | " \"timeoffset\": xxxxx, (numeric) the time offset\n" 373 | " \"connections\": xxxxx, (numeric) the number of connections\n" 374 | - " \"proxy\": \"host:port\", (string, optional) the proxy used by the server\n" 375 | + " \"networks\": [ (object) information per network\n" 376 | + " \"name\": \"xxx\" (string) network (ipv4, ipv6, onion)\n"
Nit: Also misses a , at the end.
55 | +std::string GetNetworkName(enum Network net) { 56 | + switch(net) 57 | + { 58 | + case NET_IPV4: return "ipv4"; 59 | + case NET_IPV6: return "ipv6"; 60 | + case NET_TOR: return "onion";
I'm not sure I like calling this onion, but perhaps it's already enough mixed (Tor / Onion and internal names in the code), so don't care ^^.
Well we changed the proxy option to -onion=, so IMO there is no other choice.
163 | @@ -164,6 +164,7 @@ class CService : public CNetAddr 164 | typedef CService proxyType; 165 | 166 | enum Network ParseNetwork(std::string net); 167 | +std::string GetNetworkName(enum Network net);
Return type could be const?
Defining const for return-as-value serves no purpose (would be different when returning a reference or pointer).
Looks good, after fixing the nits this an untested ACK ;).
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4605_0590a54818ceb6c907c7ff6ff2f1a28d5ef383b2/ 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.
This commit adds per-network information to the
getnetworkinfo RPC call:
- Is the network limited?
- Is the network reachable
- Which proxy is used for this network, if any
Inspired by #2575.
370 | @@ -351,7 +371,13 @@ Value getnetworkinfo(const Array& params, bool fHelp) 371 | " \"localservices\": \"xxxxxxxxxxxxxxxx\", (string) the services we offer to the network\n" 372 | " \"timeoffset\": xxxxx, (numeric) the time offset\n" 373 | " \"connections\": xxxxx, (numeric) the number of connections\n" 374 | - " \"proxy\": \"host:port\", (string, optional) the proxy used by the server\n" 375 | + " \"networks\": [ (array) information per network\n" 376 | + " \"name\": \"xxx\", (string) network (ipv4, ipv6, onion)\n"
Another small nit: In the help message we have IPv4, IPv6 <b>or</b> Onion.
I prefer to use lowercase here. People will be using case-sensitive comparisons on this output, and thus using only lowercase rules out one source of typos.
I know that the -help documentation shows the network names uppercased, which is IMO a bit strange, but we do case-insensitive comparison so for us it doesn't matter.
I wasn't talking about uppercase, just , vs. or ;).