Page MenuHomePhabricator

Added a script to generate chainparams constants from intermediate files
ClosedPublic

Authored by jasonbcox on Sep 20 2019, 23:32.

Details

Summary

Building on D3732 so that chainparams constants no longer need to be copy-pasted manually.

Depends on D4140

Completes T663

Test Plan

For actually testing the process flow, generate the chainparams_*.txt files:

bitcoind
make_chainparams.py > chainparams_main.txt

bitcoind --testnet
make_chainparams.py -a 127.0.0.1:18332 > chainparams_main.txt

For this diff, I did not do the above, I copied the existing chainparams manually into the chainparams_*.txt files so that we don't add updating those to the review process of this diff.
After the chainparams_*.txt files are ready, do the following:

generate_chainparams_constants.py .

make check
ninja check

TeamCity IBD

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Sep 20 2019, 23:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 20 2019, 23:32
jasonbcox planned changes to this revision.Sep 20 2019, 23:32
jasonbcox updated this revision to Diff 13094.Sep 23 2019, 21:32

Added generated files and migrated chainparams.cpp to use the new constants.

jasonbcox edited the summary of this revision. (Show Details)Sep 23 2019, 21:33
jasonbcox edited the test plan for this revision. (Show Details)
jasonbcox edited the test plan for this revision. (Show Details)Sep 23 2019, 21:36
jasonbcox edited the test plan for this revision. (Show Details)Sep 23 2019, 21:39
Fabien requested changes to this revision.Sep 24 2019, 11:20
Fabien added a subscriber: Fabien.
Fabien added inline comments.
contrib/devtools/chainparams/generate_chainparams_constants.py
38 ↗(On Diff #13094)

This is what print does by default, you can replace output.write with print and this line becomes unecessary.

59 ↗(On Diff #13094)

You can use a single text block (heredoc style) and format it to improve the readability.

59 ↗(On Diff #13094)

I came to an example:

def process_constants(indir, file_name):
    with open(os.path.join(indir, file_name), 'r', encoding="utf8") as f:
        constants = f.readlines()
    return [line.strip() for line in constants]

def main():
    if len(sys.argv) != 2:
        print('Usage: {} <path_to_chainparams_txt>'.format(sys.argv[0]), file=sys.stderr)
        sys.exit(1)

    indir = sys.argv[1]

    print('''
#ifndef BITCOIN_CHAINPARAMSCONSTANTS_H
#define BITCOIN_CHAINPARAMSCONSTANTS_H
/**
 * @{} by contrib/devtools/chainparams/generate_chainparams_constants.py
 */

#include <uint256.h>

namespace ChainParamsConstants {{
    const uint256 MAINNET_DEFAULT_ASSUME_VALID = uint256S("{}");
    const uint256 MAINNET_MINIMUM_CHAIN_WORK = uint256S("{}");

    const uint256 TESTNET_DEFAULT_ASSUME_VALID = uint256S("{}");
    const uint256 TESTNET_MINIMUM_CHAIN_WORK = uint256S("{}");
}} // namespace ChainParamsConstants

#endif // BITCOIN_CHAINPARAMSCONSTANTS_H

'''.format(
        "generated",
        *process_constants(indir, 'chainparams_main.txt'),
        *process_constants(indir, 'chainparams_test.txt'))[1:-1]
    )
This revision now requires changes to proceed.Sep 24 2019, 11:20
jasonbcox updated this revision to Diff 13121.Sep 24 2019, 17:18

Incorporated feedback with a minor adjustment to simplify the formatting of newlines at the beginning and end of the generated output.

Fabien requested changes to this revision.Sep 25 2019, 05:59
Fabien added inline comments.
contrib/devtools/chainparams/generate_chainparams_constants.py
30 ↗(On Diff #13121)

Assert that there are exactly 2 lines before returning, otherwise the output can silently corrupt.

This revision now requires changes to proceed.Sep 25 2019, 05:59
Fabien added inline comments.Sep 25 2019, 06:00
contrib/devtools/chainparams/generate_chainparams_constants.py
41 ↗(On Diff #13121)

Should we add a copyright header to generated files ?

jasonbcox added inline comments.Sep 25 2019, 17:30
contrib/devtools/chainparams/generate_chainparams_constants.py
41 ↗(On Diff #13121)

Considering that chainparamsseeds.h doesn't have one, it probably doesn't matter much.

jasonbcox updated this revision to Diff 13134.Sep 25 2019, 17:30

Added assert

Fabien accepted this revision.Sep 25 2019, 18:46
This revision is now accepted and ready to land.Sep 25 2019, 18:46