Tests: Chainparams: Make regtest almost fully customizable by jtimon · Pull Request #17032 · bitcoin/bitcoin

jtimon · 2019-10-02T22:20:58Z

Almost all chain params fields are made configurable for regtest and documented in --help for debug.
Unlike #8994 this doesn't allow to create an arbitrary number of regtests with different genesis blocks, but it also doesn't touch consensus code nor changes all the tests from "regtest" to "regtest2" to make sure the custom genesis blocks work, so it should be much easier to review in comparison.

This is supposed to save people from having to create more set methods on CRegTestParams in the future.
This is also supposed to force programmers (that's us) to maintain a documentation readable with --help for any present or future chainparam and their modifications.

As a simple example, it is tested that regtest can disallow setting -acceptnonstdtxn=1, which is something, for example, signets may want some times.
If someone doesn't like the example chainparam field I picked for the test for whatever reason, anybody's welcomed to suggest some other field or set of fields to test in a new test.py file.
I'm happy to code it myself or take someone else's test as a replacement.

A long time ago, we talked about loading the chainparams from a file.
Well, combined with the existing section (by chain) feature for loading the config file, this provides that feature.

DrahtBot · 2019-10-03T20:30:48Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

BIP-325: Signet [consensus] #18267 (BIP-325: Signet [consensus] by kallewoof)
test: Change feature_config_args.py not to rely on strange regtest=0 behavior #17556 (test: Change feature_config_args.py not to rely on strange regtest=0 behavior by ryanofsky)
Chainparams: Rename IsTestChain() to AllowAcceptNonstd() #16770 (Chainparams: Rename IsTestChain() to AllowAcceptNonstd() by jtimon)
test: Set BIP34Height = 2 for regtest #16333 (test: Set BIP34Height = 2 for regtest by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2019-10-08T14:03:44Z

Concept ACK. Didn't look at the approach.

fjahr

Concept ACK. Did a light code review.

src/chainparams.cpp

jtimon · 2019-10-30T12:53:01Z

Rebased after #17306 was merged.
Fixed nit in #17306 (comment) here.

jtimon · 2020-02-19T20:41:33Z

Rebased

A rebased version of: bitcoin/bitcoin#17032 This adds consensus customization support to regtest.

Sjors

Concept ACK. Lightly tested 2aaf473 in a different project, by simulating a chain split caused by a different halving interval. It sets up two nodes, a regular one (A) and one (B) with -con_nsubsidyhalvinginterval=10. It generates a longer chain on A, which is rejected by B, and shows up as invalid in getchaintips. A test like that would be useful here.

You may also want to add tests that at least touch the other params, and e.g. make sure the node doesn't complain about unrecognized params.

Sjors · 2020-03-03T16:56:51Z

src/chainparamsbase.cpp

+    gArgs.AddArg("-assumed_blockchain_size", "Estimated current blockchain size (in GB) for UI purposes. Default 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-assumed_chain_state_size", "Estimated current chain state size (in GB) for UI purposes. Default 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-fdefaultconsistencychecks", "Whether -checkblockindex and -checkmempool are active by default or not. Consider using those options instead. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-frequirestandard", "Whether standard policy rules are applied in the local mempool by default. Consider using -acceptnonstdtxn=0 instead of changing the default. Default: 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);


Why even expose this option if -acceptnonstdtxn=0 does the trick?

Sjors · 2020-03-03T16:57:28Z

src/chainparamsbase.cpp

+    gArgs.AddArg("-assumed_chain_state_size", "Estimated current chain state size (in GB) for UI purposes. Default 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-fdefaultconsistencychecks", "Whether -checkblockindex and -checkmempool are active by default or not. Consider using those options instead. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-frequirestandard", "Whether standard policy rules are applied in the local mempool by default. Consider using -acceptnonstdtxn=0 instead of changing the default. Default: 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-is_test_chain", "Whether it's allowed to set -acceptnonstdtxn=0 for this chain or not. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);


This seems useless?

Sjors · 2020-03-03T16:58:08Z

src/chainparamsbase.cpp

+    gArgs.AddArg("-fdefaultconsistencychecks", "Whether -checkblockindex and -checkmempool are active by default or not. Consider using those options instead. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-frequirestandard", "Whether standard policy rules are applied in the local mempool by default. Consider using -acceptnonstdtxn=0 instead of changing the default. Default: 0 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-is_test_chain", "Whether it's allowed to set -acceptnonstdtxn=0 for this chain or not. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-is_mockable_chain", "Whether mocktime rpc calls are allowed or not. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);


Can you think of a use case where disabling mocktime on regtest makes sense?

Sjors · 2020-03-03T17:00:47Z

src/chainparamsbase.cpp

+    gArgs.AddArg("-is_test_chain", "Whether it's allowed to set -acceptnonstdtxn=0 for this chain or not. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-is_mockable_chain", "Whether mocktime rpc calls are allowed or not. Default: 1 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-bech32_hrp", "Human readable part for bech32 addresses. See BIP173 for more info. Default: bcrt (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
+    gArgs.AddArg("-pubkeyprefix", "Magic for base58 pubkeys. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);


Light preference for making this hex, to be consistent with -extpubkeyprefix and friends.

Sjors · 2020-03-03T17:06:33Z

test/functional/feature_config_args.py


        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
-            conf.write('regtest=0\n') # mainnet
+            conf.write('is_test_chain=0\n') # like mainnet


This seems unsafe. We really want to test with actual mainnet, see e.g. #17449.

jtimon · 2020-05-06T13:35:11Z

Thanks for all the reviews, really.
But I'm no longer interested in this.
Please, if anyone is interested, don't hesitate to take the branch and improve it and try, rewrite it or whatever.

DrahtBot added Tests Validation labels Oct 2, 2019

This was referenced Oct 3, 2019

Testchains: Introduce custom chain whose constructor... #8994

Closed

Testschains: Many regtests with different genesis and default datadir #17037

Closed

This was referenced Oct 8, 2019

Tests: Use self.chain instead of 'regtest' in all current tests #16681

Merged

Signet with #17037 on top kallewoof/bitcoin#10

Closed

fjahr reviewed Oct 28, 2019

View reviewed changes

src/chainparams.cpp Outdated Show resolved Hide resolved

src/chainparams.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Oct 30, 2019

jtimon force-pushed the b20-customizable-regtest branch from 6c54f7d to 56fd4b3 Compare October 30, 2019 12:51

DrahtBot removed the Needs rebase label Oct 30, 2019

DrahtBot added the Needs rebase label Feb 18, 2020

jtimon force-pushed the b20-customizable-regtest branch from 56fd4b3 to ff1d546 Compare February 19, 2020 20:39

jtimon added 2 commits February 19, 2020 21:49

Chainparams: Make regtest almost fully customizable

340d62d

Tests: Make sure regtest is now customizable

2aaf473

jtimon force-pushed the b20-customizable-regtest branch from ff1d546 to 2aaf473 Compare February 19, 2020 20:50

DrahtBot removed the Needs rebase label Feb 19, 2020

Sjors added a commit to jonathanbier/forkmonitor that referenced this pull request Mar 3, 2020

Switch vendor/bitcoin to custom branch

1bf3281

A rebased version of: bitcoin/bitcoin#17032 This adds consensus customization support to regtest.

Sjors mentioned this pull request Mar 3, 2020

Customize regtest consensus params jonathanbier/forkmonitor#127

Merged

Sjors reviewed Mar 3, 2020

View reviewed changes

DrahtBot mentioned this pull request Mar 5, 2020

BIP-325: Signet [consensus] #18267

Merged

jtimon closed this May 6, 2020

Sjors mentioned this pull request Dec 16, 2020

Revive customizable regtest or find alternative jonathanbier/forkmonitor#147

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Conversation

jtimon commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

maflcko commented Oct 8, 2019

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jtimon commented Oct 30, 2019

Uh oh!

jtimon commented Feb 19, 2020

Uh oh!

Sjors left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

Sjors Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

Sjors Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

Sjors Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

Sjors Mar 3, 2020

Choose a reason for hiding this comment

Uh oh!

jtimon commented May 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jtimon commented Oct 2, 2019 •

edited

Loading

DrahtBot commented Oct 3, 2019 •

edited

Loading

Sjors left a comment •

edited

Loading