Remove CBaseChainParams::NetworkID() #4804

jtimon · 2014-08-31T22:45:18Z

~~Continues #4802 and~~ Includes and closes #4803
Completes the set of PRs #3823 #3824 #4333 #4801 #4802 #4803

laanwj · 2014-09-01T07:55:56Z

src/chainparams.h

(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 · 2014-09-01T19:16:23Z

@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 · 2014-09-15T14:02:35Z

@jtimon also test cases like

-regtest is provided in bitcoin.conf
-datadir is provided in bitcoin.conf

BitcoinPullTester · 2014-09-21T19:40:40Z

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 · 2014-09-23T01:01:46Z

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 · 2014-10-09T17:19:14Z

@TheBlueMatt's nit solved with a fix commit to be squashed on the previous one.

laanwj · 2014-10-11T10:09:57Z

src/util.cpp

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();

…ion"

jtimon · 2014-10-11T18:57:43Z

@laanwj added you proposed changes to an additional commit to be squashed on the first commit.

laanwj · 2014-10-13T11:31:31Z

Looks good to me now. Going to test.

makhmet · 2014-10-13T11:35:24Z

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

2014-10-13 15:31 GMT+04:00 Wladimir J. van der Laan <
[email protected]>:

Looks good to me now. Going to test.

—
Reply to this email directly or view it on GitHub
#4804 (comment).

TheBlueMatt · 2014-10-16T07:10:39Z

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

Tested ACK Commithash ebdb9ff
-----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-----

TheBlueMatt · 2014-10-16T07:14:45Z

src/chainparams.cpp

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

laanwj · 2014-10-16T07:16:59Z

@TheBlueMatt you forgot to write it in Russian :)

Tested ACK.

jtimon · 2014-10-16T20:50:57Z

@TheBlueMatt 's nit solved

Remove CBaseChainParams::NetworkID()

jtimon force-pushed the chainparams3 branch from ee9548a to adeefa2 Compare August 31, 2014 23:02

laanwj reviewed Sep 1, 2014
View reviewed changes

jtimon mentioned this pull request Sep 1, 2014

Reserve only one network specific cached path per session #4803

Closed

jtimon force-pushed the chainparams3 branch from adeefa2 to 3312a80 Compare September 1, 2014 09:30

jtimon mentioned this pull request Sep 1, 2014

Remove CChainParams::NetworkID() #4802

Merged

jtimon force-pushed the chainparams3 branch from 3312a80 to 9fa2c74 Compare September 1, 2014 19:07

laanwj added the Improvement label Sep 3, 2014

jtimon force-pushed the chainparams3 branch from 9fa2c74 to 908358b Compare September 13, 2014 20:38

jtimon added 2 commits September 21, 2014 21:21

Reserve only one network specific cached path per session

f297479

Remove CBaseChainParams::NetworkID()

3fdb9e8

jtimon force-pushed the chainparams3 branch from 908358b to 3fdb9e8 Compare September 21, 2014 19:25

jtimon force-pushed the chainparams3 branch from 77f5b16 to 83dc1d5 Compare October 10, 2014 09:08

SQUASHME: NetworkIdFromCommandLine() function instead of method

b796cb0

jtimon force-pushed the chainparams3 branch from 83dc1d5 to b796cb0 Compare October 10, 2014 10:05

laanwj reviewed Oct 11, 2014
View reviewed changes

SQUASHME: fix "Reserve only one network specific cached path per sess…

ebdb9ff

…ion"

TheBlueMatt reviewed Oct 16, 2014
View reviewed changes

src/chainparams.cpp Outdated

Copy link

Contributor

TheBlueMatt Oct 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

squashme on 3fdb9e8 (Bluematt's bikeshedding)

ca3ce0f

laanwj added a commit that referenced this pull request Oct 17, 2014

Merge pull request #4804 from jtimon/chainparams3

494ff05

Remove CBaseChainParams::NetworkID()

laanwj merged commit 494ff05 into bitcoin:master Oct 17, 2014

jtimon mentioned this pull request Nov 6, 2014

Chainparams: Generic chainparam selection with -chain=<strNetworkID> #5229

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Remove CBaseChainParams::NetworkID() #4804

Remove CBaseChainParams::NetworkID() #4804

Uh oh!

Conversation

jtimon commented Aug 31, 2014

Uh oh!

laanwj Sep 1, 2014

Choose a reason for hiding this comment

Uh oh!

jtimon commented Sep 1, 2014

Uh oh!

laanwj commented Sep 15, 2014

Uh oh!

BitcoinPullTester commented Sep 21, 2014

Uh oh!

jtimon commented Sep 23, 2014

Uh oh!

jtimon commented Oct 9, 2014

Uh oh!

laanwj Oct 11, 2014

Choose a reason for hiding this comment

Uh oh!

jtimon commented Oct 11, 2014

Uh oh!

laanwj commented Oct 13, 2014

Uh oh!

makhmet commented Oct 13, 2014

Uh oh!

TheBlueMatt commented Oct 16, 2014

Uh oh!

TheBlueMatt Oct 16, 2014

Choose a reason for hiding this comment

Uh oh!

laanwj commented Oct 16, 2014

Uh oh!

jtimon commented Oct 16, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants