Tests: Chainparams: Make regtest almost fully customizable #17032

pull jtimon wants to merge 2 commits into bitcoin:master from jtimon:b20-customizable-regtest changing 3 files +105 −49
  1. jtimon commented at 10:20 PM on October 2, 2019: contributor

    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.

  2. DrahtBot added the label Tests on Oct 2, 2019
  3. DrahtBot added the label Validation on Oct 2, 2019
  4. jtimon cross-referenced this on Oct 3, 2019 from issue Testchains: Introduce custom chain whose constructor... by jtimon
  5. jtimon cross-referenced this on Oct 3, 2019 from issue Testschains: Many regtests with different genesis and default datadir by jtimon
  6. DrahtBot commented at 8:30 PM on October 3, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18267 (BIP-325: Signet [consensus] by kallewoof)
    • #17556 (test: Change feature_config_args.py not to rely on strange regtest=0 behavior by ryanofsky)
    • #16770 (Chainparams: Rename IsTestChain() to AllowAcceptNonstd() by jtimon)
    • #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.

  7. MarcoFalke commented at 2:03 PM on October 8, 2019: member

    Concept ACK. Didn't look at the approach.

  8. jtimon cross-referenced this on Oct 8, 2019 from issue Tests: Use self.chain instead of 'regtest' in all current tests by jtimon
  9. jtimon cross-referenced this on Oct 10, 2019 from issue Signet with #17037 on top by jtimon
  10. in src/chainparams.cpp:294 in 6c54f7d447 outdated 
     290 | @@ -334,10 +291,12 @@ class CRegTestParams : public CChainParams {
 291 |          consensus.vDeployments[d].nTimeout = nTimeout;
 292 |      }
 293 |      void UpdateActivationParametersFromArgs(const ArgsManager& args);
    fjahr commented at 5:45 PM on October 28, 2019:

    Since this is called inside of UpdateFromArgs() I think this line can be removed?

    jtimon commented at 7:10 PM on October 29, 2019:

    No, the method still needs to be declared.

    chainparams.cpp:296:6: error: no declaration matches ‘void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager&)’
 void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager& args)
      ^~~~~~~~~~~~~~
