From c2bd834f3b127295bcbebcdaf98122f577a24658 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 28 Apr 2021 09:45:24 +0200 Subject: [PATCH 01/10] Merge bitcoin/bitcoin#21169: fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 545404e7e1c72985557ccffe865cea269143e5dd fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (practicalswift) Pull request description: Add RPC interface fuzzing. This PR increases overall fuzzing line coverage from [~65%](https://marcofalke.github.io/btc_cov/fuzz.coverage/) to ~70% 🎉 To test this PR: ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined $ make -C src/ test/fuzz/fuzz $ FUZZ=rpc src/test/fuzz/fuzz ``` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for more information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: MarcoFalke: Concept ACK 545404e7e1c72985557ccffe865cea269143e5dd Tree-SHA512: 35fc1b508af42bf480ee3762326b09ff2eecdb7960a1917ad16345fadd5c0c21d666dafe736176e5a848ff6492483c782e4ea914cd9000faf50190df051950fd --- src/Makefile.test.include | 1 + src/test/fuzz/rpc.cpp | 378 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 379 insertions(+) create mode 100644 src/test/fuzz/rpc.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 8e25d6682e41f..f491f8deca233 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -307,6 +307,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/psbt.cpp \ test/fuzz/random.cpp \ test/fuzz/rolling_bloom_filter.cpp \ + test/fuzz/rpc.cpp \ test/fuzz/script.cpp \ test/fuzz/script_bitcoin_consensus.cpp \ test/fuzz/script_descriptor_cache.cpp \ diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp new file mode 100644 index 0000000000000..cbe5de6d52507 --- /dev/null +++ b/src/test/fuzz/rpc.cpp @@ -0,0 +1,378 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace { +struct RPCFuzzTestingSetup : public TestingSetup { + RPCFuzzTestingSetup(const std::string& chain_name, const std::vector& extra_args) : TestingSetup{chain_name, extra_args} + { + } + + UniValue CallRPC(const std::string& rpc_method, const std::vector& arguments) + { + JSONRPCRequest request; + request.context = m_node; + request.strMethod = rpc_method; + request.params = RPCConvertValues(rpc_method, arguments); + return tableRPC.execute(request); + } + + std::vector GetRPCCommands() const + { + return tableRPC.listCommands(); + } +}; + +RPCFuzzTestingSetup* rpc_testing_setup = nullptr; +std::string g_limit_to_rpc_command; + +// RPC commands which are not appropriate for fuzzing: such as RPC commands +// reading or writing to a filename passed as an RPC parameter, RPC commands +// resulting in network activity, etc. +const std::vector RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{ + "addconnection", // avoid DNS lookups + "addnode", // avoid DNS lookups + "addpeeraddress", // avoid DNS lookups + "analyzepsbt", // avoid signed integer overflow in CFeeRate::GetFee(unsigned long) (https://github.com/bitcoin/bitcoin/issues/20607) + "dumptxoutset", // avoid writing to disk +#ifdef ENABLE_WALLET + "dumpwallet", // avoid writing to disk +#endif + "echoipc", // avoid assertion failure (Assertion `"EnsureAnyNodeContext(request.context).init" && check' failed.) + "generatetoaddress", // avoid timeout + "gettxoutproof", // avoid slow execution +#ifdef ENABLE_WALLET + "importwallet", // avoid reading from disk + "loadwallet", // avoid reading from disk +#endif + "mockscheduler", // avoid assertion failure (Assertion `delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}' failed.) + "prioritisetransaction", // avoid signed integer overflow in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) (https://github.com/bitcoin/bitcoin/issues/20626) + "setban", // avoid DNS lookups + "stop", // avoid shutdown state +}; + +// RPC commands which are safe for fuzzing. +const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ + "clearbanned", + "combinepsbt", + "combinerawtransaction", + "converttopsbt", + "createmultisig", + "createpsbt", + "createrawtransaction", + "decodepsbt", + "decoderawtransaction", + "decodescript", + "deriveaddresses", + "disconnectnode", + "echo", + "echojson", + "estimaterawfee", + "estimatesmartfee", + "finalizepsbt", + "generate", + "generateblock", + "generatetodescriptor", + "getaddednodeinfo", + "getbestblockhash", + "getblock", + "getblockchaininfo", + "getblockcount", + "getblockfilter", + "getblockhash", + "getblockheader", + "getblockstats", + "getblocktemplate", + "getchaintips", + "getchaintxstats", + "getconnectioncount", + "getdescriptorinfo", + "getdifficulty", + "getindexinfo", + "getmemoryinfo", + "getmempoolancestors", + "getmempooldescendants", + "getmempoolentry", + "getmempoolinfo", + "getmininginfo", + "getnettotals", + "getnetworkhashps", + "getnetworkinfo", + "getnodeaddresses", + "getpeerinfo", + "getrawmempool", + "getrawtransaction", + "getrpcinfo", + "gettxout", + "gettxoutsetinfo", + "help", + "invalidateblock", + "joinpsbts", + "listbanned", + "logging", + "ping", + "preciousblock", + "pruneblockchain", + "reconsiderblock", + "savemempool", + "scantxoutset", + "sendrawtransaction", + "setmocktime", + "setnetworkactive", + "signmessagewithprivkey", + "signrawtransactionwithkey", + "submitblock", + "submitheader", + "syncwithvalidationinterfacequeue", + "testmempoolaccept", + "uptime", + "utxoupdatepsbt", + "validateaddress", + "verifychain", + "verifymessage", + "verifytxoutproof", + "waitforblock", + "waitforblockheight", + "waitfornewblock", +}; + +std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + const size_t max_string_length = 4096; + std::string r; + CallOneOf( + fuzzed_data_provider, + [&] { + // string argument + r = fuzzed_data_provider.ConsumeRandomLengthString(max_string_length); + }, + [&] { + // base64 argument + r = EncodeBase64(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // hex argument + r = HexStr(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // bool argument + r = fuzzed_data_provider.ConsumeBool() ? "true" : "false"; + }, + [&] { + // range argument + r = "[" + ToString(fuzzed_data_provider.ConsumeIntegral()) + "," + ToString(fuzzed_data_provider.ConsumeIntegral()) + "]"; + }, + [&] { + // integral argument (int64_t) + r = ToString(fuzzed_data_provider.ConsumeIntegral()); + }, + [&] { + // integral argument (uint64_t) + r = ToString(fuzzed_data_provider.ConsumeIntegral()); + }, + [&] { + // floating point argument + r = strprintf("%f", fuzzed_data_provider.ConsumeFloatingPoint()); + }, + [&] { + // tx destination argument + r = EncodeDestination(ConsumeTxDestination(fuzzed_data_provider)); + }, + [&] { + // uint160 argument + r = ConsumeUInt160(fuzzed_data_provider).ToString(); + }, + [&] { + // uint256 argument + r = ConsumeUInt256(fuzzed_data_provider).ToString(); + }, + [&] { + // base32 argument + r = EncodeBase32(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length)); + }, + [&] { + // base58 argument + r = EncodeBase58(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length))); + }, + [&] { + // base58 argument with checksum + r = EncodeBase58Check(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length))); + }, + [&] { + // hex encoded block + std::optional opt_block = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_block) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_block; + r = HexStr(data_stream); + }, + [&] { + // hex encoded block header + std::optional opt_block_header = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_block_header) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_block_header; + r = HexStr(data_stream); + }, + [&] { + // hex encoded tx + std::optional opt_tx = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_tx) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_tx; + r = HexStr(data_stream); + }, + [&] { + // base64 encoded psbt + std::optional opt_psbt = ConsumeDeserializable(fuzzed_data_provider); + if (!opt_psbt) { + return; + } + CDataStream data_stream{SER_NETWORK, PROTOCOL_VERSION}; + data_stream << *opt_psbt; + r = EncodeBase64(data_stream); + }, + [&] { + // base58 encoded key + const std::vector random_bytes = fuzzed_data_provider.ConsumeBytes(32); + CKey key; + key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + if (!key.IsValid()) { + return; + } + r = EncodeSecret(key); + }, + [&] { + // hex encoded pubkey + const std::vector random_bytes = fuzzed_data_provider.ConsumeBytes(32); + CKey key; + key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + if (!key.IsValid()) { + return; + } + r = HexStr(key.GetPubKey()); + }); + return r; +} + +std::string ConsumeArrayRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + std::vector scalar_arguments; + while (fuzzed_data_provider.ConsumeBool()) { + scalar_arguments.push_back(ConsumeScalarRPCArgument(fuzzed_data_provider)); + } + return "[\"" + Join(scalar_arguments, "\",\"") + "\"]"; +} + +std::string ConsumeRPCArgument(FuzzedDataProvider& fuzzed_data_provider) +{ + return fuzzed_data_provider.ConsumeBool() ? ConsumeScalarRPCArgument(fuzzed_data_provider) : ConsumeArrayRPCArgument(fuzzed_data_provider); +} + +RPCFuzzTestingSetup* InitializeRPCFuzzTestingSetup() +{ + static const auto setup = MakeNoLogFileContext(); + SetRPCWarmupFinished(); + return setup.get(); +} +}; // namespace + +void initialize_rpc() +{ + rpc_testing_setup = InitializeRPCFuzzTestingSetup(); + const std::vector supported_rpc_commands = rpc_testing_setup->GetRPCCommands(); + for (const std::string& rpc_command : supported_rpc_commands) { + const bool safe_for_fuzzing = std::find(RPC_COMMANDS_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_SAFE_FOR_FUZZING.end(); + const bool not_safe_for_fuzzing = std::find(RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_NOT_SAFE_FOR_FUZZING.end(); + if (!(safe_for_fuzzing || not_safe_for_fuzzing)) { + std::cerr << "Error: RPC command \"" << rpc_command << "\" not found in RPC_COMMANDS_SAFE_FOR_FUZZING or RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + if (safe_for_fuzzing && not_safe_for_fuzzing) { + std::cerr << "Error: RPC command \"" << rpc_command << "\" found in *both* RPC_COMMANDS_SAFE_FOR_FUZZING and RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + } + for (const std::string& rpc_command : RPC_COMMANDS_SAFE_FOR_FUZZING) { + const bool supported_rpc_command = std::find(supported_rpc_commands.begin(), supported_rpc_commands.end(), rpc_command) != supported_rpc_commands.end(); + if (!supported_rpc_command) { + std::cerr << "Error: Unknown RPC command \"" << rpc_command << "\" found in RPC_COMMANDS_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + } + for (const std::string& rpc_command : RPC_COMMANDS_NOT_SAFE_FOR_FUZZING) { + const bool supported_rpc_command = std::find(supported_rpc_commands.begin(), supported_rpc_commands.end(), rpc_command) != supported_rpc_commands.end(); + if (!supported_rpc_command) { + std::cerr << "Error: Unknown RPC command \"" << rpc_command << "\" found in RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; + std::terminate(); + } + } + const char* limit_to_rpc_command_env = std::getenv("LIMIT_TO_RPC_COMMAND"); + if (limit_to_rpc_command_env != nullptr) { + g_limit_to_rpc_command = std::string{limit_to_rpc_command_env}; + } +} + +FUZZ_TARGET_INIT(rpc, initialize_rpc) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + SetMockTime(ConsumeTime(fuzzed_data_provider)); + const std::string rpc_command = fuzzed_data_provider.ConsumeRandomLengthString(64); + if (!g_limit_to_rpc_command.empty() && rpc_command != g_limit_to_rpc_command) { + return; + } + const bool safe_for_fuzzing = std::find(RPC_COMMANDS_SAFE_FOR_FUZZING.begin(), RPC_COMMANDS_SAFE_FOR_FUZZING.end(), rpc_command) != RPC_COMMANDS_SAFE_FOR_FUZZING.end(); + if (!safe_for_fuzzing) { + return; + } + std::vector arguments; + while (fuzzed_data_provider.ConsumeBool()) { + arguments.push_back(ConsumeRPCArgument(fuzzed_data_provider)); + } + try { + rpc_testing_setup->CallRPC(rpc_command, arguments); + } catch (const UniValue&) { + } catch (const std::runtime_error&) { + } +} From 4dd36f603a82e4a9885de70d2969529c527ada66 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 6 May 2021 16:03:46 +0200 Subject: [PATCH 02/10] Merge bitcoin/bitcoin#21798: fuzz: Create a block template in tx_pool targets fa03d0acd6bd8bb6d3d5227512f042ff537ad993 fuzz: Create a block template in tx_pool targets (MarcoFalke) fa61ce5cf5c1d73d352173806571bcd7799ed2ee fuzz: Limit mocktime to MTP in tx_pool targets (MarcoFalke) fab646b8ea293bb2b03707c6ef6790982625e492 fuzz: Use correct variant of ConsumeRandomLengthString instead of hardcoding a maximum size (MarcoFalke) fae2c8bc54e6c0fe69a82bd1b232c52edd1acd34 fuzz: Allow to pass min/max to ConsumeTime (MarcoFalke) Pull request description: Relatively simple check to ensure a block can always be created from the mempool ACKs for top commit: practicalswift: Tested ACK fa03d0acd6bd8bb6d3d5227512f042ff537ad993 Tree-SHA512: e613376ccc88591cbe594db14ea21ebc9b2b191f6325b3aa4ee0cd379695352ad3b480e286134ef6ee30f043d486cf9792a1bc7e44445c41045ac8c3b931c7ff --- src/test/fuzz/tx_pool.cpp | 40 ++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 2a69f1fde5f6d..7f1c9c4703762 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -77,6 +78,30 @@ void SetMempoolConstraints(ArgsManager& args, FuzzedDataProvider& fuzzed_data_pr ToString(fuzzed_data_provider.ConsumeIntegralInRange(0, 999))); } +void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, const NodeContext& node, CChainState& chainstate) +{ + WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + { + BlockAssembler::Options options; + options.nBlockMaxSize = fuzzed_data_provider.ConsumeIntegralInRange(0U, MaxBlockSize(true)); + options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider)}; + + auto assembler = BlockAssembler{chainstate, node, *static_cast(&tx_pool), ::Params(), options}; + auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE); + Assert(block_template->block.vtx.size() >= 1); + } + const auto info_all = tx_pool.infoAll(); + if (!info_all.empty()) { + const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; + WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, /* dummy */ MemPoolRemovalReason::BLOCK)); + std::vector all_txids; + tx_pool.queryHashes(all_txids); + assert(all_txids.size() < info_all.size()); + WITH_LOCK(::cs_main, tx_pool.check(chainstate)); + } + SyncWithValidationInterfaceQueue(); +} + void MockTime(FuzzedDataProvider& fuzzed_data_provider, const CChainState& chainstate) { const auto time = ConsumeTime(fuzzed_data_provider, @@ -254,17 +279,7 @@ FUZZ_TARGET_INIT(tx_pool_standard, initialize_tx_pool) } } } - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); - const auto info_all = tx_pool.infoAll(); - if (!info_all.empty()) { - const auto& tx_to_remove = *PickValue(fuzzed_data_provider, info_all).tx; - WITH_LOCK(tx_pool.cs, tx_pool.removeRecursive(tx_to_remove, /* dummy */ MemPoolRemovalReason::BLOCK)); - std::vector all_txids; - tx_pool.queryHashes(all_txids); - assert(all_txids.size() < info_all.size()); - WITH_LOCK(::cs_main, tx_pool.check(chainstate)); - } - SyncWithValidationInterfaceQueue(); + Finish(fuzzed_data_provider, tx_pool, node, chainstate); } FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) @@ -318,8 +333,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) if (accepted) { txids.push_back(tx->GetHash()); } - - SyncWithValidationInterfaceQueue(); } + Finish(fuzzed_data_provider, tx_pool, node, chainstate); } } // namespace From 813993d44ae6152249acd7604674539bbf0c1fcf Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 11 May 2021 20:35:17 +0200 Subject: [PATCH 03/10] Merge bitcoin/bitcoin#21892: fuzz: Avoid excessively large min fee rate in tx_pool 99993f066405863c66ccaec0a8427129f4515768 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke) Pull request description: Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078 ACKs for top commit: practicalswift: cr ACK 99993f066405863c66ccaec0a8427129f4515768: patch looks correct despite no `fa` prefix in commit hash Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6 --- src/test/fuzz/tx_pool.cpp | 2 +- src/test/fuzz/util.cpp | 5 +++++ src/test/fuzz/util.h | 5 +---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 7f1c9c4703762..a11c258b1abc5 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -84,7 +84,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, con { BlockAssembler::Options options; options.nBlockMaxSize = fuzzed_data_provider.ConsumeIntegralInRange(0U, MaxBlockSize(true)); - options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider)}; + options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /* max */ COIN)}; auto assembler = BlockAssembler{chainstate, node, *static_cast(&tx_pool), ::Params(), options}; auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE); diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 79d7706f24bd7..7141037f9535b 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -293,6 +293,11 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); } +CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional& max) noexcept +{ + return fuzzed_data_provider.ConsumeIntegralInRange(0, max.value_or(MAX_MONEY)); +} + int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional& min, const std::optional& max) noexcept { // Avoid t=0 (1970-01-01T00:00:00Z) since SetMockTime(0) disables mocktime. diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 543522c969d5f..259323dd73970 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -179,10 +179,7 @@ template return static_cast(fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_OPCODE)); } -[[nodiscard]] inline CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - return fuzzed_data_provider.ConsumeIntegralInRange(0, MAX_MONEY); -} +[[nodiscard]] CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional& max = std::nullopt) noexcept; [[nodiscard]] int64_t ConsumeTime(FuzzedDataProvider& fuzzed_data_provider, const std::optional& min = std::nullopt, const std::optional& max = std::nullopt) noexcept; From 2398283ff6f1174dfdf1c32f14f72632d4781514 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 12 May 2021 11:02:25 +1000 Subject: [PATCH 04/10] Merge bitcoin/bitcoin#21922: fuzz: Avoid timeout in EncodeBase58 faa0d94a7d9cdd10e81ee231a7b06d4b14b37e13 fuzz: Avoid timeout in EncodeBase58 (MarcoFalke) Pull request description: The complexity is O(N^2), so limit the size. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34126 Oss-Fuzz testcase for `rpc` fuzzer: https://github.com/bitcoin/bitcoin/files/6461382/clusterfuzz-testcase-minimized-rpc-4831734974775296.log ACKs for top commit: practicalswift: cr ACK faa0d94a7d9cdd10e81ee231a7b06d4b14b37e13: patch looks correct sipa: utACK faa0d94a7d9cdd10e81ee231a7b06d4b14b37e13 Tree-SHA512: 57ad9de8d811b828982d09a586782fc8a62fa3685590301d58120e2249caa30a9dccd3abe0b47e00ea8482de705fe0edbed298ab8761ea0d29496b50ed2db5d7 --- src/test/fuzz/rpc.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index cbe5de6d52507..36572943492d5 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -174,6 +174,7 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) { const size_t max_string_length = 4096; + const size_t max_base58_bytes_length{64}; std::string r; CallOneOf( fuzzed_data_provider, @@ -227,11 +228,11 @@ std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) }, [&] { // base58 argument - r = EncodeBase58(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length))); + r = EncodeBase58(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_base58_bytes_length))); }, [&] { // base58 argument with checksum - r = EncodeBase58Check(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_string_length))); + r = EncodeBase58Check(MakeUCharSpan(fuzzed_data_provider.ConsumeRandomLengthString(max_base58_bytes_length))); }, [&] { // hex encoded block From 3bfefde53b6de019cb8382ecd160ecd1536a2bf6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 12 May 2021 21:23:00 +0200 Subject: [PATCH 05/10] Merge bitcoin/bitcoin#21931: ci: Bump cirrus fuzz CPUs to avoid timeout fa397a6a9c1dd158d0b7dae36d8a88cfcbb9af38 ci: Bump cirrus fuzz CPUs to avoid timeout (MarcoFalke) Pull request description: ACKs for top commit: hebasto: ACK fa397a6a9c1dd158d0b7dae36d8a88cfcbb9af38, let's try it. Tree-SHA512: 7e06dda66c71d76e5fd144f6b5bb10f0bcac72feb15bd0f400ef08ba4dcb92558319401ef5f9d3822376affceb2192df1903b3a79c0ab2d7283ca21454054dea --- .cirrus.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.cirrus.yml b/.cirrus.yml index a5f780728afa4..3af0f0228e05a 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -128,7 +128,10 @@ task: << : *GLOBAL_TASK_TEMPLATE container: image: ubuntu:focal + cpu: 4 # Increase CPU and memory to avoid timeout + memory: 16G env: + MAKEJOBS: "-j8" FILE_ENV: "./ci/test/00_setup_env_native_fuzz.sh" task: From 82a6aa590780dfe2159ab2add1c7f3be5484c111 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 3 May 2021 19:45:59 +0200 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#21810: fuzz: Various RPC fuzzer follow-ups 5252f86eb616a1112e427c268c8e8825f2a63d67 fuzz: Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of #ifdef forests (practicalswift) 54549dda310e2becee9cb4997d1408a90e91934f fuzz: RPC fuzzer post-merge follow-ups. Remove unused includes. Update list of fuzzed RPC commands. (practicalswift) Pull request description: Various RPC fuzzer follow-ups: * Remove unused includes. * Update list of fuzzed RPC commands. * Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of `#ifdef` forests. Context: https://github.com/bitcoin/bitcoin/pull/21169#pullrequestreview-646723483 ACKs for top commit: MarcoFalke: Concept ACK 5252f86eb616a1112e427c268c8e8825f2a63d67 Tree-SHA512: 286d70798131706ffb157758e1c73f7f00ed96ce120c7d9dc849e672b283f1362df47b206cfec9da44d5debb5869225e721761dcd5c38a7d5d1019dc6c912ab2 --- src/test/fuzz/rpc.cpp | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 36572943492d5..a2156b95afac0 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -3,9 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include #include -#include #include #include #include @@ -70,18 +68,15 @@ const std::vector RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{ "addpeeraddress", // avoid DNS lookups "analyzepsbt", // avoid signed integer overflow in CFeeRate::GetFee(unsigned long) (https://github.com/bitcoin/bitcoin/issues/20607) "dumptxoutset", // avoid writing to disk -#ifdef ENABLE_WALLET "dumpwallet", // avoid writing to disk -#endif - "echoipc", // avoid assertion failure (Assertion `"EnsureAnyNodeContext(request.context).init" && check' failed.) - "generatetoaddress", // avoid timeout - "gettxoutproof", // avoid slow execution -#ifdef ENABLE_WALLET + "echoipc", // avoid assertion failure (Assertion `"EnsureAnyNodeContext(request.context).init" && check' failed.) + "generatetoaddress", // avoid prohibitively slow execution (when `num_blocks` is large) + "generatetodescriptor", // avoid prohibitively slow execution (when `nblocks` is large) + "gettxoutproof", // avoid prohibitively slow execution "importwallet", // avoid reading from disk "loadwallet", // avoid reading from disk -#endif - "mockscheduler", // avoid assertion failure (Assertion `delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}' failed.) "prioritisetransaction", // avoid signed integer overflow in CTxMemPool::PrioritiseTransaction(uint256 const&, long const&) (https://github.com/bitcoin/bitcoin/issues/20626) + "savemempool", // disabled as a precautionary measure: may take a file path argument in the future "setban", // avoid DNS lookups "stop", // avoid shutdown state }; @@ -107,7 +102,6 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "finalizepsbt", "generate", "generateblock", - "generatetodescriptor", "getaddednodeinfo", "getbestblockhash", "getblock", @@ -145,11 +139,11 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "joinpsbts", "listbanned", "logging", + "mockscheduler", "ping", "preciousblock", "pruneblockchain", "reconsiderblock", - "savemempool", "scantxoutset", "sendrawtransaction", "setmocktime", @@ -335,20 +329,6 @@ void initialize_rpc() std::terminate(); } } - for (const std::string& rpc_command : RPC_COMMANDS_SAFE_FOR_FUZZING) { - const bool supported_rpc_command = std::find(supported_rpc_commands.begin(), supported_rpc_commands.end(), rpc_command) != supported_rpc_commands.end(); - if (!supported_rpc_command) { - std::cerr << "Error: Unknown RPC command \"" << rpc_command << "\" found in RPC_COMMANDS_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; - std::terminate(); - } - } - for (const std::string& rpc_command : RPC_COMMANDS_NOT_SAFE_FOR_FUZZING) { - const bool supported_rpc_command = std::find(supported_rpc_commands.begin(), supported_rpc_commands.end(), rpc_command) != supported_rpc_commands.end(); - if (!supported_rpc_command) { - std::cerr << "Error: Unknown RPC command \"" << rpc_command << "\" found in RPC_COMMANDS_NOT_SAFE_FOR_FUZZING. Please update " << __FILE__ << ".\n"; - std::terminate(); - } - } const char* limit_to_rpc_command_env = std::getenv("LIMIT_TO_RPC_COMMAND"); if (limit_to_rpc_command_env != nullptr) { g_limit_to_rpc_command = std::string{limit_to_rpc_command_env}; From d89847f43e813ce873db8471f5360c7f49e265b9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 21 May 2021 09:03:22 +0200 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#22004: fuzz: Speed up transaction fuzz target bbbb51877a96c3473aeea5914f751eec7835b5c4 fuzz: Speed up transaction fuzz target (MarcoFalke) Pull request description: `hashBlock` and `include_addresses` are orthogonal, so no need to do an exhaustive "search". Might fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34491 ACKs for top commit: practicalswift: cr ACK bbbb51877a96c3473aeea5914f751eec7835b5c4: patch looks correct, and `TxToUniv` surprisingly wide in the `transaction_fuzz_target` flame graph! Putting it on a diet makes sense. Tree-SHA512: 1e7c30c7fecf96364a9a1597c0a22139389fdeb67db59f3c2c6fc088196e3332877b2865991a957980d542f99a2f48cc066dd7cc16c695a5113190fe06205089 --- src/test/fuzz/transaction.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 891e85936d2a5..8d2e595e6c94c 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -94,9 +94,6 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) (void)AreInputsStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); - TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ true, u); - TxToUniv(tx, /* hashBlock */ {}, /* include_addresses */ false, u); - static const uint256 u256_max(uint256S("ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")); - TxToUniv(tx, u256_max, /* include_addresses */ true, u); - TxToUniv(tx, u256_max, /* include_addresses */ false, u); + TxToUniv(tx, /* hashBlock */ uint256::ZERO, /* include_addresses */ true, u); + TxToUniv(tx, /* hashBlock */ uint256::ONE, /* include_addresses */ false, u); } From 06ea87040b8d90c6376475c8de0182d155552ec1 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 19 May 2021 15:57:15 +0200 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#20773: refactor: split CWallet::Create 489ebb7b34c403a3ce78ff6fb271f8e6ecb47304 wallet: make chain optional for CWallet::Create (Ivan Metlushko) d73ae939649f3b30e52b5a2cccd7fafd1ab36766 CWallet::Create move chain init message up into calling code (Ivan Metlushko) 44c430ffac940e1d1dd7f5939a495470bc694489 refactor: Add CWallet:::AttachChain method (Russell Yanofsky) e2a47ce08528dfb39c0340145c6977f6afd587f6 refactor: move first run detection to client code (Ivan Metlushko) Pull request description: This is a followup for https://github.com/bitcoin/bitcoin/pull/20365#discussion_r522265003 First part of a refactoring with overall goal to simplify `CWallet` and de-duplicate code with `wallettool` **Rationale**: split `CWallet::Create` and create `CWallet::AttachChain`. `CWallet::AttachChain` takes chain as first parameter on purpose. In future I suggest we can remove `chain` from `CWallet` constructor. The second commit is based on be164f9cf89b123f03b926aa980996919924ee64 from #15719 (thanks ryanofsky) cc ryanofsky achow101 ACKs for top commit: ryanofsky: Code review ACK 489ebb7b34c403a3ce78ff6fb271f8e6ecb47304. Only changes since last review were adding a const variable declaration, and implementing suggestion not to move feerate option checks to AttachChain. Thanks for updates and fast responses! Tree-SHA512: 00235abfe1b00874c56c449adcab8a36582424abb9ba27440bf750af8f3f217b68c11ca74eb30f78a2109ad1d9009315480effc78345e16a3074a1b5d8128721 --- src/bench/wallet_balance.cpp | 3 +- src/qt/test/addressbooktests.cpp | 3 +- src/qt/test/wallettests.cpp | 3 +- src/wallet/dump.cpp | 3 +- src/wallet/load.cpp | 3 +- src/wallet/test/coinjoin_tests.cpp | 3 +- src/wallet/test/coinselector_tests.cpp | 15 ++-- src/wallet/test/wallet_test_fixture.cpp | 3 +- src/wallet/test/wallet_tests.cpp | 29 +++--- src/wallet/wallet.cpp | 114 ++++++++++++++---------- src/wallet/wallet.h | 11 ++- src/wallet/wallettool.cpp | 3 +- 12 files changed, 106 insertions(+), 87 deletions(-) diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 166e8d3d49a48..cf70367a30f1b 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -21,8 +21,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); - bool first_run; - if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); + if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index 83c3b8c7a3065..5eb00a1c32ce8 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -62,8 +62,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); auto build_address = [wallet]() { CKey key; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index f66dd73830bba..15140e9645cad 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -110,8 +110,7 @@ void TestGUI(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase()); AddWallet(wallet); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index cad59a72f5970..ef1431a17ce62 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -202,8 +202,7 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling std::shared_ptr wallet(new CWallet(nullptr /* chain */, /*coinjoin_loader=*/ nullptr, name, std::move(database)), WalletToolReleaseWallet); { LOCK(wallet->cs_wallet); - bool first_run = true; - DBErrors load_wallet_ret = wallet->LoadWallet(first_run); + DBErrors load_wallet_ret = wallet->LoadWallet(); if (load_wallet_ret != DBErrors::LOAD_OK) { error = strprintf(_("Error creating %s"), name); return false; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2ba9149f1b6f0..66647d6804a4d 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -110,7 +110,8 @@ bool LoadWallets(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoi if (!database && status == DatabaseStatus::FAILED_NOT_FOUND) { continue; } - std::shared_ptr pwallet = database ? CWallet::Create(chain, coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr pwallet = database ? CWallet::Create(&chain, coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error_string); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index c807888e42e93..d6d3b174e3653 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -132,8 +132,7 @@ class CTransactionBuilderTestSetup : public TestChain100Setup CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); wallet = std::make_unique(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddWallet(wallet); { LOCK2(wallet->cs_wallet, cs_main); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c7ce15447b945..960f1c29aa054 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -259,8 +259,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); LOCK(wallet->cs_wallet); bool bnb_used; @@ -283,8 +282,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -309,8 +307,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_AUTO_TEST_CASE(knapsack_solver_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -591,8 +588,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); @@ -615,8 +611,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) BOOST_AUTO_TEST_CASE(SelectCoins_test) { std::unique_ptr wallet = std::make_unique(m_node.chain.get(), /* coinjoin_loader = */ nullptr, "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 349797b8cc2be..8d882e80defa7 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -10,8 +10,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, m_node.coinjoin_loader, *Assert(m_node.args))}, m_wallet(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()) { - bool fFirstRun; - m_wallet.LoadWallet(fFirstRun); + m_wallet.LoadWallet(); m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_wallet_loader->registerRpcs(); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 369c0e8048840..8595b56b57e78 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -51,15 +51,17 @@ namespace { constexpr CAmount fallbackFee = 1000; } // anonymous namespace -static std::shared_ptr TestLoadWallet(NodeContext& node) +static std::shared_ptr TestLoadWallet(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader) { DatabaseOptions options; DatabaseStatus status; bilingual_str error; std::vector warnings; auto database = MakeWalletDatabase("", options, status, error); - auto wallet = CWallet::Create(*node.chain, *node.coinjoin_loader, "", std::move(database), options.create_flags, error, warnings); - wallet->postInitProcess(); + auto wallet = CWallet::Create(chain, coinjoin_loader, "", std::move(database), options.create_flags, error, warnings); + if (chain) { + wallet->postInitProcess(); + } return wallet; } @@ -581,8 +583,7 @@ class ListCoinsTestingSetup : public TestChain100Setup LOCK(wallet->cs_wallet); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); reserver.reserve(); @@ -715,8 +716,7 @@ class CreateTransactionTestSetup : public TestChain100Setup { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); wallet = std::make_unique(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddWallet(wallet); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); @@ -1232,7 +1232,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); // Create new wallet with known key and unload it. - auto wallet = TestLoadWallet(m_node); + auto wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); @@ -1272,7 +1272,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // Reload wallet and make sure new transactions are detected despite events // being blocked - wallet = TestLoadWallet(m_node); + wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); BOOST_CHECK(rescan_completed); BOOST_CHECK_EQUAL(addtx_count, 2); { @@ -1311,7 +1311,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); ENTER_CRITICAL_SECTION(cs_wallets); }); - wallet = TestLoadWallet(m_node); + wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); BOOST_CHECK_EQUAL(addtx_count, 4); { LOCK(wallet->cs_wallet); @@ -1390,11 +1390,18 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) BOOST_CHECK_EXCEPTION(vr >> w_desc, std::ios_base::failure, malformed_descriptor); } +BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) +{ + auto wallet = TestLoadWallet(nullptr, *m_node.coinjoin_loader); + BOOST_CHECK(wallet); + UnloadWallet(std::move(wallet)); +} + BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); auto chain = interfaces::MakeChain(m_node); - auto wallet = TestLoadWallet(m_node); + auto wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b3cf5abdb3f7a..a5657cff41a7d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -238,7 +238,8 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, interfaces return nullptr; } - std::shared_ptr wallet = CWallet::Create(chain, coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr wallet = CWallet::Create(&chain, coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -303,7 +304,8 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, interfaces::Coin } // Make the wallet - std::shared_ptr wallet = CWallet::Create(chain, coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr wallet = CWallet::Create(&chain, coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -3966,11 +3968,10 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } } -DBErrors CWallet::LoadWallet(bool& fFirstRunRet) +DBErrors CWallet::LoadWallet() { LOCK(cs_wallet); - fFirstRunRet = false; DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { @@ -3983,9 +3984,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } - // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys - fFirstRunRet = m_spk_managers.empty() && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); - if (fFirstRunRet) { + if (m_spk_managers.empty()) { assert(m_external_spk_managers == nullptr); assert(m_internal_spk_managers == nullptr); } @@ -4677,22 +4676,19 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) +std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) { const std::string& walletFile = database->Filename(); - chain.initMessage(_("Loading wallet…").translated); - int64_t nStart = GetTimeMillis(); - bool fFirstRun = true; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr walletInstance(new CWallet(&chain, &coinjoin_loader, name, std::move(database)), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(chain, &coinjoin_loader, name, std::move(database)), ReleaseWallet); // TODO: refactor this condition: validation of error looks like workaround if (!walletInstance->AutoBackupWallet(fs::PathFromString(walletFile), error, warnings) && !error.original.empty()) { return nullptr; } - DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); + DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { @@ -4720,6 +4716,10 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } } + // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys + const bool fFirstRun = walletInstance->m_spk_managers.empty() && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); if (fFirstRun) { walletInstance->SetMinVersion(FEATURE_LATEST); @@ -4790,7 +4790,9 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } } - walletInstance->chainStateFlushed(chain.getTipLocator()); + if (chain) { + walletInstance->chainStateFlushed(chain->getTipLocator()); + } // Try to create wallet backup right after new wallet was created bilingual_str strBackupError; @@ -4893,9 +4895,9 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C _("This is the transaction fee you will pay if you send a transaction.")); } walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; - if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) { + if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), - gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -4908,16 +4910,16 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C } else if (max_fee.value() > HIGH_MAX_TX_FEE) { warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); } - if (CFeeRate{max_fee.value(), 1000} < chain.relayMinFee()) { + if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } walletInstance->m_default_max_tx_fee = max_fee.value(); } - if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) + if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + _("The wallet will avoid paying less than the minimum relay fee.")); @@ -4931,6 +4933,41 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C LOCK(walletInstance->cs_wallet); + if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { + return nullptr; + } + + coinjoin_loader.AddWallet(*walletInstance); + + { + LOCK(cs_wallets); + for (auto& load_wallet : g_load_wallet_fns) { + load_wallet(interfaces::MakeWallet(walletInstance)); + } + } + + walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); + + { + walletInstance->WalletLogPrintf("setExternalKeyPool.size() = %u\n", walletInstance->KeypoolCountExternalKeys()); + walletInstance->WalletLogPrintf("GetKeyPoolSize() = %u\n", walletInstance->GetKeyPoolSize()); + walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); + walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); + for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { + walletInstance->WalletLogPrintf("nTimeFirstKey = %u\n", spk_man->GetTimeFirstKey()); + } + } + + return walletInstance; +} + +bool CWallet::AttachChain(const std::shared_ptr& walletInstance, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings) +{ + LOCK(walletInstance->cs_wallet); + // allow setting the chain if it hasn't been set already but prevent changing it + assert(!walletInstance->m_chain || walletInstance->m_chain == &chain); + walletInstance->m_chain = &chain; + // Register wallet with validationinterface. It's done before rescan to avoid // missing block connections between end of rescan and validation subscribing. // Because of wallet lock being hold, block connection notifications are going to @@ -4964,21 +5001,21 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C if (tip_height && *tip_height != rescan_height) { - // We can't rescan beyond non-pruned blocks, stop and throw an error. - // This might happen if a user uses an old wallet within a pruned node - // or if they ran -disablewallet for a longer time, then decided to re-enable if (chain.havePruned()) { - // Exit early and print an error. - // If a block is pruned after this check, we will load the wallet, - // but fail the rescan with a generic error. int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height; } if (rescan_height != block_height) { + // We can't rescan beyond non-pruned blocks, stop and throw an error. + // This might happen if a user uses an old wallet within a pruned node + // or if they ran -disablewallet for a longer time, then decided to re-enable + // Exit early and print an error. + // If a block is pruned after this check, we will load the wallet, + // but fail the rescan with a generic error. error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); - return nullptr; + return false; } } @@ -5003,35 +5040,14 @@ std::shared_ptr CWallet::Create(interfaces::Chain& chain, interfaces::C WalletRescanReserver reserver(*walletInstance); if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { error = _("Failed to rescan the wallet during initialization"); - return nullptr; + return false; } } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->GetDatabase().IncrementUpdateCounter(); } - coinjoin_loader.AddWallet(*walletInstance); - - { - LOCK(cs_wallets); - for (auto& load_wallet : g_load_wallet_fns) { - load_wallet(interfaces::MakeWallet(walletInstance)); - } - } - - walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); - - { - walletInstance->WalletLogPrintf("setExternalKeyPool.size() = %u\n", walletInstance->KeypoolCountExternalKeys()); - walletInstance->WalletLogPrintf("GetKeyPoolSize() = %u\n", walletInstance->GetKeyPoolSize()); - walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); - for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) { - walletInstance->WalletLogPrintf("nTimeFirstKey = %u\n", spk_man->GetTimeFirstKey()); - } - } - - return walletInstance; + return true; } bool CWallet::UpgradeWallet(int version, bilingual_str& error) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0a1cc80e19371..5e514d56cd790 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -847,6 +847,13 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize); + /** + * Catch wallet up to current chain, scanning new blocks, updating the best + * block locator and m_last_block_processed, and registering for + * notifications about new blocks and transactions. + */ + static bool AttachChain(const std::shared_ptr& wallet, interfaces::Chain& chain, bilingual_str& error, std::vector& warnings); + public: /** * Main wallet lock. @@ -1247,7 +1254,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati CAmount GetChange(const CTransaction& tx) const; void chainStateFlushed(const CBlockLocator& loc) override; - DBErrors LoadWallet(bool& fFirstRunRet); + DBErrors LoadWallet(); void AutoLockMasternodeCollaterals(); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1338,7 +1345,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool ResendTransaction(const uint256& hashTx); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr Create(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); + static std::shared_ptr Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); /** * Wallet post-init setup diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 64952df6f3dcb..96ca344c4e545 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -64,8 +64,7 @@ static std::shared_ptr MakeWallet(const std::string& name, const fs::pa std::shared_ptr wallet_instance{new CWallet(/*chain=*/ nullptr, /*coinjoin_loader=*/ nullptr, name, std::move(database)), WalletToolReleaseWallet}; DBErrors load_wallet_ret; try { - bool first_run; - load_wallet_ret = wallet_instance->LoadWallet(first_run); + load_wallet_ret = wallet_instance->LoadWallet(); } catch (const std::runtime_error&) { tfm::format(std::cerr, "Error loading %s. Is wallet being used by another process?\n", name); return nullptr; From 4125485401ac74e3473776fd9b152aa1a8c0ea26 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sun, 28 Jul 2024 16:08:59 +0700 Subject: [PATCH 09/10] fix: follow-up bitcoin#20773 - for collateral lock/unlock coins --- src/wallet/wallet.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a5657cff41a7d..fb3f5e8ba9552 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -917,8 +917,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio outputs.emplace_back(wtx.tx, i); } } - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); + // TODO: refactor duplicated code between CWallet::AddToWallet and CWallet::AutoLockMasternodeCollaterals + if (m_chain) { + for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { + LockCoin(outPoint); + } } } @@ -946,8 +949,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio } } } - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); + // TODO: refactor duplicated code with case fInstertedNew + if (m_chain) { + for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { + LockCoin(outPoint); + } } } @@ -4018,6 +4024,8 @@ DBErrors CWallet::LoadWallet() // This avoids accidental spending of collaterals. They can still be unlocked manually if a spend is really intended. void CWallet::AutoLockMasternodeCollaterals() { + if (!m_chain) return; + std::vector> outputs; LOCK(cs_wallet); @@ -4452,6 +4460,11 @@ void CWallet::ListLockedCoins(std::vector& vOutpts) const void CWallet::ListProTxCoins(std::vector& vOutpts) const { + // TODO: refactor duplicated code between CWallet::AutoLockMasternodeCollaterals and CWallet::ListProTxCoins + if (!m_chain) { + vOutpts.clear(); + return; + } std::vector> outputs; AssertLockHeld(cs_wallet); From 9ad26f1664821804aeed9b82d055093cc078c143 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 29 Jul 2024 12:29:45 +0700 Subject: [PATCH 10/10] fix: follow-up bitcoin#20773 - coinjoin loader can be nullptr too --- src/wallet/load.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 13 +++++++------ src/wallet/wallet.cpp | 12 +++++++----- src/wallet/wallet.h | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 66647d6804a4d..d09fd1f5553b3 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -111,7 +111,7 @@ bool LoadWallets(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoi continue; } chain.initMessage(_("Loading wallet...").translated); - std::shared_ptr pwallet = database ? CWallet::Create(&chain, coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; + std::shared_ptr pwallet = database ? CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), options.create_flags, error_string, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error_string); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 8595b56b57e78..9e6089e617e4d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -51,7 +51,7 @@ namespace { constexpr CAmount fallbackFee = 1000; } // anonymous namespace -static std::shared_ptr TestLoadWallet(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader) +static std::shared_ptr TestLoadWallet(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader) { DatabaseOptions options; DatabaseStatus status; @@ -1232,7 +1232,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); // Create new wallet with known key and unload it. - auto wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); + auto wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); @@ -1272,7 +1272,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // Reload wallet and make sure new transactions are detected despite events // being blocked - wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); + wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); BOOST_CHECK(rescan_completed); BOOST_CHECK_EQUAL(addtx_count, 2); { @@ -1311,7 +1311,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); ENTER_CRITICAL_SECTION(cs_wallets); }); - wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); + wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); BOOST_CHECK_EQUAL(addtx_count, 4); { LOCK(wallet->cs_wallet); @@ -1392,7 +1392,8 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) { - auto wallet = TestLoadWallet(nullptr, *m_node.coinjoin_loader); + // TODO: FIX FIX FIX - coinjoin_loader is null heere! + auto wallet = TestLoadWallet(nullptr, nullptr); BOOST_CHECK(wallet); UnloadWallet(std::move(wallet)); } @@ -1401,7 +1402,7 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); auto chain = interfaces::MakeChain(m_node); - auto wallet = TestLoadWallet(m_node.chain.get(), *m_node.coinjoin_loader); + auto wallet = TestLoadWallet(m_node.chain.get(), m_node.coinjoin_loader.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fb3f5e8ba9552..bc6bc8f4b1a12 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -239,7 +239,7 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, interfaces } chain.initMessage(_("Loading wallet...").translated); - std::shared_ptr wallet = CWallet::Create(&chain, coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); + std::shared_ptr wallet = CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -305,7 +305,7 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, interfaces::Coin // Make the wallet chain.initMessage(_("Loading wallet...").translated); - std::shared_ptr wallet = CWallet::Create(&chain, coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); + std::shared_ptr wallet = CWallet::Create(&chain, &coinjoin_loader, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -4689,14 +4689,14 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) +std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) { const std::string& walletFile = database->Filename(); int64_t nStart = GetTimeMillis(); // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr walletInstance(new CWallet(chain, &coinjoin_loader, name, std::move(database)), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(chain, coinjoin_loader, name, std::move(database)), ReleaseWallet); // TODO: refactor this condition: validation of error looks like workaround if (!walletInstance->AutoBackupWallet(fs::PathFromString(walletFile), error, warnings) && !error.original.empty()) { return nullptr; @@ -4950,7 +4950,9 @@ std::shared_ptr CWallet::Create(interfaces::Chain* chain, interfaces::C return nullptr; } - coinjoin_loader.AddWallet(*walletInstance); + if (coinjoin_loader) { + coinjoin_loader->AddWallet(*walletInstance); + } { LOCK(cs_wallets); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5e514d56cd790..4487aa17b95dc 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1345,7 +1345,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool ResendTransaction(const uint256& hashTx); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); + static std::shared_ptr Create(interfaces::Chain* chain, interfaces::CoinJoin::Loader* coinjoin_loader, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings); /** * Wallet post-init setup