Remove CBaseChainParams::NetworkID() #4804

pull jtimon wants to merge 5 commits into bitcoin:master from jtimon:chainparams3 changing 4 files +33 −22

jtimon commented at 10:45 PM on August 31, 2014: contributor

~~Continues #4802 and~~ Includes and closes #4803 Completes the set of PRs #3823 #3824 #4333 #4801 #4802 #4803
jtimon force-pushed on Aug 31, 2014

in src/chainparams.h:None in adeefa2921 outdated

  70 | @@ -69,12 +71,15 @@ class CChainParams
  71 |      /* Make miner stop after a block is found. In RPC, don't return
  72 |       * until nGenProcLimit blocks are generated */
  73 |      bool MineBlocksOnDemand() const { return fMineBlocksOnDemand; }
  74 | -    CBaseChainParams::Network NetworkID() const { return networkID; }
  75 | +    bool GuiTestColor() const { return fGuiTestColor; }

laanwj commented at 7:55 AM on September 1, 2014:

(huh I thought I commented this before but I lost my comment) Don't move this GUI decision to the core code. The GUI needs to be able to make the decision for network-based theming based on some identifier of the chain. If you insist on removing the enumeration you could match on NetworkIDString().

jtimon cross-referenced this on Sep 1, 2014 from issue Reserve only one network specific cached path per session by jtimon
jtimon force-pushed on Sep 1, 2014
jtimon cross-referenced this on Sep 1, 2014 from issue Remove CChainParams::NetworkID() by jtimon
jtimon force-pushed on Sep 1, 2014
jtimon commented at 7:16 PM on September 1, 2014: contributor
@laanwj continuing #4803 discussion here. Not including #4802 anymore: although they don't look like it they're completely independent.

I tested the following in this order:
```
./bitcoind -debug -printtoconsole
./bitcoind -testnet -debug -printtoconsole
./bitcoind -regtest -gen=1 -genproclimit=5 -debug -printtoconsole
./qt/bitcoin-qt -debug -printtoconsole
./qt/bitcoin-qt -testnet -debug -printtoconsole
./qt/bitcoin-qt -regtest -gen=1 -genproclimit=5 -debug -printtoconsole
```
Then repeated in the same order and everything looked right. Feel free to propose another testing procedure.
laanwj added the label Improvement on Sep 3, 2014
jtimon force-pushed on Sep 13, 2014
laanwj commented at 2:02 PM on September 15, 2014: member
@jtimon also test cases like
- -regtest is provided in bitcoin.conf
- -datadir is provided in bitcoin.conf
Reserve only one network specific cached path per session f297479a19
Remove CBaseChainParams::NetworkID() 3fdb9e8c15
jtimon force-pushed on Sep 21, 2014
BitcoinPullTester commented at 7:40 PM on September 21, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4804_3fdb9e8c159a2bc3ac853b469dae9ba0ecf739f3/ 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.
jtimon commented at 1:01 AM on September 23, 2014: contributor

Tested regtest and testnet provided in bitcoin.conf and then all 3 modes providing a different datadir in bitcoin.conf. Then I found out that if you set a different dataset as parameter the mode specified in ~/.bitcoin/bitcoin.conf are obviously ignored and set them as parameters. I don't know of nay other combination, but please, let me know if there is.
jtimon commented at 5:19 PM on October 9, 2014: contributor

@TheBlueMatt's nit solved with a fix commit to be squashed on the previous one.
jtimon force-pushed on Oct 10, 2014
SQUASHME: NetworkIdFromCommandLine() function instead of method b796cb084b
jtimon force-pushed on Oct 10, 2014
SQUASHME: fix "Reserve only one network specific cached path per session" ebdb9ff639

in src/util.cpp:None in b796cb084b outdated

 394 | @@ -395,7 +395,7 @@ boost::filesystem::path GetDefaultDataDir()
 395 |  #endif
 396 |  }
 397 |  
 398 | -static boost::filesystem::path pathCached[CBaseChainParams::MAX_NETWORK_TYPES+1];
 399 | +static boost::filesystem::path pathCached[2];

laanwj commented at 10:09 AM on October 11, 2014:

Small readability nit: Instead of having an array here at all, lets

static boost::filesystem::path pathCached;
static boost::filesystem::path pathCachedNetSpecific;

then below

fs::path &path = fNetSpecific ? pathCachedNetSpecific : pathCached;

and

pathCached = boost::filesystem::path();
pathCachedNetSpecific = boost::filesystem::path();

jtimon commented at 6:57 PM on October 11, 2014: contributor

@laanwj added you proposed changes to an additional commit to be squashed on the first commit.
laanwj commented at 11:31 AM on October 13, 2014: member

Looks good to me now. Going to test.
makhmet commented at 11:35 AM on October 13, 2014: none

Выглядит хорошо для меня сейчас. Собираюсь проверить.

2014-10-13 15:31 GMT+04:00 Wladimir J. van der Laan < notifications@github.com>:

Looks good to me now. Going to test.

— Reply to this email directly or view it on GitHub #4804 (comment).
TheBlueMatt commented at 7:10 AM on October 16, 2014: contributor

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

Tested ACK Commithash ebdb9ff63992fe685688f6d47cd1f3a2938ee607 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2

iQIcBAEBAgAGBQJUP29AAAoJEIm7uGY+LmXOmOMP/jIr1xBZkcgYatiWw5QEdCM1 yI1fyXK9mo/ro6F623PcWMKvA6jDvVQe5N2HhjaGL0DC4oows/8lvUrQEq2yib1g vKsQbZaN2gUP2figToeu1Soz8++kALeU9cTPCA7xAJL8zqM4dmftS0LTgt//q3rN 6kbEpnURfbOygsCN61+/VgfOSpZU39iOZ0xdntzdSMl/omcQMbPRgP21VmmoE5ue xk/kUhwSUsa3YMN774sSAiYW8Oixpda0CoEeDzRcew8ecQqIh7DPyJWzdzaVX9V7 Ty7aNT6EJ6cBuRX1EvXEtdDfz1JeXyGPHUxucI5uHsPc8g6CmHINgtP40rq84qNK +nO3tyXoU+io6e+1ZlLPf50Uzij2D4h2r+fO3bQz99P2lCvcGx/9/9W0oL7R5all IKFPJt3MdZu+jPfMjdKcVLzrwpnjVbAqfmg1YOc12HB1bS/iQtlqr/qB+zszycLs x4CbMPy81P5oV2pEQGujWw+s8VKGrPskhgLTLxcWni/bKDru4DjZyZOqswat0AeO dNTwd7Tbjsc93w4edDnfkLraqjVPUtFBLaPsfgsOHYcLT2EwqO9GFmN/zo9HbHCb cBvK2xSUNhtKXb5oUSGNkXHUO9igj3fY5MQ0aLAzfySwdjafNdMcXDWBHv/rsJKl Cbh5s0wj5fTsUwl3s4cu =ppDu -----END PGP SIGNATURE-----

in src/chainparams.cpp:None in ebdb9ff639 outdated

 239 | @@ -240,10 +240,13 @@ void SelectParams(CBaseChainParams::Network network) {
 240 |      pCurrentParams = &Params(network);
 241 |  }
 242 |  
 243 | -bool SelectParamsFromCommandLine() {
 244 | -    if (!SelectBaseParamsFromCommandLine())
 245 | +bool SelectParamsFromCommandLine()

TheBlueMatt commented at 7:14 AM on October 16, 2014:

Actually, wait...you have an extra space at the end of this line...

in src/chainparamsbase.cpp:None in ebdb9ff639 outdated

  97 | +    if (fTestNet)
  98 | +        return CBaseChainParams::TESTNET;
  99 | +    return CBaseChainParams::MAIN;
 100 | +}
 101 | +
 102 | +bool SelectBaseParamsFromCommandLine()

TheBlueMatt commented at 7:14 AM on October 16, 2014:

...and this one too.

laanwj commented at 7:16 AM on October 16, 2014: member

@TheBlueMatt you forgot to write it in Russian :)

Tested ACK.
squashme on 3fdb9e8c (Bluematt's bikeshedding) ca3ce0fa03
jtimon commented at 8:50 PM on October 16, 2014: contributor

@TheBlueMatt 's nit solved
laanwj referenced this in commit 494ff05a4c on Oct 17, 2014
laanwj merged this on Oct 17, 2014
laanwj closed this on Oct 17, 2014
jtimon cross-referenced this on Nov 6, 2014 from issue Chainparams: Generic chainparam selection with -chain=<strNetworkID> by jtimon
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#3823 MainNet is the special case for the GUI #3824 Improvements in CChainParams #4333 Replace std::string PaymentServer::mapNetworkIdToName and CChainParams::...#4801 Move checkpoint data selection to chainparams #4802 Remove CChainParams::NetworkID()#4803 Reserve only one network specific cached path per session #5229 Chainparams: Generic chainparam selection with -chain=<strNetworkID>