chainparams.cpp:296:6: note: no functions named ‘void CRegTestParams::UpdateActivationParametersFromArgs(const ArgsManager&)’
chainparams.cpp:254:7: note: ‘class CRegTestParams’ defined here
 class CRegTestParams : public CChainParams {
       ^~~~~~~~~~~~~~
chainparams.cpp: In member function ‘void CRegTestParams::UpdateFromArgs(const ArgsManager&)’:
chainparams.cpp:342:5: error: ‘UpdateActivationParametersFromArgs’ was not declared in this scope
     UpdateActivationParametersFromArgs(args);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
chainparams.cpp:342:5: note: suggested alternative: ‘UpdateVersionBitsParameters’
     UpdateActivationParametersFromArgs(args);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     UpdateVersionBitsParameters
  11. in src/chainparams.cpp:257 in 6c54f7d447 outdated 
     254 | @@ -255,54 +255,18 @@ class CRegTestParams : public CChainParams {
 255 |  public:
 256 |      explicit CRegTestParams(const ArgsManager& args) {
 257 |          strNetworkID = "regtest";
    fjahr commented at 5:45 PM on October 28, 2019:

    nit: could set this string from CBaseChainParams::REGTEST.

    jtimon commented at 7:32 PM on October 29, 2019:

    mhmm. I remember this failing to compile or to run at some point, but it seems to work now. Perhaps it had to do with some older version of cpp or something. Or perhaps I misremember and it was only networksyle.cpp where I wasn't able to to it. In any case, since this can be done for other chains other than regtest and in other places other than chainparams.cpp, I'm going to do it in another PR: https://github.com/bitcoin/bitcoin/pull/17306

  12. fjahr commented at 5:46 PM on October 28, 2019: contributor

    Concept ACK. Did a light code review.

  13. DrahtBot added the label Needs rebase on Oct 30, 2019
  14. jtimon force-pushed on Oct 30, 2019
  15. jtimon commented at 12:53 PM on October 30, 2019: contributor

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

  16. DrahtBot removed the label Needs rebase on Oct 30, 2019
  17. DrahtBot cross-referenced this on Feb 11, 2020 from issue Util: Allow scheduler to be mocked by amitiuttarwar
  18. DrahtBot cross-referenced this on Feb 11, 2020 from issue test: Change feature_config_args.py not to rely on strange regtest=0 behavior by ryanofsky
  19. DrahtBot cross-referenced this on Feb 11, 2020 from issue Chainparams: Rename IsTestChain() to AllowAcceptNonstd() by jtimon
  20. DrahtBot cross-referenced this on Feb 11, 2020 from issue BIP-325: Signet support by kallewoof
  21. DrahtBot cross-referenced this on Feb 11, 2020 from issue test: Set BIP34Height = 2 for regtest by MarcoFalke
  22. DrahtBot cross-referenced this on Feb 11, 2020 from issue Remove 16 bits from versionbits signalling system (BIP320) by btcdrak
  23. DrahtBot added the label Needs rebase on Feb 18, 2020
  24. jtimon force-pushed on Feb 19, 2020
  25. jtimon commented at 8:41 PM on February 19, 2020: contributor

    Rebased

  26. Chainparams: Make regtest almost fully customizable 340d62d48d
  27. Tests: Make sure regtest is now customizable 2aaf473e57
  28. jtimon force-pushed on Feb 19, 2020
  29. DrahtBot removed the label Needs rebase on Feb 19, 2020
  30. Sjors referenced this in commit 1bf328127f on Mar 3, 2020
  31. Sjors cross-referenced this on Mar 3, 2020 from issue Customize regtest consensus params by Sjors
  32. in src/chainparamsbase.cpp:48 in 340d62d48d outdated 
      43 | +    gArgs.AddArg("-ndefaultport", "The port to listen for connections on by default. Consider using -port instead of changing the default.  Default: 18444 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
  44 | +    gArgs.AddArg("-npruneafterheight", "Only start prunning after this height. Default: 1000 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
  45 | +    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);
  46 | +    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);
  47 | +    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);
  48 | +    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);
    Sjors commented at 4:56 PM on March 3, 2020:

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

  33. in src/chainparamsbase.cpp:49 in 340d62d48d outdated 
      44 | +    gArgs.AddArg("-npruneafterheight", "Only start prunning after this height. Default: 1000 (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
  45 | +    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);
  46 | +    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);
  47 | +    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);
  48 | +    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);
  49 | +    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);
    Sjors commented at 4:57 PM on March 3, 2020:

    This seems useless?

  34. in src/chainparamsbase.cpp:50 in 340d62d48d outdated 
      45 | +    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);
  46 | +    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);
  47 | +    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);
  48 | +    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);
  49 | +    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);
  50 | +    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);
    Sjors commented at 4:58 PM on March 3, 2020:

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

  35. in src/chainparamsbase.cpp:52 in 340d62d48d outdated 
      47 | +    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);
  48 | +    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);
  49 | +    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);
  50 | +    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);
  51 | +    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);
  52 | +    gArgs.AddArg("-pubkeyprefix", "Magic for base58 pubkeys. (regtest-only)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CHAINPARAMS);
    Sjors commented at 5:00 PM on March 3, 2020:

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

  36. in test/functional/feature_config_args.py:45 in 2aaf473e57 
      41 | @@ -42,9 +42,9 @@ def test_config_file_parser(self):
  42 |              self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Config setting for -wallet only applied on %s network when in [%s] section.' % (self.chain, self.chain))
  43 |  
  44 |          with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
  45 | -            conf.write('regtest=0\n') # mainnet
  46 | +            conf.write('is_test_chain=0\n') # like mainnet
    Sjors commented at 5:06 PM on March 3, 2020:

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

  37. Sjors commented at 5:14 PM on March 3, 2020: member

    Concept ACK. Lightly tested 2aaf473e57de8e4a016372273a95dd89e68caf7f 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.

  38. DrahtBot cross-referenced this on Mar 5, 2020 from issue BIP-325: Signet [consensus] by kallewoof
  39. jtimon commented at 1:35 PM on May 6, 2020: contributor

    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.

  40. jtimon closed this on May 6, 2020
  41. Sjors cross-referenced this on Dec 16, 2020 from issue Revive customizable regtest or find alternative by Sjors
  42. bitcoin locked this on Feb 15, 2022
Contributors
jtimonDrahtBotMarcoFalkefjahrSjors
Labels
TestsValidation
Linked (view graph)
#8994 Testchains: Introduce custom chain whose constructor... #16411 BIP-325: Signet support#16681 Tests: Use self.chain instead of 'regtest' in all current tests#17037 Testschains: Many regtests with different genesis and default datadir#17306 refactor: Use name constants in chainparams initialization#17449 fix uninitialized variable nMinerConfirmationWindow#31137 Remove BIP94 from regtest
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC