Replace std::string PaymentServer::mapNetworkIdToName and CChainParams::… #4333

1. 
   jtimon commented at 11:15 am on June 12, 2014: contributor

   related to #4326 and #3824
2. in src/rpcmisc.cpp: in 81849b1717 outdated

   72@@ -73,7 +73,7 @@ Value getinfo(const Array& params, bool fHelp)
   73     obj.push_back(Pair("connections",   (int)vNodes.size()));
   74     obj.push_back(Pair("proxy",         (proxy.first.IsValid() ? proxy.first.ToStringIPPort() : string())));
   75     obj.push_back(Pair("difficulty",    (double)GetDifficulty()));
   76-    obj.push_back(Pair("testnet",       Params().RPCisTestNet()));
   77+    obj.push_back(Pair("network",       Params().NetworkIDString()));
   

   ---

   ---

   

   laanwj commented at 11:20 am on June 12, 2014:

   Please don’t change the getinfo call, too many people rely on its current behavior. As it’s a random grabbag of information from different parts, it’s going to be deprecated at some point in favor of the more specific get*info calls.
3. 
   Diapolo commented at 11:21 am on June 12, 2014: none

   Can you rebase this onto #4326?
4. 
   jtimon commented at 11:41 am on June 12, 2014: contributor

   Rebased on top of Diapolo:clientmodel_networkname So maybe I should maintain the testnet field and add the network one? Maybe I can leave “Params().NetworkID() == CChainParams::TESTNET” there and still get rid of the RPCisTestnet param…
5. 
   laanwj commented at 11:44 am on June 12, 2014: member

   @jtimon Yes, Params().NetworkID() == CChainParams::TESTNET is clearer than RPCIsTestnet. But please keep the RPC interface the same for now.
6. 
   jtimon commented at 11:52 am on June 12, 2014: contributor

   Oh, I was coding it and didn’t read you. So should I just cancel the last commit then?
7. 
   laanwj commented at 11:55 am on June 12, 2014: member

   Yes. We can remove them at a later time when there is a a RPC overhaul with other backwards-compat breaking changes.
8. 
   jtimon commented at 12:00 pm on June 12, 2014: contributor

   Adding the “network” field doesn’t seem backwards incompatible, but it allows people to start using that instead of “testnet”, which could be declared deprecated or something to encourage people to move already in preparation for that RPC overhaul. That’s why I ask if I should remove https://github.com/jtimon/bitcoin/commit/bdb2f8ee844b20f40397c2cb2937cd7d23a2d247

   Thoughts?

9. 
   laanwj commented at 12:02 pm on June 12, 2014: member

   I’m fine with adding it to getmininginfo. But don’t touch getinfo. It should die.
10. 
    Diapolo commented at 12:04 pm on June 12, 2014: none

    Perhaps 0.10 is a good release for killing getinfo :)?
11. 
    laanwj commented at 12:07 pm on June 12, 2014: member

    @Diapolo I’d rather wait another major release (eg: 0.10 deprecates it, 0.11 removes it). But I don’t want that discussion now.
12. 
    jtimon commented at 12:17 pm on June 12, 2014: contributor

    Ok, done. I didn’t know getinfo was meant to die. Maybe it makes sense to add it in other places besides getmininginfo? I only considered those two because only them had the “testnet” field.
13. 
    laanwj commented at 12:21 pm on June 12, 2014: member

    Well getblockchaininfo and getmininginfo have it now, I’d guess that is enough.
14. 
    jtimon commented at 12:36 pm on June 12, 2014: contributor

    oh, I see. Probably I should use “chain” instead of “network” and the help could be uniform too. Maybe it should be: " "chain": "xxxx", (string) current network name as defined in BIP70 (main, test, regtest)\n" instead of " "chain": "xxxx", (string) current chain (main, testnet3, regtest)\n"

    but the change in #4326 doesn’t seem backwards compatible (it’s going to return “test” instead of “testnet3” [by the way, that shouldn’t had changed in bip70 given that this “chain” was here already]). So maybe it’s better to just keep “chain” as it was, and include the new “network” parameter.

    Thoughts?

15. 
    laanwj commented at 12:45 pm on June 12, 2014: member

    @jtimon chain is very new, there is IMO no problem changing the values there.
16. 
    jtimon commented at 12:48 pm on June 12, 2014: contributor

    Good. Then I will use chain as well, and correct the help info in getblockchaininfo
17. 
    laanwj commented at 8:00 am on June 17, 2014: member

    Needs rebase
18. Get rid of Params().RPCisTestNet() b82b7ec3dc
19. Add "chain" to getmininginfo, improve help in getblockchaininfo f6984e8141
20. 
    jtimon commented at 11:14 am on June 17, 2014: contributor

    Rebased
21. 
    BitcoinPullTester commented at 11:46 am on June 17, 2014: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f6984e814134d2383e1353ce8aa951649f5fcbd2 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.
22. laanwj merged this on Jun 17, 2014
23. laanwj closed this on Jun 17, 2014

    ---

24. laanwj referenced this in commit 2151419b1b on Jun 17, 2014
25. jtimon deleted the branch on Jun 18, 2014
26. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon laanwj Diapolo BitcoinPullTester