Page MenuHomePhabricator
Differential D17988

refactor: Replace string chain name constants with ChainTypes
Needs ReviewPublic
Actions

Authored by PiRK on Thu, Apr 24, 11:01.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Summary

This commit effectively moves the definition of these constants
out of the chainparamsbase to their own file.

Using the ChainType enums provides better type safety compared to
passing around strings.

The commit is part of an ongoing effort to decouple the libbitcoinkernel
library from the ArgsManager and other functionality that should not be
part of the kernel library.

Backport note: the Config ctor could also take a ChainType param instead of a network string but lets do this in a separate diff

This is a partial backport of core#27491
https://github.com/bitcoin/bitcoin/pull/27491/commits/ba8fc7d788932b25864fb260ca14983aa2398c23

Depends on D17987

Test Plan

With Chronik compiled:
ninja all check-all bench-bitcoin bitcoin-fuzzers

Diff Detail

Event Timeline

PiRK created this revision.Thu, Apr 24, 11:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Apr 24, 11:01
PiRK requested review of this revision.Thu, Apr 24, 11:01
teamcity edited the summary of this revision. (Show Details)Thu, Apr 24, 11:01
PiRK added a child revision: D17989: Fix chainparams includes.Thu, Apr 24, 11:03
PiRK added a comment.Thu, Apr 24, 11:11

It's a big diff, but the change is pretty trivial. I suggest paying particular attention to paymentserver.cpp during the review because of the raw pointers.

src/chainparamsbase.cpp
61–62 ↗(On Diff #53665)

This line and other similar lines are effectively dead code, and should be replaced with assert false; in a future backport (see https://github.com/bitcoin/bitcoin/pull/27611)

Harbormaster completed remote builds in B33089: Diff 53665.Thu, Apr 24, 11:28
teamcity added a comment.Thu, Apr 24, 11:28
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

tests/test_iguana.py::test_redeem_script_input_sigchecks ##teamcity[testStarted timestamp='2025-04-24T11:27:18.859' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' metainfo='test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']
PASSED          [100%]##teamcity[testFinished timestamp='2025-04-24T11:27:18.916' duration='56' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']


============================== 20 passed in 0.67s ==============================
[200/521] Test Bitcoin utilities...
[201/521] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_tests.cpp.o
[202/521] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/options_tests.cpp.o
[203/521] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/db_tests.cpp.o
[204/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o
[205/521] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o
[206/521] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/eda_tests.cpp.o
[207/521] Linking CXX executable src/pow/test/test-pow
[208/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o
[209/521] pow: testing daa_tests
[210/521] Running utility command for check-pow-daa_tests
[211/521] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o
[212/521] pow: testing eda_tests
[213/521] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o
[214/521] Running utility command for check-pow-eda_tests
[215/521] pow: testing grasberg_tests
[216/521] Running utility command for check-pow-grasberg_tests
[217/521] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o
[218/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o
[219/521] Linking CXX executable src/seeder/test/test-seeder
[220/521] seeder: testing db_tests
[221/521] seeder: testing message_writer_tests
[222/521] Running utility command for check-seeder-db_tests
[223/521] Running utility command for check-seeder-message_writer_tests
[224/521] seeder: testing options_tests
[225/521] seeder: testing p2p_messaging_tests
[226/521] Running utility command for check-seeder-options_tests
[227/521] Running utility command for check-seeder-p2p_messaging_tests
[228/521] seeder: testing parse_name_tests
[229/521] Running utility command for check-seeder-parse_name_tests
[230/521] seeder: testing write_name_tests
[231/521] Running utility command for check-seeder-write_name_tests
[232/521] Running seeder test suite
PASSED: seeder test suite
[233/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o
[234/521] pow: testing aserti32d_tests
[235/521] Running utility command for check-pow-aserti32d_tests
[236/521] Running pow test suite
PASSED: pow test suite
[237/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[238/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[239/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/optiontests.cpp.o
[240/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[241/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[242/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[243/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[244/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[245/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[246/521] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[247/521] Linking CXX executable src/qt/test/test_bitcoin-qt
[248/521] bitcoin-qt: testing test_bitcoin-qt
[249/521] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
Fabien requested changes to this revision.Thu, Apr 24, 12:38
Fabien added a subscriber: Fabien.

clang-tidy is not happy

This revision now requires changes to proceed.Thu, Apr 24, 12:38
PiRK updated this revision to Diff 53671.Thu, Apr 24, 13:59

fix argument name

Harbormaster completed remote builds in B33092: Diff 53671.Thu, Apr 24, 14:09
teamcity added a comment.Thu, Apr 24, 14:09
Build Bitcoin ABC Diffs / Diff Testing (ecash-lib-integration-tests) failed.

Tail of the build log:

  hdwallet.ts                  |       0 |        0 |       0 |       0 | 12-176                    
  hmac.ts                      |       0 |        0 |       0 |       0 | 16-73                     
  index.ts                     |       0 |        0 |       0 |       0 |                           
  indexBrowser.ts              |       0 |        0 |       0 |       0 |                           
  indexNodeJs.ts               |       0 |        0 |       0 |       0 |                           
  initBrowser.ts               |       0 |      100 |       0 |       0 | 11-17                     
  initNodeJs.ts                |   54.54 |      100 |     100 |      80 | 10                        
  messages.ts                  |       0 |        0 |       0 |       0 | 18-123                    
  mnemonic.ts                  |       0 |        0 |       0 |       0 | 9-144                     
  op.ts                        |   20.13 |    23.33 |   36.36 |   39.47 | ...09,111,119-124,135-163 
  opcode.ts                    |    50.2 |    83.33 |     100 |     100 | 1                         
  pbkdf2.ts                    |       0 |      100 |       0 |       0 | 17-51                     
  script.ts                    |   28.44 |    20.58 |   29.03 |   50.87 | ...33-144,150,160,182-193 
  sigHashType.ts               |      40 |       25 |   46.15 |   78.94 | 26-38                     
  tx.ts                        |   47.25 |    45.23 |   47.61 |   87.23 | 110,114,123-125,144       
  txBuilder.ts                 |   40.74 |    33.92 |   54.54 |   80.43 | ...62,181-186,191,261-265 
  unsignedTx.ts                |   25.27 |       16 |   30.76 |   46.15 | ...12,320,326-329,345,357 
 src/address                   |   11.35 |    15.15 |    5.12 |   22.41 |                           
  address.ts                   |   10.95 |    11.36 |    3.22 |   21.05 | ...39-240,255-256,266-344 
  legacyaddr.ts                |   12.04 |    22.72 |    12.5 |      25 | ...9,23-38,70-111,124-128 
 src/ffi                       |   14.37 |    12.87 |    8.25 |   14.54 |                           
  ecash_lib_wasm_bg_browser.js |       0 |      100 |     100 |       0 | 1                         
  ecash_lib_wasm_browser.js    |       0 |        0 |       0 |       0 | 3-681                     
  ecash_lib_wasm_nodejs.js     |    29.9 |    36.11 |   17.64 |   30.09 | ...19,526-591,597-598,602 
 src/io                        |   29.72 |    41.17 |   37.87 |   57.44 |                           
  bytes.ts                     |    7.04 |       60 |   10.52 |   13.88 | 12,23-83                  
  hex.ts                       |   41.55 |       50 |   44.44 |   82.35 | 41-45,50,58               
  int.ts                       |       0 |        0 |       0 |       0 |                           
  str.ts                       |   46.15 |    83.33 |      40 |   85.71 | 15                        
  varsize.ts                   |   16.32 |    21.05 |      40 |      32 | 14-24,40-47               
  writer.ts                    |       0 |        0 |       0 |       0 |                           
  writerbytes.ts               |   42.62 |    40.62 |   53.33 |   83.87 | 34,44,54,64,80            
  writerlength.ts              |   53.33 |    83.33 |   53.84 |     100 | 1                         
 src/payment                   |       0 |        0 |       0 |       0 |                           
  asn1.ts                      |       0 |        0 |       0 |       0 | 77-439                    
  index.ts                     |       0 |        0 |       0 |       0 |                           
  x509.ts                      |       0 |      100 |     100 |       0 | 5-15                      
 src/test                      |   46.05 |    37.68 |      50 |   89.04 |                           
  testRunner.ts                |   46.05 |    37.68 |      50 |   89.04 | 66-68,81-82,105,114,157   
 src/token                     |   31.19 |    24.63 |   34.56 |   46.42 |                           
  alp.parse.ts                 |       0 |        0 |       0 |       0 | 102-256                   
  alp.ts                       |   42.68 |    53.12 |   43.47 |   83.33 | 119-132,151               
  common.ts                    |      52 |    83.33 |     100 |     100 | 1                         
  empp.ts                      |   52.17 |       60 |   57.14 |   91.66 | 12                        
  slp.parse.ts                 |       0 |        0 |       0 |       0 | 128-382                   
  slp.ts                       |   47.16 |    33.82 |      52 |   90.36 | ...82,188,196,199,218,223 
-------------------------------|---------|----------|---------|---------|---------------------------

##teamcity[blockOpened name='Code Coverage Summary']
##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='879']
##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4206']
##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='263']
##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='1193']
##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='144']
##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='692']
##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='858']
##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='3057']
##teamcity[blockClosed name='Code Coverage Summary']
mv: cannot stat 'test_results/ecash-lib-integration-tests-junit.xml': No such file or directory
Build ecash-lib-integration-tests failed with exit code 1

Revision Contents
Changeset List

PathSize
chronik/
chronik-cpp/
15 lines
src/
avalanche/
test/
11 lines
bench/
5 lines
3 lines
5 lines
3 lines
3 lines
3 lines
25 lines
2 lines
2 lines
2 lines
5 lines
34 lines
14 lines
54 lines
common/
2 lines
18 lines
5 lines
30 lines
kernel/
13 lines
13 lines
2 lines
2 lines
networks/
abc/
22 lines
pow/
test/
6 lines
2 lines
23 lines
9 lines
qt/
4 lines
30 lines
3 lines
4 lines
17 lines
36 lines
2 lines
test/
4 lines
4 lines
3 lines
3 lines
4 lines
7 lines
rpc/
2 lines
2 lines
4 lines
seeder/
2 lines
2 lines
test/
7 lines
7 lines
test/
3 lines
3 lines
5 lines
24 lines
3 lines
5 lines
7 lines
4 lines
3 lines
fuzz/
3 lines
3 lines
3 lines
3 lines
3 lines
3 lines
4 lines
3 lines
3 lines
3 lines
3 lines
3 lines
2 lines
5 lines
14 lines
10 lines
3 lines
11 lines
3 lines
util/
15 lines
23 lines
47 lines
10 lines
util/
6 lines
8 lines
wallet/
test/
3 lines
6 lines
4 lines
5 lines

Diff 53671

View Options

chronik/chronik-cpp/chronik.cpp

Loading...
View Options

src/avalanche/test/stakingrewards_tests.cpp

Loading...
View Options

src/bench/coin_selection.cpp

Loading...
View Options

src/bench/load_external.cpp

Loading...
View Options

src/bench/mempool_stress.cpp

Loading...
View Options

src/bench/rpc_blockchain.cpp

Loading...
View Options

src/bench/rpc_mempool.cpp

Loading...
View Options

src/bitcoin-chainstate.cpp

Loading...
View Options

src/bitcoin-cli.cpp

Loading...
View Options

src/bitcoin-tx.cpp

Loading...
View Options

src/bitcoin-wallet.cpp

Loading...
View Options

src/bitcoind.cpp

Loading...
View Options

src/chainparams.h

Loading...
View Options

src/chainparams.cpp

Loading...
View Options

src/chainparamsbase.h

Loading...
View Options

src/chainparamsbase.cpp

Loading...
View Options

src/common/args.h

Loading...
View Options

src/common/args.cpp

Loading...
View Options

src/common/configfile.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/kernel/chainparams.h

Loading...
View Options

src/kernel/chainparams.cpp

Loading...
View Options

src/mempool_args.cpp

Loading...
View Options

src/minerfund.cpp

Loading...
View Options

src/networks/abc/checkpoints.cpp

Loading...
View Options

src/pow/test/aserti32d_tests.cpp

Loading...
View Options

src/pow/test/daa_tests.cpp

Loading...
View Options

src/pow/test/eda_tests.cpp

Loading...
View Options

src/pow/test/grasberg_tests.cpp

Loading...
View Options

src/qt/bitcoin.cpp

Loading...
View Options

src/qt/guiutil.cpp

Loading...
View Options

src/qt/intro.cpp

Loading...
View Options

src/qt/networkstyle.h

Loading...
View Options

src/qt/networkstyle.cpp

Loading...
View Options

src/qt/paymentserver.cpp

Loading...
View Options

src/qt/rpcconsole.cpp

Loading...
View Options

src/qt/test/apptests.cpp

Loading...
View Options

src/qt/test/bitcoinaddressvalidatortests.cpp

Loading...
View Options

src/qt/test/guiutiltests.cpp

Loading...
View Options

src/qt/test/paymentservertests.cpp

Loading...
View Options

src/qt/test/test_main.cpp

Loading...
View Options

src/qt/test/uritests.cpp

Loading...
View Options

src/rpc/blockchain.cpp

Loading...
View Options

src/rpc/mining.cpp

Loading...
View Options

src/rpc/net.cpp

Loading...
View Options

src/seeder/main.cpp

Loading...
View Options

src/seeder/options.cpp

Loading...
View Options

src/seeder/test/message_writer_tests.cpp

Loading...
View Options

src/seeder/test/p2p_messaging_tests.cpp

Loading...
View Options

src/test/activation_tests.cpp

Loading...
View Options

src/test/blockcheck_tests.cpp

Loading...
View Options

src/test/blockmanager_tests.cpp

Loading...
View Options

src/test/cashaddrenc_tests.cpp

Loading...
View Options

src/test/checkpoints_tests.cpp

Loading...
View Options

src/test/checkqueue_tests.cpp

Loading...
View Options

src/test/config_tests.cpp

Loading...
View Options

src/test/dnsseeds_tests.cpp

Loading...
View Options

src/test/dstencode_tests.cpp

Loading...
View Options

src/test/fuzz/block.cpp

Loading...
View Options

src/test/fuzz/descriptor_parse.cpp

Loading...
View Options

src/test/fuzz/integer.cpp

Loading...
View Options

src/test/fuzz/key.cpp

Loading...
View Options

src/test/fuzz/key_io.cpp

Loading...
View Options

src/test/fuzz/message.cpp

Loading...
View Options

src/test/fuzz/net.cpp

Loading...
View Options

src/test/fuzz/p2p_transport_deserializer.cpp

Loading...
View Options

src/test/fuzz/parse_univalue.cpp

Loading...
View Options

src/test/fuzz/pow.cpp

Loading...
View Options

src/test/fuzz/script.cpp

Loading...
View Options

src/test/fuzz/script_sign.cpp

Loading...
View Options

src/test/fuzz/system.cpp

Loading...
View Options

src/test/fuzz/transaction.cpp

Loading...
View Options

src/test/key_io_tests.cpp

Loading...
View Options

src/test/key_tests.cpp

Loading...
View Options

src/test/miner_tests.cpp

Loading...
View Options

src/test/settings_tests.cpp

Loading...
View Options

src/test/undo_tests.cpp

Loading...
View Options

src/test/util/setup_common.h

Loading...
View Options

src/test/util/setup_common.cpp

Loading...
View Options

src/test/util_tests.cpp

Loading...
View Options

src/test/validation_tests.cpp

Loading...
View Options

src/util/settings.h

Loading...
View Options

src/util/settings.cpp

Loading...
View Options

src/wallet/test/init_test_fixture.h

Loading...
View Options

src/wallet/test/init_test_fixture.cpp

Loading...
View Options

src/wallet/test/wallet_test_fixture.h

Loading...
View Options

src/wallet/test/wallet_test_fixture.cpp

Loading...