-
Notifications
You must be signed in to change notification settings - Fork 38.6k
Chainparams: Generic chainparam selection with -chain=<strNetworkID> #5229
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/chainparams.h
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't actually return anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will correct it.
|
The value of removing -testnet and -regtest is unclear. Also, please test and make sure bitcoin-cli and bitcoin-tx utilities remain consistent with bitcoind. |
src/chainparamsbase.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 times "testnet"? Guess this should read "regtest".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ops, I will correct it.
|
@jgarzik -testnet and -regtest still work, I'm just logging a warning to discourage its use. |
|
NACK on removing/deprecating -testnet and -regtest. Selecting the network is a primary function of the client, so it can be a top-level option. |
|
I think it has been a misunderstanding. This PR doesn't disable -testnet and -regtest, just allows using -network instead. #5238 does disable them. |
|
I just don't want them to be deprecated. People rely on them, and they work fine. Forcing people to use another option seems like a boondoggle. I'm not against also adding |
|
So are you against the warning messages? |
|
Indeed, remove the warning messages. It makes sense to keep those quick shortcut option to select regtest and testnet (but not for more obscure networks like unittest). |
|
Warnings removed. I'm fine just not adding new network shortcuts from now on. |
|
Should I squash already? |
|
NACK on removing/deprecating -testnet and -regtest. |
|
@petertodd I told @jgarzik and then @Diapolo that this does not disable -testnet or -regtest. |
33371b6 to
83eb80a
Compare
|
Closing until #5598 gets merged. |
|
Reopened and rebased after #5598 has been merged. Now it's easier to review. |
|
Rather that having the argument be "-network" why don't we rename it to "-chain"? Network is a term that's already overloaded in multiple contexts. After all, we called it chain params rather than network params. On 7 January 2015 18:48:08 GMT-05:00, "Jorge Timón" [email protected] wrote:
|
|
Added 3 commits to hopefully make it more interesting. |
|
Removed jtimon@ac1c555 since is now a proposal for the trivial branch (theuni#27). To better read commit "Chainparams: The hash of the genesis block it's the genesis checkpoint and chain id", please rgrep hashGenesisBlock All mentions of it are whether to:
Why the heck are we trying to check the genesis block in the first place? But what about checkpoints? The logic of checkpoints can always safely rely on the first checkpoint being true. Therefore, we should not:
|
1b5fda1 to
424a85c
Compare
|
Sorry, pushed too fast. Now I use containers instead of those ugly ClearSelectedParams functions, and the first commit actually compiles (now all of them do). |
c7db474 to
c535d85
Compare
|
Closing for now, I will open a version with some of the changes but without adding the new option. |
…ants (suggested by Wladimir)
… CBaseChainParams::REGTEST s/"regtest"/CBaseChainParams::REGTEST s/"main"/CBaseChainParams::MAIN s/"testnet"/CBaseChainParams::TESTNET
…(and user-facing testnet to "testnet3")
…of -testnet and -regtest
|
This is the promised version with some of the improvements but without adding the new option: #6229 |
Rebased by Pieter Wuille. Cleanup by Matt Corallo.
Continues #4804.
Deprecate -testnet and -regtest in favor of -network=testnet and -network=regtest.
When the old args are used a warning will be logged.
The repetition of the invalid combination error string is also avoided.
Additionally, the chainparam networkID is removed since it's not used anymore (as it shouldn't).