Page MenuHomePhabricator

Decouple ChainParams from ArgsManager, add factory functions
ClosedPublic

Authored by PiRK on May 6 2024, 07:46.

Details

Summary

Chain params can now be initialized by configuring a ChainOptions struct. This offers an interface for
creating ChainParams without a gArgs object.

The factory functions can also easily be used from a context without an instantiated ArgsManager, e.g. from libbitcoin kernel code, unlike the existing CreateChainParams method.

Inspired by:
https://github.com/bitcoin/bitcoin/pull/13311/commits/6fa901fb4726ddac025d5396ecf09d047a8aa9a1 (make globalChainParams const)
https://github.com/bitcoin/bitcoin/pull/20004/commits/fa23308e9aad70c99a31f91d8556f1876ea02c04 (Remove gArgs global from CreateChainParams)
https://github.com/bitcoin/bitcoin/pull/26177/commits/84b85786f0f5cb23cc257a4464ae345e1d372313 (Decouple ChainParams from ArgsManager)
https://github.com/bitcoin/bitcoin/pull/26177/commits/edabbc78a3bc272b2b802e1dbab73d6ed8e31e96 (Add factory functions for Main/Test/Reg chainparams)

The main difference with the source material is that we need the ArgsManager for all chains, because of the -ecash arg.

Test Plan

ninja all check-all bitcoin-fuzzers

Event Timeline

PiRK requested review of this revision.May 6 2024, 07:46

Tail of the build log:

[371/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[372/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/script_sigcache.cpp.o
[373/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/transaction.cpp.o
FAILED: src/test/fuzz/CMakeFiles/fuzz.dir/transaction.cpp.o 
/usr/bin/ccache /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/miniupnpc -Werror -g -O2 -fPIE -fvisibility=hidden -fsanitize=fuzzer -fstack-protector-all -Wstack-protector -fcf-protection=full -fstack-clash-protection -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -pthread -std=gnu++17 -MD -MT src/test/fuzz/CMakeFiles/fuzz.dir/transaction.cpp.o -MF src/test/fuzz/CMakeFiles/fuzz.dir/transaction.cpp.o.d -o src/test/fuzz/CMakeFiles/fuzz.dir/transaction.cpp.o -c ../../src/test/fuzz/transaction.cpp
../../src/test/fuzz/transaction.cpp:76:35: error: no viable conversion from 'unique_ptr<const CChainParams>' to 'unique_ptr<CChainParams>'
    std::unique_ptr<CChainParams> params =
                                  ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:320:12: note: candidate constructor template not viable: no known conversion from 'std::unique_ptr<const CChainParams>' to 'std::nullptr_t' (aka 'nullptr_t') for 1st argument
        constexpr unique_ptr(nullptr_t) noexcept
                  ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:327:7: note: candidate constructor not viable: no known conversion from 'std::unique_ptr<const CChainParams>' to 'std::unique_ptr<CChainParams> &&' for 1st argument
      unique_ptr(unique_ptr&&) = default;
      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:468:7: note: candidate constructor not viable: no known conversion from 'std::unique_ptr<const CChainParams>' to 'const std::unique_ptr<CChainParams> &' for 1st argument
      unique_ptr(const unique_ptr&) = delete;
      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/type_traits:2192:46: note: candidate template ignored: disabled by 'enable_if' [with _Up = const CChainParams, _Ep = std::default_delete<const CChainParams>]
    using __enable_if_t = typename enable_if<_Cond, _Tp>::type;
                                             ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:350:2: note: candidate template ignored: could not match 'auto_ptr' against 'unique_ptr'
        unique_ptr(auto_ptr<_Up>&& __u) noexcept;
        ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/unique_ptr.h:281:2: note: explicit constructor is not a candidate
        unique_ptr(pointer __p) noexcept
        ^
1 error generated.
[374/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/timedata.cpp.o
[375/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/string.cpp.o
[376/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/system.cpp.o
[377/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[378/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/strprintf.cpp.o
[379/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[380/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[381/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/script_sign.cpp.o
[382/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[383/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/validation_load_mempool.cpp.o
[384/405] Building CXX object src/test/fuzz/CMakeFiles/fuzz.dir/txrequest.cpp.o
[385/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[386/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[387/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[388/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[389/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[390/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[391/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[392/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[393/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[394/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[395/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[396/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[397/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[398/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[399/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[400/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[401/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[402/405] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[403/405] Linking CXX static library src/wallet/libwallet.a
[404/405] Linking CXX static library src/libserver.a
ninja: build stopped: cannot make progress due to previous errors.
Build build-fuzzer failed with exit code 1
Fabien requested changes to this revision.May 6 2024, 08:44
Fabien added a subscriber: Fabien.

Fuzzer is broken, back to your queue

This revision now requires changes to proceed.May 6 2024, 08:44
PiRK edited the test plan for this revision. (Show Details)

make chainparams const in fuzzer, add bitcoin-fuzzers to test plan

Fabien requested changes to this revision.May 6 2024, 12:22
Fabien added inline comments.
src/chainparams.cpp
508

TIL you use vi

This revision now requires changes to proceed.May 6 2024, 12:22
src/chainparams.cpp
508

In the terminal for commit messages, mostly. But sometimes the cursor is in the wrong window.

fix accidental :wq, convert a gArgs into args

This revision is now accepted and ready to land.May 6 2024, 13:55