diff options
Diffstat (limited to 'src')
72 files changed, 1127 insertions, 805 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 0068c94070..91cdecad40 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -70,6 +70,7 @@ FUZZ_TARGETS = \ test/fuzz/message \ test/fuzz/messageheader_deserialize \ test/fuzz/multiplication_overflow \ + test/fuzz/net \ test/fuzz/net_permissions \ test/fuzz/netaddr_deserialize \ test/fuzz/netaddress \ @@ -132,6 +133,8 @@ FUZZ_TARGETS = \ test/fuzz/script_sigcache \ test/fuzz/script_sign \ test/fuzz/scriptnum_ops \ + test/fuzz/secp256k1_ec_seckey_import_export_der \ + test/fuzz/secp256k1_ecdsa_signature_parse_der_lax \ test/fuzz/service_deserialize \ test/fuzz/signature_checker \ test/fuzz/snapshotmetadata_deserialize \ @@ -722,6 +725,12 @@ test_fuzz_multiplication_overflow_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_multiplication_overflow_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_multiplication_overflow_SOURCES = test/fuzz/multiplication_overflow.cpp +test_fuzz_net_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_net_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_net_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_net_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_net_SOURCES = test/fuzz/net.cpp + test_fuzz_net_permissions_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_net_permissions_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_net_permissions_LDADD = $(FUZZ_SUITE_LD_COMMON) @@ -1094,6 +1103,18 @@ test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp +test_fuzz_secp256k1_ec_seckey_import_export_der_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_secp256k1_ec_seckey_import_export_der_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_secp256k1_ec_seckey_import_export_der_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_secp256k1_ec_seckey_import_export_der_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_secp256k1_ec_seckey_import_export_der_SOURCES = test/fuzz/secp256k1_ec_seckey_import_export_der.cpp + +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_LDADD = $(FUZZ_SUITE_LD_COMMON) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_secp256k1_ecdsa_signature_parse_der_lax_SOURCES = test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp + test_fuzz_service_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSERVICE_DESERIALIZE=1 test_fuzz_service_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_service_deserialize_LDADD = $(FUZZ_SUITE_LD_COMMON) diff --git a/src/base58.cpp b/src/base58.cpp index 18cd2090e0..0dc6044145 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -35,7 +35,7 @@ static const int8_t mapBase58[256] = { -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, }; -bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len) +NODISCARD static bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch, int max_ret_len) { // Skip leading spaces. while (*psz && IsSpace(*psz)) @@ -141,7 +141,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input) return EncodeBase58(vch); } -bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len) +NODISCARD static bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len) { if (!DecodeBase58(psz, vchRet, max_ret_len > std::numeric_limits<int>::max() - 4 ? std::numeric_limits<int>::max() : max_ret_len + 4) || (vchRet.size() < 4)) { diff --git a/src/base58.h b/src/base58.h index b87664b78b..468c3e2589 100644 --- a/src/base58.h +++ b/src/base58.h @@ -26,13 +26,6 @@ std::string EncodeBase58(Span<const unsigned char> input); /** - * Decode a base58-encoded string (psz) into a byte vector (vchRet). - * return true if decoding is successful. - * psz cannot be nullptr. - */ -NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len); - -/** * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ @@ -44,12 +37,6 @@ NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& std::string EncodeBase58Check(Span<const unsigned char> input); /** - * Decode a base58-encoded string (psz) that includes a checksum into a byte - * vector (vchRet), return true if decoding is successful - */ -NODISCARD bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet, int max_ret_len); - -/** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 01466d0b6f..012057e792 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -70,7 +70,10 @@ void benchmark::BenchRunner::RunAll(const Args& args) } std::cout << bench.complexityBigO() << std::endl; } - benchmarkResults.push_back(bench.results().back()); + + if (!bench.results().empty()) { + benchmarkResults.push_back(bench.results().back()); + } } GenerateTemplateResults(benchmarkResults, args.output_csv, "# Benchmark, evals, iterations, total, min, max, median\n" diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 19d7bc0dbc..ffa772d8c1 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -14,8 +14,6 @@ #include <vector> - -static const int MIN_CORES = 2; static const size_t BATCHES = 101; static const size_t BATCH_SIZE = 30; static const int PREVECTOR_SIZE = 28; @@ -26,6 +24,9 @@ static const unsigned int QUEUE_BATCH_SIZE = 128; // and there is a little bit of work done between calls to Add. static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) { + // We shouldn't ever be running with the checkqueue on a single core machine. + if (GetNumCores() <= 1) return; + const ECCVerifyHandle verify_handle; ECC_Start(); @@ -44,7 +45,9 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) }; CCheckQueue<PrevectorJob> queue {QUEUE_BATCH_SIZE}; boost::thread_group tg; - for (auto x = 0; x < std::max(MIN_CORES, GetNumCores()); ++x) { + // The main thread should be counted to prevent thread oversubscription, and + // to decrease the variance of benchmark results. + for (auto x = 0; x < GetNumCores() - 1; ++x) { tg.create_thread([&]{queue.Thread();}); } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 227626f40f..02074f820a 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -44,7 +44,6 @@ static void WaitForShutdown(NodeContext& node) static bool AppInit(int argc, char* argv[]) { NodeContext node; - node.chain = interfaces::MakeChain(node); bool fRet = false; @@ -144,7 +143,7 @@ static bool AppInit(int argc, char* argv[]) // If locking the data directory failed, exit immediately return false; } - fRet = AppInitMain(context, node); + fRet = AppInitInterfaces(node) && AppInitMain(context, node); } catch (const std::exception& e) { PrintExceptionContinue(&e, "AppInit()"); diff --git a/src/chain.h b/src/chain.h index 802e23f775..43e8a39f36 100644 --- a/src/chain.h +++ b/src/chain.h @@ -398,12 +398,6 @@ public: return vChain[nHeight]; } - /** Compare two chains efficiently. */ - friend bool operator==(const CChain &a, const CChain &b) { - return a.vChain.size() == b.vChain.size() && - a.vChain[a.vChain.size() - 1] == b.vChain[b.vChain.size() - 1]; - } - /** Efficiently check whether a block is present in this chain. */ bool Contains(const CBlockIndex *pindex) const { return (*this)[pindex->nHeight] == pindex; diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 380d4eb8ac..8d2dcd0279 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -4,11 +4,8 @@ #include <util/system.h> #include <walletinitinterface.h> -#include <support/allocators/secure.h> class CWallet; -enum class WalletCreationStatus; -struct bilingual_str; namespace interfaces { class Chain; @@ -49,7 +46,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-walletdir=<dir>", "-walletnotify=<cmd>", "-walletrbf", - "-zapwallettxes=<mode>", "-dblogsize=<n>", "-flushwallet", "-privdb", @@ -59,37 +55,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const const WalletInitInterface& g_wallet_init_interface = DummyWalletInit(); -fs::path GetWalletDir() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::vector<fs::path> ListWalletDir() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::vector<std::shared_ptr<CWallet>> GetWallets() -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - -using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wallet)>; -std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet) -{ - throw std::logic_error("Wallet function called in non-wallet build."); -} - namespace interfaces { std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) diff --git a/src/init.cpp b/src/init.cpp index ecd57960ad..bb93f5c797 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1229,6 +1229,17 @@ bool AppInitLockDataDirectory() return true; } +bool AppInitInterfaces(NodeContext& node) +{ + node.chain = interfaces::MakeChain(node); + // Create client interfaces for wallets that are supposed to be loaded + // according to -wallet and -disablewallet options. This only constructs + // the interfaces, it doesn't load wallet data. Wallets actually get loaded + // when load() and start() interface methods are called below. + g_wallet_init_interface.Construct(node); + return true; +} + bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const ArgsManager& args = *Assert(node.args); @@ -1318,12 +1329,6 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA GetMainSignals().RegisterBackgroundSignalScheduler(*node.scheduler); - // Create client interfaces for wallets that are supposed to be loaded - // according to -wallet and -disablewallet options. This only constructs - // the interfaces, it doesn't load wallet data. Wallets actually get loaded - // when load() and start() interface methods are called below. - g_wallet_init_interface.Construct(node); - /* Register RPC commands regardless of -server setting so they will be * available in the GUI RPC console even if external calls are disabled. */ @@ -1550,7 +1555,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA const int64_t load_block_index_start_time = GetTimeMillis(); try { LOCK(cs_main); - chainman.InitializeChainstate(); + chainman.InitializeChainstate(*Assert(node.mempool)); chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; @@ -1828,20 +1833,15 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA } #if HAVE_SYSTEM - if (args.IsArgSet("-blocknotify")) { - const std::string block_notify = args.GetArg("-blocknotify", ""); - const auto BlockNotifyCallback = [block_notify](SynchronizationState sync_state, const CBlockIndex* pBlockIndex) { - if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex) - return; - - std::string strCmd = block_notify; - if (!strCmd.empty()) { - boost::replace_all(strCmd, "%s", pBlockIndex->GetBlockHash().GetHex()); - std::thread t(runCommand, strCmd); - t.detach(); // thread runs free - } - }; - uiInterface.NotifyBlockTip_connect(BlockNotifyCallback); + const std::string block_notify = args.GetArg("-blocknotify", ""); + if (!block_notify.empty()) { + uiInterface.NotifyBlockTip_connect([block_notify](SynchronizationState sync_state, const CBlockIndex* pBlockIndex) { + if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex) return; + std::string command = block_notify; + boost::replace_all(command, "%s", pBlockIndex->GetBlockHash().GetHex()); + std::thread t(runCommand, command); + t.detach(); // thread runs free + }); } #endif diff --git a/src/init.h b/src/init.h index ce12a80dc7..679e875da1 100644 --- a/src/init.h +++ b/src/init.h @@ -53,6 +53,10 @@ bool AppInitSanityChecks(); */ bool AppInitLockDataDirectory(); /** + * Initialize node and wallet interface pointers. Has no prerequisites or side effects besides allocating memory. + */ +bool AppInitInterfaces(NodeContext& node); +/** * Bitcoin core main initialization. * @note This should only be done after daemonization. Call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 053d40335f..6e50ccb27a 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -314,24 +314,11 @@ public: //! Set mock time. virtual void setMockTime(int64_t time) = 0; - - //! Return interfaces for accessing wallets (if any). - virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; }; //! Return implementation of Chain interface. std::unique_ptr<Chain> MakeChain(NodeContext& node); -//! Return implementation of ChainClient interface for a wallet client. This -//! function will be undefined in builds where ENABLE_WALLET is false. -//! -//! Currently, wallets are the only chain clients. But in the future, other -//! types of chain clients could be added, such as tools for monitoring, -//! analysis, or fee estimation. These clients need to expose their own -//! MakeXXXClient functions returning their implementations of the ChainClient -//! interface. -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); - } // namespace interfaces #endif // BITCOIN_INTERFACES_CHAIN_H diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 206262eb03..2c5f8627e6 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -27,6 +27,7 @@ #include <support/allocators/secure.h> #include <sync.h> #include <txmempool.h> +#include <util/check.h> #include <util/ref.h> #include <util/system.h> #include <util/translation.h> @@ -41,16 +42,7 @@ #include <boost/signals2/signal.hpp> -class CWallet; -fs::path GetWalletDir(); -std::vector<fs::path> ListWalletDir(); -std::vector<std::shared_ptr<CWallet>> GetWallets(); -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings); -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result); -std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet); - namespace interfaces { - namespace { class NodeImpl : public Node @@ -64,11 +56,10 @@ public: bool baseInitialize() override { return AppInitBasicSetup(gArgs) && AppInitParameterInteraction(gArgs) && AppInitSanityChecks() && - AppInitLockDataDirectory(); + AppInitLockDataDirectory() && AppInitInterfaces(*m_context); } bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override { - m_context->chain = MakeChain(*m_context); return AppInitMain(m_context_ref, *m_context, tip_info); } void appShutdown() override @@ -240,36 +231,9 @@ public: LOCK(::cs_main); return ::ChainstateActive().CoinsTip().GetCoin(output, coin); } - std::string getWalletDir() override - { - return GetWalletDir().string(); - } - std::vector<std::string> listWalletDir() override - { - std::vector<std::string> paths; - for (auto& path : ListWalletDir()) { - paths.push_back(path.string()); - } - return paths; - } - std::vector<std::unique_ptr<Wallet>> getWallets() override + WalletClient& walletClient() override { - std::vector<std::unique_ptr<Wallet>> wallets; - for (auto& client : m_context->chain_clients) { - auto client_wallets = client->getWallets(); - std::move(client_wallets.begin(), client_wallets.end(), std::back_inserter(wallets)); - } - return wallets; - } - std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override - { - return MakeWallet(LoadWallet(*m_context->chain, name, error, warnings)); - } - std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override - { - std::shared_ptr<CWallet> wallet; - status = CreateWallet(*m_context->chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); - return MakeWallet(wallet); + return *Assert(m_context->wallet_client); } std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override { @@ -287,10 +251,6 @@ public: { return MakeHandler(::uiInterface.ShowProgress_connect(fn)); } - std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override - { - return HandleLoadWallet(std::move(fn)); - } std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { return MakeHandler(::uiInterface.NotifyNumConnectionsChanged_connect(fn)); diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 0cff7ae3a1..5079be038e 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -29,14 +29,13 @@ class RPCTimerInterface; class UniValue; class proxyType; enum class SynchronizationState; -enum class WalletCreationStatus; struct CNodeStateStats; struct NodeContext; struct bilingual_str; namespace interfaces { class Handler; -class Wallet; +class WalletClient; struct BlockTip; //! Block and header tip information @@ -173,22 +172,8 @@ public: //! Get unspent outputs associated with a transaction. virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0; - //! Return default wallet directory. - virtual std::string getWalletDir() = 0; - - //! Return available wallets in wallet directory. - virtual std::vector<std::string> listWalletDir() = 0; - - //! Return interfaces for accessing wallets (if any). - virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; - - //! Attempts to load a wallet from file or directory. - //! The loaded wallet is also notified to handlers previously registered - //! with handleLoadWallet. - virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; - - //! Create a wallet from file - virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) = 0; + //! Get wallet client. + virtual WalletClient& walletClient() = 0; //! Register handler for init messages. using InitMessageFn = std::function<void(const std::string& message)>; @@ -210,10 +195,6 @@ public: using ShowProgressFn = std::function<void(const std::string& title, int progress, bool resume_possible)>; virtual std::unique_ptr<Handler> handleShowProgress(ShowProgressFn fn) = 0; - //! Register handler for load wallet messages. - using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; - virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; - //! Register handler for number of connections changed messages. using NotifyNumConnectionsChangedFn = std::function<void(int new_num_connections)>; virtual std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 7fd24425cf..63c109658e 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -37,6 +37,7 @@ namespace { //! Construct wallet tx struct. WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) { + LOCK(wallet.cs_wallet); WalletTx result; result.tx = wtx.tx; result.txin_is_mine.reserve(wtx.tx->vin.size()); @@ -132,7 +133,11 @@ public: { return m_wallet->SignMessage(message, pkhash, str_sig); } - bool isSpendable(const CTxDestination& dest) override { return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; } + bool isSpendable(const CTxDestination& dest) override + { + LOCK(m_wallet->cs_wallet); + return m_wallet->IsMine(dest) & ISMINE_SPENDABLE; + } bool haveWatchOnly() override { auto spk_man = m_wallet->GetLegacyScriptPubKeyMan(); @@ -480,7 +485,7 @@ public: std::shared_ptr<CWallet> m_wallet; }; -class WalletClientImpl : public ChainClient +class WalletClientImpl : public WalletClient { public: WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) @@ -489,6 +494,9 @@ public: m_context.chain = &chain; m_context.args = &args; } + ~WalletClientImpl() override { UnloadWallets(); } + + //! ChainClient methods void registerRpcs() override { for (const CRPCCommand& command : GetWalletRPCCommands()) { @@ -504,6 +512,30 @@ public: void flush() override { return FlushWallets(); } void stop() override { return StopWallets(); } void setMockTime(int64_t time) override { return SetMockTime(time); } + + //! WalletClient methods + std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) override + { + std::shared_ptr<CWallet> wallet; + status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); + return MakeWallet(std::move(wallet)); + } + std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override + { + return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), error, warnings)); + } + std::string getWalletDir() override + { + return GetWalletDir().string(); + } + std::vector<std::string> listWalletDir() override + { + std::vector<std::string> paths; + for (auto& path : ListWalletDir()) { + paths.push_back(path.string()); + } + return paths; + } std::vector<std::unique_ptr<Wallet>> getWallets() override { std::vector<std::unique_ptr<Wallet>> wallets; @@ -512,7 +544,10 @@ public: } return wallets; } - ~WalletClientImpl() override { UnloadWallets(); } + std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override + { + return HandleLoadWallet(std::move(fn)); + } WalletContext m_context; const std::vector<std::string> m_wallet_filenames; @@ -524,7 +559,7 @@ public: std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; } -std::unique_ptr<ChainClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) +std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames) { return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames)); } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 3cdadbc72e..186f5d81a5 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -6,6 +6,7 @@ #define BITCOIN_INTERFACES_WALLET_H #include <amount.h> // For CAmount +#include <interfaces/chain.h> // For ChainClient #include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation) #include <script/standard.h> // For CTxDestination #include <support/allocators/secure.h> // For SecureString @@ -28,9 +29,11 @@ class CWallet; enum class FeeReason; enum class OutputType; enum class TransactionError; +enum class WalletCreationStatus; enum isminetype : unsigned int; struct CRecipient; struct PartiallySignedTransaction; +struct WalletContext; struct bilingual_str; typedef uint8_t isminefilter; @@ -301,6 +304,34 @@ public: virtual CWallet* wallet() { return nullptr; } }; +//! Wallet chain client that in addition to having chain client methods for +//! starting up, shutting down, and registering RPCs, also has additional +//! methods (called by the GUI) to load and create wallets. +class WalletClient : public ChainClient +{ +public: + //! Create new wallet. + virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + + //! Load existing wallet. + virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + + //! Return default wallet directory. + virtual std::string getWalletDir() = 0; + + //! Return available wallets in wallet directory. + virtual std::vector<std::string> listWalletDir() = 0; + + //! Return interfaces for accessing wallets (if any). + virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0; + + //! Register handler for load wallet messages. This callback is triggered by + //! createWallet and loadWallet above, and also triggered when wallets are + //! loaded at startup or by RPC. + using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>; + virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0; +}; + //! Information about one wallet address. struct WalletAddress { @@ -379,6 +410,10 @@ struct WalletTxOut //! dummywallet.cpp and throws if the wallet component is not compiled. std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet); +//! Return implementation of ChainClient interface for a wallet client. This +//! function will be undefined in builds where ENABLE_WALLET is false. +std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames); + } // namespace interfaces #endif // BITCOIN_INTERFACES_WALLET_H diff --git a/src/key.cpp b/src/key.cpp index 4ed74a39b1..868a8b9b0e 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -31,7 +31,7 @@ static secp256k1_context* secp256k1_context_sign = nullptr; * * out32 must point to an output buffer of length at least 32 bytes. */ -static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) { +int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out32, const unsigned char *seckey, size_t seckeylen) { const unsigned char *end = seckey + seckeylen; memset(out32, 0, 32); /* sequence header */ @@ -88,7 +88,7 @@ static int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char *out * will be set to the number of bytes used in the buffer. * key32 must point to a 32-byte raw private key. */ -static int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) { +int ec_seckey_export_der(const secp256k1_context *ctx, unsigned char *seckey, size_t *seckeylen, const unsigned char *key32, bool compressed) { assert(*seckeylen >= CKey::SIZE); secp256k1_pubkey pubkey; size_t pubkeylen = 0; diff --git a/src/net.cpp b/src/net.cpp index 883e57bdf0..e35d05cec0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -816,6 +816,7 @@ struct NodeEvictionCandidate CAddress addr; uint64_t nKeyedNetGroup; bool prefer_evict; + bool m_is_local; }; static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) @@ -828,6 +829,12 @@ static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, cons return a.nTimeConnected > b.nTimeConnected; } +static bool CompareLocalHostTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + if (a.m_is_local != b.m_is_local) return b.m_is_local; + return a.nTimeConnected > b.nTimeConnected; +} + static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) { return a.nKeyedNetGroup < b.nKeyedNetGroup; } @@ -849,6 +856,14 @@ static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEviction return a.nTimeConnected > b.nTimeConnected; } +// Pick out the potential block-relay only peers, and sort them by last block time. +static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) +{ + if (a.fRelayTxes != b.fRelayTxes) return a.fRelayTxes; + if (a.nLastBlockTime != b.nLastBlockTime) return a.nLastBlockTime < b.nLastBlockTime; + if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices; + return a.nTimeConnected > b.nTimeConnected; +} //! Sort an array by the specified comparator, then erase the last K elements. template<typename T, typename Comparator> @@ -891,7 +906,7 @@ bool CConnman::AttemptToEvictConnection() node->nLastBlockTime, node->nLastTXTime, HasAllDesirableServiceFlags(node->nServices), peer_relay_txes, peer_filter_not_null, node->addr, node->nKeyedNetGroup, - node->m_prefer_evict}; + node->m_prefer_evict, node->addr.IsLocal()}; vEvictionCandidates.push_back(candidate); } } @@ -904,15 +919,34 @@ bool CConnman::AttemptToEvictConnection() // Protect the 8 nodes with the lowest minimum ping time. // An attacker cannot manipulate this metric without physically moving nodes closer to the target. EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8); - // Protect 4 nodes that most recently sent us transactions. + // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool. // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4); - // Protect 4 nodes that most recently sent us blocks. + // Protect up to 8 non-tx-relay peers that have sent us novel blocks. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareNodeBlockRelayOnlyTime); + size_t erase_size = std::min(size_t(8), vEvictionCandidates.size()); + vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.end() - erase_size, vEvictionCandidates.end(), [](NodeEvictionCandidate const &n) { return !n.fRelayTxes && n.fRelevantServices; }), vEvictionCandidates.end()); + + // Protect 4 nodes that most recently sent us novel blocks. // An attacker cannot manipulate this metric without performing useful work. EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4); + // Protect the half of the remaining nodes which have been connected the longest. // This replicates the non-eviction implicit behavior, and precludes attacks that start later. - EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, vEvictionCandidates.size() / 2); + // Reserve half of these protected spots for localhost peers, even if + // they're not longest-uptime overall. This helps protect tor peers, which + // tend to be otherwise disadvantaged under our eviction criteria. + size_t initial_size = vEvictionCandidates.size(); + size_t total_protect_size = initial_size / 2; + + // Pick out up to 1/4 peers that are localhost, sorted by longest uptime. + std::sort(vEvictionCandidates.begin(), vEvictionCandidates.end(), CompareLocalHostTimeConnected); + size_t local_erase_size = total_protect_size / 2; + vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.end() - local_erase_size, vEvictionCandidates.end(), [](NodeEvictionCandidate const &n) { return n.m_is_local; }), vEvictionCandidates.end()); + // Calculate how many we removed, and update our total number of peers that + // we want to protect based on uptime accordingly. + total_protect_size -= initial_size - vEvictionCandidates.size(); + EraseLastKElements(vEvictionCandidates, ReverseCompareNodeTimeConnected, total_protect_size); if (vEvictionCandidates.empty()) return false; @@ -1843,41 +1877,45 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) // but inbound and manual peers do not use our outbound slots. Inbound peers // also have the added issue that they could be attacker controlled and used // to prevent us from connecting to particular hosts if we used them here. - switch(pnode->m_conn_type){ + switch (pnode->m_conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: break; - case ConnectionType::OUTBOUND: + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap)); - } + } // no default case, so the compiler can warn about missing cases } } - // Feeler Connections - // - // Design goals: - // * Increase the number of connectable addresses in the tried table. - // - // Method: - // * Choose a random address from new and attempt to connect to it if we can connect - // successfully it is added to tried. - // * Start attempting feeler connections only after node finishes making outbound - // connections. - // * Only make a feeler connection once every few minutes. - // + ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY; + int64_t nTime = GetTimeMicros(); bool fFeeler = false; - if (nOutboundFullRelay >= m_max_outbound_full_relay && nOutboundBlockRelay >= m_max_outbound_block_relay && !GetTryNewOutboundPeer()) { - int64_t nTime = GetTimeMicros(); // The current time right now (in microseconds). - if (nTime > nNextFeeler) { - nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); - fFeeler = true; - } else { - continue; - } + // Determine what type of connection to open. Opening + // OUTBOUND_FULL_RELAY connections gets the highest priority until we + // meet our full-relay capacity. Then we open BLOCK_RELAY connection + // until we hit our block-relay-only peer limit. + // GetTryNewOutboundPeer() gets set when a stale tip is detected, so we + // try opening an additional OUTBOUND_FULL_RELAY connection. If none of + // these conditions are met, check the nNextFeeler timer to decide if + // we should open a FEELER. + + if (nOutboundFullRelay < m_max_outbound_full_relay) { + // OUTBOUND_FULL_RELAY + } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { + conn_type = ConnectionType::BLOCK_RELAY; + } else if (GetTryNewOutboundPeer()) { + // OUTBOUND_FULL_RELAY + } else if (nTime > nNextFeeler) { + nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); + conn_type = ConnectionType::FEELER; + fFeeler = true; + } else { + // skip to next iteration of while loop + continue; } addrman.ResolveCollisions(); @@ -1944,23 +1982,6 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect) LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString()); } - ConnectionType conn_type; - // Determine what type of connection to open. If fFeeler is not - // set, open OUTBOUND connections until we meet our full-relay - // capacity. Then open BLOCK_RELAY connections until we hit our - // block-relay peer limit. Otherwise, default to opening an - // OUTBOUND connection. - if (fFeeler) { - conn_type = ConnectionType::FEELER; - } else if (nOutboundFullRelay < m_max_outbound_full_relay) { - conn_type = ConnectionType::OUTBOUND; - } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { - conn_type = ConnectionType::BLOCK_RELAY; - } else { - // GetTryNewOutboundPeer() is true - conn_type = ConnectionType::OUTBOUND; - } - OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type); } } @@ -2784,6 +2805,9 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hashContinue = uint256(); if (conn_type_in != ConnectionType::BLOCK_RELAY) { m_tx_relay = MakeUnique<TxRelay>(); + } + + if (RelayAddrsWithConn()) { m_addr_known = MakeUnique<CRollingBloomFilter>(5000, 0.001); } @@ -118,12 +118,54 @@ struct CSerializedNetMsg * information we have available at the time of opening or accepting the * connection. Aside from INBOUND, all types are initiated by us. */ enum class ConnectionType { - INBOUND, /**< peer initiated connections */ - OUTBOUND, /**< full relay connections (blocks, addrs, txns) made automatically. Addresses selected from AddrMan. */ - MANUAL, /**< connections to addresses added via addnode or the connect command line argument */ - FEELER, /**< short lived connections used to test address validity */ - BLOCK_RELAY, /**< only relay blocks to these automatic outbound connections. Addresses selected from AddrMan. */ - ADDR_FETCH, /**< short lived connections used to solicit addrs when starting the node without a populated AddrMan */ + /** + * Inbound connections are those initiated by a peer. This is the only + * property we know at the time of connection, until P2P messages are + * exchanged. + */ + INBOUND, + + /** + * These are the default connections that we use to connect with the + * network. There is no restriction on what is relayed- by default we relay + * blocks, addresses & transactions. We automatically attempt to open + * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan. + */ + OUTBOUND_FULL_RELAY, + + + /** + * We open manual connections to addresses that users explicitly inputted + * via the addnode RPC, or the -connect command line argument. Even if a + * manual connection is misbehaving, we do not automatically disconnect or + * add it to our discouragement filter. + */ + MANUAL, + + /** + * Feeler connections are short lived connections used to increase the + * number of connectable addresses in our AddrMan. Approximately every + * FEELER_INTERVAL, we attempt to connect to a random address from the new + * table. If successful, we add it to the tried table. + */ + FEELER, + + /** + * We use block-relay-only connections to help prevent against partition + * attacks. By not relaying transactions or addresses, these connections + * are harder to detect by a third party, thus helping obfuscate the + * network topology. We automatically attempt to open + * MAX_BLOCK_RELAY_ONLY_CONNECTIONS using addresses from our AddrMan. + */ + BLOCK_RELAY, + + /** + * AddrFetch connections are short lived connections used to solicit + * addresses from peers. These are initiated to addresses submitted via the + * -seednode command line argument, or under certain conditions when the + * AddrMan is empty. + */ + ADDR_FETCH, }; class NetEventsInterface; @@ -209,7 +251,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, ConnectionType conn_type = ConnectionType::OUTBOUND); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function<bool(CNode* pnode)> func); @@ -823,8 +865,8 @@ public: std::atomic_bool fPauseSend{false}; bool IsOutboundOrBlockRelayConn() const { - switch(m_conn_type) { - case ConnectionType::OUTBOUND: + switch (m_conn_type) { + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: return true; case ConnectionType::INBOUND: @@ -832,13 +874,13 @@ public: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: return false; - } + } // no default case, so the compiler can warn about missing cases assert(false); } bool IsFullOutboundConn() const { - return m_conn_type == ConnectionType::OUTBOUND; + return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY; } bool IsManualConn() const { @@ -861,17 +903,23 @@ public: return m_conn_type == ConnectionType::INBOUND; } + /* Whether we send addr messages over this connection */ + bool RelayAddrsWithConn() const + { + return m_conn_type != ConnectionType::BLOCK_RELAY; + } + bool ExpectServicesFromConn() const { - switch(m_conn_type) { + switch (m_conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: case ConnectionType::FEELER: return false; - case ConnectionType::OUTBOUND: + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: return true; - } + } // no default case, so the compiler can warn about missing cases assert(false); } @@ -886,13 +934,11 @@ public: // flood relay std::vector<CAddress> vAddrToSend; - std::unique_ptr<CRollingBloomFilter> m_addr_known = nullptr; + std::unique_ptr<CRollingBloomFilter> m_addr_known{nullptr}; bool fGetAddr{false}; std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; - bool IsAddrRelayPeer() const { return m_addr_known != nullptr; } - // List of block ids we still have announce. // There is no final sorting before sending, as they are always sent immediately // and in the order requested. @@ -932,8 +978,17 @@ public: // Used for headers announcements - unfiltered blocks to relay std::vector<uint256> vBlockHashesToAnnounce GUARDED_BY(cs_inventory); - // Block and TXN accept times + /** UNIX epoch time of the last block received from this peer that we had + * not yet seen (e.g. not already received from another peer), that passed + * preliminary validity checks and was saved to disk, even if we don't + * connect the block or it eventually fails connection. Used as an inbound + * peer eviction criterium in CConnman::AttemptToEvictConnection. */ std::atomic<int64_t> nLastBlockTime{0}; + + /** UNIX epoch time of the last transaction received from this peer that we + * had not yet seen (e.g. not already received from another peer) and that + * was accepted into our mempool. Used as an inbound peer eviction criterium + * in CConnman::AttemptToEvictConnection. */ std::atomic<int64_t> nLastTXTime{0}; // Ping time measurement: diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 60bdfbe9f5..ce4ac3cd75 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -278,12 +278,6 @@ struct CNodeState { const CService address; //! Whether we have a fully established connection. bool fCurrentlyConnected; - //! Accumulated misbehaviour score for this peer. - int nMisbehavior; - //! Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). - bool m_should_discourage; - //! String name of this peer (debugging/logging purposes). - const std::string name; //! The best known block we know this peer has announced. const CBlockIndex *pindexBestKnownBlock; //! The hash of the last unknown block this peer has announced. @@ -432,13 +426,10 @@ struct CNodeState { //! Whether this peer relays txs via wtxid bool m_wtxid_relay{false}; - CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : - address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), - m_is_manual_connection (is_manual) + CNodeState(CAddress addrIn, bool is_inbound, bool is_manual) + : address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual) { fCurrentlyConnected = false; - nMisbehavior = 0; - m_should_discourage = false; pindexBestKnownBlock = nullptr; hashLastUnknownBlock.SetNull(); pindexLastCommonBlock = nullptr; @@ -476,6 +467,50 @@ static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return &it->second; } +/** + * Data structure for an individual peer. This struct is not protected by + * cs_main since it does not contain validation-critical data. + * + * Memory is owned by shared pointers and this object is destructed when + * the refcount drops to zero. + * + * TODO: move most members from CNodeState to this structure. + * TODO: move remaining application-layer data members from CNode to this structure. + */ +struct Peer { + /** Same id as the CNode object for this peer */ + const NodeId m_id{0}; + + /** Protects misbehavior data members */ + Mutex m_misbehavior_mutex; + /** Accumulated misbehavior score for this peer */ + int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; + /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */ + bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; + + Peer(NodeId id) : m_id(id) {} +}; + +using PeerRef = std::shared_ptr<Peer>; + +/** + * Map of all Peer objects, keyed by peer id. This map is protected + * by the global g_peer_mutex. Once a shared pointer reference is + * taken, the lock may be released. Individual fields are protected by + * their own locks. + */ +Mutex g_peer_mutex; +static std::map<NodeId, PeerRef> g_peer_map GUARDED_BY(g_peer_mutex); + +/** Get a shared pointer to the Peer object. + * May return nullptr if the Peer object can't be found. */ +static PeerRef GetPeerRef(NodeId id) +{ + LOCK(g_peer_mutex); + auto it = g_peer_map.find(id); + return it != g_peer_map.end() ? it->second : nullptr; +} + static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; @@ -628,13 +663,12 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman& connma } } connman.ForNode(nodeid, [&connman](CNode* pfrom){ - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1; if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [&connman, nCMPCTBLOCKVersion](CNode* pnodeStop){ - AssertLockHeld(cs_main); connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); return true; }); @@ -841,10 +875,16 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { NodeId nodeid = pnode->GetId(); { LOCK(cs_main); - mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, std::move(addrName), pnode->IsInboundConn(), pnode->IsManualConn())); + mapNodeState.emplace_hint(mapNodeState.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(addr, pnode->IsInboundConn(), pnode->IsManualConn())); + } + { + PeerRef peer = std::make_shared<Peer>(nodeid); + LOCK(g_peer_mutex); + g_peer_map.emplace_hint(g_peer_map.end(), nodeid, std::move(peer)); } - if(!pnode->IsInboundConn()) + if (!pnode->IsInboundConn()) { PushNodeVersion(*pnode, m_connman, GetTime()); + } } void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const @@ -870,13 +910,21 @@ void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime) { fUpdateConnectionTime = false; LOCK(cs_main); + int misbehavior{0}; + { + PeerRef peer = GetPeerRef(nodeid); + assert(peer != nullptr); + misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); + LOCK(g_peer_mutex); + g_peer_map.erase(nodeid); + } CNodeState *state = State(nodeid); assert(state != nullptr); if (state->fSyncStarted) nSyncStarted--; - if (state->nMisbehavior == 0 && state->fCurrentlyConnected) { + if (misbehavior == 0 && state->fCurrentlyConnected) { fUpdateConnectionTime = true; } @@ -906,17 +954,23 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim } bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) { - LOCK(cs_main); - CNodeState *state = State(nodeid); - if (state == nullptr) - return false; - stats.nMisbehavior = state->nMisbehavior; - stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1; - stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1; - for (const QueuedBlock& queue : state->vBlocksInFlight) { - if (queue.pindex) - stats.vHeightInFlight.push_back(queue.pindex->nHeight); + { + LOCK(cs_main); + CNodeState* state = State(nodeid); + if (state == nullptr) + return false; + stats.nSyncHeight = state->pindexBestKnownBlock ? state->pindexBestKnownBlock->nHeight : -1; + stats.nCommonHeight = state->pindexLastCommonBlock ? state->pindexLastCommonBlock->nHeight : -1; + for (const QueuedBlock& queue : state->vBlocksInFlight) { + if (queue.pindex) + stats.vHeightInFlight.push_back(queue.pindex->nHeight); + } } + + PeerRef peer = GetPeerRef(nodeid); + if (peer == nullptr) return false; + stats.m_misbehavior_score = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); + return true; } @@ -1060,21 +1114,21 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. */ -void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) { assert(howmuch > 0); - CNodeState* const state = State(pnode); - if (state == nullptr) return; + PeerRef peer = GetPeerRef(pnode); + if (peer == nullptr) return; - state->nMisbehavior += howmuch; + LOCK(peer->m_misbehavior_mutex); + peer->m_misbehavior_score += howmuch; const std::string message_prefixed = message.empty() ? "" : (": " + message); - if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) - { - LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); - state->m_should_discourage = true; + if (peer->m_misbehavior_score >= DISCOURAGEMENT_THRESHOLD && peer->m_misbehavior_score - howmuch < DISCOURAGEMENT_THRESHOLD) { + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed); + peer->m_should_discourage = true; } else { - LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed); } } @@ -1096,7 +1150,6 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s case BlockValidationResult::BLOCK_CONSENSUS: case BlockValidationResult::BLOCK_MUTATED: if (!via_compact_block) { - LOCK(cs_main); Misbehaving(nodeid, 100, message); return true; } @@ -1120,18 +1173,12 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s case BlockValidationResult::BLOCK_INVALID_HEADER: case BlockValidationResult::BLOCK_CHECKPOINT: case BlockValidationResult::BLOCK_INVALID_PREV: - { - LOCK(cs_main); - Misbehaving(nodeid, 100, message); - } + Misbehaving(nodeid, 100, message); return true; // Conflicting (but not necessarily invalid) data or different policy: case BlockValidationResult::BLOCK_MISSING_PREV: - { - // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) - LOCK(cs_main); - Misbehaving(nodeid, 10, message); - } + // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) + Misbehaving(nodeid, 10, message); return true; case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE: case BlockValidationResult::BLOCK_TIME_FUTURE: @@ -1155,11 +1202,8 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, break; // The node is providing invalid data: case TxValidationResult::TX_CONSENSUS: - { - LOCK(cs_main); - Misbehaving(nodeid, 100, message); - return true; - } + Misbehaving(nodeid, 100, message); + return true; // Conflicting (but not necessarily invalid) data or different policy: case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: case TxValidationResult::TX_INPUTS_NOT_STANDARD: @@ -1327,7 +1371,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: } m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) @@ -1422,54 +1466,48 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidatio // -bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool static AlreadyHaveTx(const GenTxid& gtxid, const CTxMemPool& mempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - switch (inv.type) - { - case MSG_TX: - case MSG_WITNESS_TX: - case MSG_WTX: - { - assert(recentRejects); - if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) - { - // If the chain tip has changed previously rejected transactions - // might be now valid, e.g. due to a nLockTime'd tx becoming valid, - // or a double-spend. Reset the rejects filter and give those - // txs a second chance. - hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); - recentRejects->reset(); - } - - { - LOCK(g_cs_orphans); - if (!inv.IsMsgWtx() && mapOrphanTransactions.count(inv.hash)) { - return true; - } else if (inv.IsMsgWtx() && g_orphans_by_wtxid.count(inv.hash)) { - return true; - } - } + assert(recentRejects); + if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) { + // If the chain tip has changed previously rejected transactions + // might be now valid, e.g. due to a nLockTime'd tx becoming valid, + // or a double-spend. Reset the rejects filter and give those + // txs a second chance. + hashRecentRejectsChainTip = ::ChainActive().Tip()->GetBlockHash(); + recentRejects->reset(); + } - { - LOCK(g_cs_recent_confirmed_transactions); - if (g_recent_confirmed_transactions->contains(inv.hash)) return true; - } + const uint256& hash = gtxid.GetHash(); - return recentRejects->contains(inv.hash) || mempool.exists(ToGenTxid(inv)); + { + LOCK(g_cs_orphans); + if (!gtxid.IsWtxid() && mapOrphanTransactions.count(hash)) { + return true; + } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(hash)) { + return true; } - case MSG_BLOCK: - case MSG_WITNESS_BLOCK: - return LookupBlockIndex(inv.hash) != nullptr; } - // Don't know what it is, just say we already got one - return true; + + { + LOCK(g_cs_recent_confirmed_transactions); + if (g_recent_confirmed_transactions->contains(hash)) return true; + } + + return recentRejects->contains(hash) || mempool.exists(gtxid); +} + +bool static AlreadyHaveBlock(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + return LookupBlockIndex(block_hash) != nullptr; } void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) { connman.ForEachNode([&txid, &wtxid](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); + CNodeState &state = *State(pnode->GetId()); if (state.m_wtxid_relay) { pnode->PushTxInventory(wtxid); @@ -1494,7 +1532,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& assert(nRelayNodes <= best.size()); auto sortfunc = [&best, &hasher, nRelayNodes](CNode* pnode) { - if (pnode->IsAddrRelayPeer()) { + if (pnode->RelayAddrsWithConn()) { uint64_t hashKey = CSipHasher(hasher).Write(pnode->GetId()).Finalize(); for (unsigned int i = 0; i < nRelayNodes; i++) { if (hashKey > best[i].first) { @@ -1564,7 +1602,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c // disconnect node in case we have reached the outbound limit for serving historical blocks if (send && connman.OutboundTargetReached(true) && - (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.type == MSG_FILTERED_BLOCK) && + (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); @@ -1590,7 +1628,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c std::shared_ptr<const CBlock> pblock; if (a_recent_block && a_recent_block->GetHash() == pindex->GetBlockHash()) { pblock = a_recent_block; - } else if (inv.type == MSG_WITNESS_BLOCK) { + } else if (inv.IsMsgWitnessBlk()) { // Fast-path: in this case it is possible to serve the block directly from disk, // as the network format matches the format on disk std::vector<uint8_t> block_data; @@ -1607,12 +1645,11 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c pblock = pblockRead; } if (pblock) { - if (inv.type == MSG_BLOCK) + if (inv.IsMsgBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_WITNESS_BLOCK) + } else if (inv.IsMsgWitnessBlk()) { connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, *pblock)); - else if (inv.type == MSG_FILTERED_BLOCK) - { + } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; if (pfrom.m_tx_relay != nullptr) { @@ -1636,9 +1673,7 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } // else // no response - } - else if (inv.type == MSG_CMPCT_BLOCK) - { + } else if (inv.IsMsgCmpctBlk()) { // If a peer is asking for old blocks, we're almost guaranteed // they won't have a useful mempool to match against a compact block, // and we don't feel like constructing the object for them, so @@ -1766,7 +1801,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // expensive to process. if (it != pfrom.vRecvGetData.end() && !pfrom.fPauseSend) { const CInv &inv = *it++; - if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) { + if (inv.IsGenBlkMsg()) { ProcessGetBlockData(pfrom, chainparams, inv, connman); } // else: If the first item on the queue is an unknown type, we erase it @@ -1806,7 +1841,6 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac BlockTransactions resp(req); for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices"); return; } @@ -1984,7 +2018,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan } } - if (!pfrom.fDisconnect && pfrom.IsOutboundOrBlockRelayConn() && nodestate->pindexBestKnownBlock != nullptr && pfrom.m_tx_relay != nullptr) { + if (!pfrom.fDisconnect && pfrom.IsFullOutboundConn() && nodestate->pindexBestKnownBlock != nullptr) { // If this is an outbound full-relay peer, check to see if we should protect // it from the bad/lagging chain logic. // Note that block-relay-only peers are already implicitly protected, so we @@ -2318,7 +2352,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Each connection can only send one version message if (pfrom.nVersion != 0) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "redundant version message"); return; } @@ -2426,9 +2459,23 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty UpdatePreferredDownload(pfrom, State(pfrom.GetId())); } - if (!pfrom.IsInboundConn() && pfrom.IsAddrRelayPeer()) - { - // Advertise our address + if (!pfrom.IsInboundConn() && !pfrom.IsBlockOnlyConn()) { + // For outbound peers, we try to relay our address (so that other + // nodes can try to find us more quickly, as we have no guarantee + // that an outbound peer is even aware of how to reach us) and do a + // one-time address fetch (to help populate/update our addrman). If + // we're starting up for the first time, our addrman may be pretty + // empty and no one will know who we are, so these mechanisms are + // important to help us connect to the network. + // + // We also update the addrman to record connection success for + // these peers (which include OUTBOUND_FULL_RELAY and FEELER + // connections) so that addrman will have an up-to-date notion of + // which peers are online and available. + // + // We skip these operations for BLOCK_RELAY peers to avoid + // potentially leaking information about our BLOCK_RELAY + // connections via the addrman or address relay. if (fListen && !::ChainstateActive().IsInitialBlockDownload()) { CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); @@ -2447,6 +2494,9 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Get recent addresses m_connman.PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; + + // Moves address from New to Tried table in Addrman, resolves + // tried-table collisions, etc. m_connman.MarkAddressGood(pfrom.addr); } @@ -2478,7 +2528,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (pfrom.nVersion == 0) { // Must have a version message before anything else - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake"); return; } @@ -2545,7 +2594,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else - LOCK(cs_main); Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake"); return; } @@ -2554,12 +2602,11 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty std::vector<CAddress> vAddr; vRecv >> vAddr; - if (!pfrom.IsAddrRelayPeer()) { + if (!pfrom.RelayAddrsWithConn()) { return; } if (vAddr.size() > MAX_ADDR_TO_SEND) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("addr message size = %u", vAddr.size())); return; } @@ -2638,7 +2685,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("inv message size = %u", vInv.size())); return; } @@ -2654,14 +2700,11 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty LOCK(cs_main); - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); uint256* best_block{nullptr}; - for (CInv &inv : vInv) - { - if (interruptMsgProc) - return; + for (CInv& inv : vInv) { + if (interruptMsgProc) return; // Ignore INVs that don't match wtxidrelay setting. // Note that orphan parent fetching always uses MSG_TX GETDATAs regardless of the wtxidrelay setting. @@ -2672,14 +2715,10 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (inv.IsMsgWtx()) continue; } - bool fAlreadyHave = AlreadyHave(inv, m_mempool); - LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - - if (inv.IsMsgTx()) { - inv.type |= nFetchFlags; - } + if (inv.IsMsgBlk()) { + const bool fAlreadyHave = AlreadyHaveBlock(inv.hash); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - if (inv.type == MSG_BLOCK) { UpdateBlockAvailability(pfrom.GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { // Headers-first is the primary method of announcement on @@ -2689,15 +2728,21 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // then fetch the blocks we need to catch up. best_block = &inv.hash; } - } else { + } else if (inv.IsGenTxMsg()) { + const GenTxid gtxid = ToGenTxid(inv); + const bool fAlreadyHave = AlreadyHaveTx(gtxid, mempool); + LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); + pfrom.AddKnownTx(inv.hash); if (fBlocksOnly) { LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom.GetId()); pfrom.fDisconnect = true; return; } else if (!fAlreadyHave && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - RequestTx(State(pfrom.GetId()), ToGenTxid(inv), current_time); + RequestTx(State(pfrom.GetId()), gtxid, current_time); } + } else { + LogPrint(BCLog::NET, "Unknown inv type \"%s\" received from peer=%d\n", inv.ToString(), pfrom.GetId()); } } @@ -2714,7 +2759,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("getdata message size = %u", vInv.size())); return; } @@ -2969,7 +3013,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (!AlreadyHave(CInv(MSG_WTX, wtxid), m_mempool) && + if (!AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid), m_mempool) && AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { m_mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman); @@ -3013,7 +3057,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } } if (!fRejectedParents) { - uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); for (const uint256& parent_txid : unique_parents) { @@ -3022,9 +3065,9 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // wtxidrelay peers. // Eventually we should replace this with an improved // protocol for getting all unconfirmed parents. - CInv _inv(MSG_TX | nFetchFlags, parent_txid); + const GenTxid gtxid{/* is_wtxid=*/false, parent_txid}; pfrom.AddKnownTx(parent_txid); - if (!AlreadyHave(_inv, m_mempool)) RequestTx(State(pfrom.GetId()), ToGenTxid(_inv), current_time); + if (!AlreadyHaveTx(gtxid, m_mempool)) RequestTx(State(pfrom.GetId()), gtxid, current_time); } AddOrphanTx(ptx, pfrom.GetId()); @@ -3439,7 +3482,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks. unsigned int nCount = ReadCompactSize(vRecv); if (nCount > MAX_HEADERS_RESULTS) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount)); return; } @@ -3498,7 +3540,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom.GetId()); return; } - if (!pfrom.IsAddrRelayPeer()) { + if (!pfrom.RelayAddrsWithConn()) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from block-relay-only connection. peer=%d\n", pfrom.GetId()); return; } @@ -3641,7 +3683,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty if (!filter.IsWithinSizeConstraints()) { // There is no excuse for sending a too-large filter - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); } else if (pfrom.m_tx_relay != nullptr) @@ -3675,7 +3716,6 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } } if (bad) { - LOCK(cs_main); Misbehaving(pfrom.GetId(), 100, "bad filteradd message"); } return; @@ -3761,15 +3801,17 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) { const NodeId peer_id{pnode.GetId()}; + PeerRef peer = GetPeerRef(peer_id); + if (peer == nullptr) return false; + { - LOCK(cs_main); - CNodeState& state = *State(peer_id); + LOCK(peer->m_misbehavior_mutex); // There's nothing to do if the m_should_discourage flag isn't set - if (!state.m_should_discourage) return false; + if (!peer->m_should_discourage) return false; - state.m_should_discourage = false; - } // cs_main + peer->m_should_discourage = false; + } // peer.m_misbehavior_mutex if (pnode.HasPermission(PF_NOBAN)) { // We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag @@ -3957,7 +3999,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) int64_t oldest_block_announcement = std::numeric_limits<int64_t>::max(); m_connman.ForEachNode([&](CNode* pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // Ignore non-outbound peers, or nodes marked for disconnect already if (!pnode->IsOutboundOrBlockRelayConn() || pnode->fDisconnect) return; @@ -3974,7 +4016,7 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) }); if (worst_peer != -1) { bool disconnected = m_connman.ForNode(worst_peer, [&](CNode *pnode) { - AssertLockHeld(cs_main); + LockAssertion lock(::cs_main); // Only disconnect a peer that has been connected to us for // some reasonable fraction of our check-frequency, to give @@ -4098,7 +4140,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) int64_t nNow = GetTimeMicros(); auto current_time = GetTime<std::chrono::microseconds>(); - if (pto->IsAddrRelayPeer() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { + if (pto->RelayAddrsWithConn() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { AdvertiseLocal(pto); pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } @@ -4106,7 +4148,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // // Message: addr // - if (pto->IsAddrRelayPeer() && pto->m_next_addr_send < current_time) { + if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) { pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector<CAddress> vAddr; vAddr.reserve(pto->vAddrToSend.size()); @@ -4575,7 +4617,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); CInv inv(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); - if (!AlreadyHave(inv, m_mempool)) { + if (!AlreadyHaveTx(ToGenTxid(inv), m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. const auto last_request_time = GetTxRequestTime(gtxid); diff --git a/src/net_processing.h b/src/net_processing.h index 74d6603747..4f1c52fc17 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -97,7 +97,7 @@ private: }; struct CNodeStateStats { - int nMisbehavior = 0; + int m_misbehavior_score = 0; int nSyncHeight = -1; int nCommonHeight = -1; std::vector<int> vHeightInFlight; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 8adfe38dc9..b50cf74069 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -270,17 +270,6 @@ bool CNetAddr::IsLocal() const */ bool CNetAddr::IsValid() const { - // Cleanup 3-byte shifted addresses caused by garbage in size field - // of addr messages from versions before 0.2.9 checksum. - // Two consecutive addr messages look like this: - // header20 vectorlen3 addr26 addr26 addr26 header20 vectorlen3 addr26 addr26 addr26... - // so if the first length field is garbled, it reads the second batch - // of addr misaligned by 3 bytes. - if (IsIPv6() && memcmp(m_addr.data(), IPV4_IN_IPV6_PREFIX.data() + 3, - sizeof(IPV4_IN_IPV6_PREFIX) - 3) == 0) { - return false; - } - // unspecified IPv6 address (::/128) unsigned char ipNone6[16] = {}; if (IsIPv6() && memcmp(m_addr.data(), ipNone6, sizeof(ipNone6)) == 0) { diff --git a/src/node/context.h b/src/node/context.h index be568cba36..793c9dfc34 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -20,6 +20,7 @@ class PeerLogicValidation; namespace interfaces { class Chain; class ChainClient; +class WalletClient; } // namespace interfaces //! NodeContext struct containing references to chain state and connection @@ -40,7 +41,11 @@ struct NodeContext { std::unique_ptr<BanMan> banman; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr<interfaces::Chain> chain; + //! List of all chain clients (wallet processes or other client) connected to node. std::vector<std::unique_ptr<interfaces::ChainClient>> chain_clients; + //! Reference to chain client that should used to load or create wallets + //! opened by the gui. + interfaces::WalletClient* wallet_client{nullptr}; std::unique_ptr<CScheduler> scheduler; std::function<void()> rpc_interruption_point = [] {}; diff --git a/src/protocol.h b/src/protocol.h index 415922dfb6..7fb84cddf1 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -247,7 +247,7 @@ extern const char* CFCHECKPT; * txid. * @since protocol version 70016 as described by BIP 339. */ -extern const char *WTXIDRELAY; +extern const char* WTXIDRELAY; }; // namespace NetMsgType /* Get a vector of all valid message types (see above) */ @@ -418,12 +418,22 @@ public: std::string ToString() const; // Single-message helper methods - bool IsMsgTx() const { return type == MSG_TX; } - bool IsMsgWtx() const { return type == MSG_WTX; } - bool IsMsgWitnessTx() const { return type == MSG_WITNESS_TX; } + bool IsMsgTx() const { return type == MSG_TX; } + bool IsMsgBlk() const { return type == MSG_BLOCK; } + bool IsMsgWtx() const { return type == MSG_WTX; } + bool IsMsgFilteredBlk() const { return type == MSG_FILTERED_BLOCK; } + bool IsMsgCmpctBlk() const { return type == MSG_CMPCT_BLOCK; } + bool IsMsgWitnessBlk() const { return type == MSG_WITNESS_BLOCK; } // Combined-message helper methods - bool IsGenTxMsg() const { return type == MSG_TX || type == MSG_WTX || type == MSG_WITNESS_TX; } + bool IsGenTxMsg() const + { + return type == MSG_TX || type == MSG_WTX || type == MSG_WITNESS_TX; + } + bool IsGenBlkMsg() const + { + return type == MSG_BLOCK || type == MSG_FILTERED_BLOCK || type == MSG_CMPCT_BLOCK || type == MSG_WITNESS_BLOCK; + } uint32_t type; uint256 hash; diff --git a/src/pubkey.cpp b/src/pubkey.cpp index ef42aa5bc7..fc14f41a0c 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -24,7 +24,7 @@ secp256k1_context* secp256k1_context_verify = nullptr; * strict DER before being passed to this module, and we know it supports all * violations present in the blockchain before that point. */ -static int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) { +int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char *input, size_t inputlen) { size_t rpos, rlen, spos, slen; size_t pos = 0; size_t lenbyte; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index e63ffdfb36..eaaaaeb904 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -206,6 +206,7 @@ BitcoinApplication::BitcoinApplication(): returnValue(0), platformStyle(nullptr) { + // Qt runs setlocale(LC_ALL, "") on initialization. RegisterMetaTypes(); setQuitOnLastWindowClosed(false); } @@ -266,6 +267,7 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) // We don't hold a direct pointer to the splash screen after creation, but the splash // screen will take care of deleting itself when finish() happens. m_splash->show(); + connect(this, &BitcoinApplication::requestedInitialize, m_splash, &SplashScreen::handleLoadWallet); connect(this, &BitcoinApplication::splashFinished, m_splash, &SplashScreen::finish); connect(this, &BitcoinApplication::requestedShutdown, m_splash, &QWidget::close); } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 8935ff19bf..aa58c0b10e 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1189,7 +1189,7 @@ void BitcoinGUI::incomingTransaction(const QString& date, int unit, const CAmoun // On new transaction, make an info balloon QString msg = tr("Date: %1\n").arg(date) + tr("Amount: %1\n").arg(BitcoinUnits::formatWithUnit(unit, amount, true)); - if (m_node.getWallets().size() > 1 && !walletName.isEmpty()) { + if (m_node.walletClient().getWallets().size() > 1 && !walletName.isEmpty()) { msg += tr("Wallet: %1\n").arg(walletName); } msg += tr("Type: %1\n").arg(type); diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 93840b4169..d210faec03 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -290,7 +290,7 @@ <item row="11" column="0"> <widget class="QLabel" name="label_3"> <property name="text"> - <string>Current number of blocks</string> + <string>Current block height</string> </property> </widget> </item> diff --git a/src/qt/forms/receivecoinsdialog.ui b/src/qt/forms/receivecoinsdialog.ui index 7dbee6d689..06d39426c9 100644 --- a/src/qt/forms/receivecoinsdialog.ui +++ b/src/qt/forms/receivecoinsdialog.ui @@ -114,6 +114,12 @@ <iconset resource="../bitcoin.qrc"> <normaloff>:/icons/receiving_addresses</normaloff>:/icons/receiving_addresses</iconset> </property> + <property name="autoDefault"> + <bool>false</bool> + </property> + <property name="default"> + <bool>true</bool> + </property> </widget> </item> <item> diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 5debded4ea..d374d610ee 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -242,22 +242,6 @@ void ReceiveCoinsDialog::resizeEvent(QResizeEvent *event) columnResizingFixer->stretchColumnWidth(RecentRequestsTableModel::Message); } -void ReceiveCoinsDialog::keyPressEvent(QKeyEvent *event) -{ - if (event->key() == Qt::Key_Return) - { - // press return -> submit form - if (ui->reqLabel->hasFocus() || ui->reqAmount->hasFocus() || ui->reqMessage->hasFocus()) - { - event->ignore(); - on_receiveButton_clicked(); - return; - } - } - - this->QDialog::keyPressEvent(event); -} - QModelIndex ReceiveCoinsDialog::selectedRow() { if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel()) diff --git a/src/qt/receivecoinsdialog.h b/src/qt/receivecoinsdialog.h index 2f48cd58f0..27455e2906 100644 --- a/src/qt/receivecoinsdialog.h +++ b/src/qt/receivecoinsdialog.h @@ -49,9 +49,6 @@ public Q_SLOTS: void reject() override; void accept() override; -protected: - virtual void keyPressEvent(QKeyEvent *event) override; - private: Ui::ReceiveCoinsDialog *ui; GUIUtil::TableViewLastColumnResizingFixer *columnResizingFixer; diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index bd63d6e7fb..8e381861a0 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -185,21 +185,21 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr : _("press q to shutdown").translated) + strprintf("\n%d", nProgress) + "%"); } -#ifdef ENABLE_WALLET -void SplashScreen::ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet) -{ - m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false))); - m_connected_wallets.emplace_back(std::move(wallet)); -} -#endif void SplashScreen::subscribeToCoreSignals() { // Connect signals to client m_handler_init_message = m_node->handleInitMessage(std::bind(InitMessage, this, std::placeholders::_1)); m_handler_show_progress = m_node->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); +} + +void SplashScreen::handleLoadWallet() +{ #ifdef ENABLE_WALLET - m_handler_load_wallet = m_node->handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { ConnectWallet(std::move(wallet)); }); + m_handler_load_wallet = m_node->walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { + m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false))); + m_connected_wallets.emplace_back(std::move(wallet)); + }); #endif } diff --git a/src/qt/splashscreen.h b/src/qt/splashscreen.h index 2213b02c55..a0cd677d3d 100644 --- a/src/qt/splashscreen.h +++ b/src/qt/splashscreen.h @@ -43,6 +43,9 @@ public Q_SLOTS: /** Show message and progress */ void showMessage(const QString &message, int alignment, const QColor &color); + /** Handle wallet load notifications. */ + void handleLoadWallet(); + protected: bool eventFilter(QObject * obj, QEvent * ev) override; @@ -53,8 +56,6 @@ private: void unsubscribeFromCoreSignals(); /** Initiate shutdown */ void shutdown(); - /** Connect wallet signals to splash screen */ - void ConnectWallet(std::unique_ptr<interfaces::Wallet> wallet); QPixmap pixmap; QString curMessage; diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 3aed98e0e8..828f84ffcc 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -35,11 +35,11 @@ WalletController::WalletController(ClientModel& client_model, const PlatformStyl , m_platform_style(platform_style) , m_options_model(client_model.getOptionsModel()) { - m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { + m_handler_load_wallet = m_node.walletClient().handleLoadWallet([this](std::unique_ptr<interfaces::Wallet> wallet) { getOrCreateWallet(std::move(wallet)); }); - for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.getWallets()) { + for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.walletClient().getWallets()) { getOrCreateWallet(std::move(wallet)); } @@ -66,7 +66,7 @@ std::map<std::string, bool> WalletController::listWalletDir() const { QMutexLocker locker(&m_mutex); std::map<std::string, bool> wallets; - for (const std::string& name : m_node.listWalletDir()) { + for (const std::string& name : m_node.walletClient().listWalletDir()) { wallets[name] = false; } for (WalletModel* wallet_model : m_wallets) { @@ -250,7 +250,7 @@ void CreateWalletActivity::createWallet() QTimer::singleShot(500, worker(), [this, name, flags] { WalletCreationStatus status; - std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, status); + std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().createWallet(name, m_passphrase, flags, status, m_error_message, m_warning_message); if (status == WalletCreationStatus::SUCCESS) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); @@ -321,7 +321,7 @@ void OpenWalletActivity::open(const std::string& path) showProgressDialog(tr("Opening Wallet <b>%1</b>...").arg(name.toHtmlEscaped())); QTimer::singleShot(0, worker(), [this, path] { - std::unique_ptr<interfaces::Wallet> wallet = node().loadWallet(path, m_error_message, m_warning_message); + std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().loadWallet(path, m_error_message, m_warning_message); if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e374dd191c..6a3f903206 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -413,7 +413,7 @@ void WalletModel::subscribeToCoreSignals() m_handler_transaction_changed = m_wallet->handleTransactionChanged(std::bind(NotifyTransactionChanged, this, std::placeholders::_1, std::placeholders::_2)); m_handler_show_progress = m_wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2)); m_handler_watch_only_changed = m_wallet->handleWatchOnlyChanged(std::bind(NotifyWatchonlyChanged, this, std::placeholders::_1)); - m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(boost::bind(NotifyCanGetAddressesChanged, this)); + m_handler_can_get_addrs_changed = m_wallet->handleCanGetAddressesChanged(std::bind(NotifyCanGetAddressesChanged, this)); } void WalletModel::unsubscribeFromCoreSignals() @@ -581,7 +581,7 @@ QString WalletModel::getDisplayName() const bool WalletModel::isMultiwallet() { - return m_node.getWallets().size() > 1; + return m_node.walletClient().getWallets().size() > 1; } void WalletModel::refresh(bool pk_hash_only) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 76aa9dbfc1..b04e106b2d 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -81,9 +81,9 @@ static UniValue GetNetworkHashPS(int lookup, int height) { return workDiff.getdouble() / timeDiff; } -static UniValue getnetworkhashps(const JSONRPCRequest& request) +static RPCHelpMan getnetworkhashps() { - RPCHelpMan{"getnetworkhashps", + return RPCHelpMan{"getnetworkhashps", "\nReturns the estimated network hashes per second based on the last n blocks.\n" "Pass in [blocks] to override # of blocks, -1 specifies since last difficulty change.\n" "Pass in [height] to estimate the network speed at the time when a certain block was found.\n", @@ -97,10 +97,12 @@ static UniValue getnetworkhashps(const JSONRPCRequest& request) HelpExampleCli("getnetworkhashps", "") + HelpExampleRpc("getnetworkhashps", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); return GetNetworkHashPS(!request.params[0].isNull() ? request.params[0].get_int() : 120, !request.params[1].isNull() ? request.params[1].get_int() : -1); +}, + }; } static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& max_tries, unsigned int& extra_nonce, uint256& block_hash) @@ -200,9 +202,9 @@ static bool getScriptFromDescriptor(const std::string& descriptor, CScript& scri } } -static UniValue generatetodescriptor(const JSONRPCRequest& request) +static RPCHelpMan generatetodescriptor() { - RPCHelpMan{ + return RPCHelpMan{ "generatetodescriptor", "\nMine blocks immediately to a specified descriptor (before the RPC call returns)\n", { @@ -218,9 +220,8 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) }, RPCExamples{ "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")}, - } - .Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const int num_blocks{request.params[0].get_int()}; const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; @@ -234,22 +235,25 @@ static UniValue generatetodescriptor(const JSONRPCRequest& request) ChainstateManager& chainman = EnsureChainman(request.context); return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); +}, + }; } -static UniValue generate(const JSONRPCRequest& request) +static RPCHelpMan generate() { - const std::string help_str{"generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information."}; + return RPCHelpMan{"generate", "has been replaced by the -generate cli option. Refer to -help for more information.", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { if (request.fHelp) { - throw std::runtime_error(help_str); + throw std::runtime_error(self.ToString()); } else { - throw JSONRPCError(RPC_METHOD_NOT_FOUND, help_str); + throw JSONRPCError(RPC_METHOD_NOT_FOUND, self.ToString()); } + }}; } -static UniValue generatetoaddress(const JSONRPCRequest& request) +static RPCHelpMan generatetoaddress() { - RPCHelpMan{"generatetoaddress", + return RPCHelpMan{"generatetoaddress", "\nMine blocks immediately to a specified address (before the RPC call returns)\n", { {"nblocks", RPCArg::Type::NUM, RPCArg::Optional::NO, "How many blocks are generated immediately."}, @@ -267,8 +271,8 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) + "If you are using the " PACKAGE_NAME " wallet, you can get a new address to send the newly generated bitcoin to with:\n" + HelpExampleCli("getnewaddress", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const int num_blocks{request.params[0].get_int()}; const uint64_t max_tries{request.params[2].isNull() ? DEFAULT_MAX_TRIES : request.params[2].get_int()}; @@ -283,11 +287,13 @@ static UniValue generatetoaddress(const JSONRPCRequest& request) CScript coinbase_script = GetScriptForDestination(destination); return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); +}, + }; } -static UniValue generateblock(const JSONRPCRequest& request) +static RPCHelpMan generateblock() { - RPCHelpMan{"generateblock", + return RPCHelpMan{"generateblock", "\nMine a block with a set of ordered transactions immediately to a specified address or descriptor (before the RPC call returns)\n", { {"output", RPCArg::Type::STR, RPCArg::Optional::NO, "The address or descriptor to send the newly generated bitcoin to."}, @@ -309,8 +315,8 @@ static UniValue generateblock(const JSONRPCRequest& request) "\nGenerate a block to myaddress, with txs rawtx and mempool_txid\n" + HelpExampleCli("generateblock", R"("myaddress" '["rawtx", "mempool_txid"]')") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ const auto address_or_descriptor = request.params[0].get_str(); CScript coinbase_script; std::string error; @@ -390,11 +396,13 @@ static UniValue generateblock(const JSONRPCRequest& request) UniValue obj(UniValue::VOBJ); obj.pushKV("hash", block_hash.GetHex()); return obj; +}, + }; } -static UniValue getmininginfo(const JSONRPCRequest& request) +static RPCHelpMan getmininginfo() { - RPCHelpMan{"getmininginfo", + return RPCHelpMan{"getmininginfo", "\nReturns a json object containing mining-related information.", {}, RPCResult{ @@ -413,8 +421,8 @@ static UniValue getmininginfo(const JSONRPCRequest& request) HelpExampleCli("getmininginfo", "") + HelpExampleRpc("getmininginfo", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); const CTxMemPool& mempool = EnsureMemPool(request.context); @@ -423,18 +431,20 @@ static UniValue getmininginfo(const JSONRPCRequest& request) if (BlockAssembler::m_last_block_weight) obj.pushKV("currentblockweight", *BlockAssembler::m_last_block_weight); if (BlockAssembler::m_last_block_num_txs) obj.pushKV("currentblocktx", *BlockAssembler::m_last_block_num_txs); obj.pushKV("difficulty", (double)GetDifficulty(::ChainActive().Tip())); - obj.pushKV("networkhashps", getnetworkhashps(request)); + obj.pushKV("networkhashps", getnetworkhashps().HandleRequest(request)); obj.pushKV("pooledtx", (uint64_t)mempool.size()); obj.pushKV("chain", Params().NetworkIDString()); obj.pushKV("warnings", GetWarnings(false).original); return obj; +}, + }; } // NOTE: Unlike wallet RPC (which use BTC values), mining RPCs follow GBT (BIP 22) in using satoshi amounts -static UniValue prioritisetransaction(const JSONRPCRequest& request) +static RPCHelpMan prioritisetransaction() { - RPCHelpMan{"prioritisetransaction", + return RPCHelpMan{"prioritisetransaction", "Accepts the transaction into mined blocks at a higher (or lower) priority\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id."}, @@ -451,8 +461,8 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) HelpExampleCli("prioritisetransaction", "\"txid\" 0.0 10000") + HelpExampleRpc("prioritisetransaction", "\"txid\", 0.0, 10000") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -464,6 +474,8 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) EnsureMemPool(request.context).PrioritiseTransaction(hash, nAmount); return true; +}, + }; } @@ -495,9 +507,9 @@ static std::string gbt_vb_name(const Consensus::DeploymentPos pos) { return s; } -static UniValue getblocktemplate(const JSONRPCRequest& request) +static RPCHelpMan getblocktemplate() { - RPCHelpMan{"getblocktemplate", + return RPCHelpMan{"getblocktemplate", "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n" "It returns data needed to construct a block to work on.\n" "For full specification, see BIPs 22, 23, 9, and 145:\n" @@ -511,12 +523,13 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings", { - {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasetxn', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, }, }, {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings", { - {"support", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported softfork deployment"}, + {"segwit", RPCArg::Type::STR, RPCArg::Optional::NO, "(literal) indicates client side segwit support"}, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"}, }, }, }, @@ -528,7 +541,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM, "version", "The preferred block version"}, {RPCResult::Type::ARR, "rules", "specific block rules that are to be enforced", { - {RPCResult::Type::STR, "", "rulename"}, + {RPCResult::Type::STR, "", "name of a rule the client must understand to some extent; see BIP 9 for format"}, }}, {RPCResult::Type::OBJ_DYN, "vbavailable", "set of pending, supported versionbit (BIP 9) softfork deployments", { @@ -536,7 +549,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) }}, {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, - {RPCResult::Type::ARR, "", "contents of non-coinbase transactions that should be included in the next block", + {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", { {RPCResult::Type::OBJ, "", "", { @@ -552,15 +565,12 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM, "weight", "total transaction weight, as counted for purposes of block limits"}, }}, }}, - {RPCResult::Type::OBJ, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", + {RPCResult::Type::OBJ_DYN, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", { - {RPCResult::Type::ELISION, "", ""}, + {RPCResult::Type::STR_HEX, "key", "values must be in the coinbase (keys may be ignored)"}, }}, {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"}, - {RPCResult::Type::OBJ, "coinbasetxn", "information for coinbase transaction", - { - {RPCResult::Type::ELISION, "", ""}, - }}, + {RPCResult::Type::STR, "longpollid", "an id to include with a request to longpoll on an update to this template"}, {RPCResult::Type::STR, "target", "The hash target"}, {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME}, {RPCResult::Type::ARR, "mutable", "list of ways the block template may be changed", @@ -574,13 +584,14 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, {RPCResult::Type::STR, "bits", "compressed target of next block"}, {RPCResult::Type::NUM, "height", "The height of the next block"}, + {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"} }}, RPCExamples{ HelpExampleCli("getblocktemplate", "'{\"rules\": [\"segwit\"]}'") + HelpExampleRpc("getblocktemplate", "{\"rules\": [\"segwit\"]}") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(cs_main); std::string strMode = "template"; @@ -888,6 +899,8 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) } return result; +}, + }; } class submitblock_StateCatcher final : public CValidationInterface @@ -908,10 +921,10 @@ protected: } }; -static UniValue submitblock(const JSONRPCRequest& request) +static RPCHelpMan submitblock() { // We allow 2 arguments for compliance with BIP22. Argument 2 is ignored. - RPCHelpMan{"submitblock", + return RPCHelpMan{"submitblock", "\nAttempts to submit new block to network.\n" "See https://en.bitcoin.it/wiki/BIP_0022 for full specification.\n", { @@ -923,8 +936,8 @@ static UniValue submitblock(const JSONRPCRequest& request) HelpExampleCli("submitblock", "\"mydata\"") + HelpExampleRpc("submitblock", "\"mydata\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CBlock> blockptr = std::make_shared<CBlock>(); CBlock& block = *blockptr; if (!DecodeHexBlk(block, request.params[0].get_str())) { @@ -969,11 +982,13 @@ static UniValue submitblock(const JSONRPCRequest& request) return "inconclusive"; } return BIP22ValidationResult(sc->state); +}, + }; } -static UniValue submitheader(const JSONRPCRequest& request) +static RPCHelpMan submitheader() { - RPCHelpMan{"submitheader", + return RPCHelpMan{"submitheader", "\nDecode the given hexdata as a header and submit it as a candidate chain tip if valid." "\nThrows when the header is invalid.\n", { @@ -985,8 +1000,8 @@ static UniValue submitheader(const JSONRPCRequest& request) HelpExampleCli("submitheader", "\"aabbcc\"") + HelpExampleRpc("submitheader", "\"aabbcc\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ CBlockHeader h; if (!DecodeHexBlockHeader(h, request.params[0].get_str())) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block header decode failed"); @@ -1005,11 +1020,13 @@ static UniValue submitheader(const JSONRPCRequest& request) throw JSONRPCError(RPC_VERIFY_ERROR, state.ToString()); } throw JSONRPCError(RPC_VERIFY_ERROR, state.GetRejectReason()); +}, + }; } -static UniValue estimatesmartfee(const JSONRPCRequest& request) +static RPCHelpMan estimatesmartfee() { - RPCHelpMan{"estimatesmartfee", + return RPCHelpMan{"estimatesmartfee", "\nEstimates the approximate fee per kilobyte needed for a transaction to begin\n" "confirmation within conf_target blocks if possible and return the number of blocks\n" "for which the estimate is valid. Uses virtual transaction size as defined\n" @@ -1043,8 +1060,8 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) RPCExamples{ HelpExampleCli("estimatesmartfee", "6") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR}); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); @@ -1070,11 +1087,13 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) } result.pushKV("blocks", feeCalc.returnedTarget); return result; +}, + }; } -static UniValue estimaterawfee(const JSONRPCRequest& request) +static RPCHelpMan estimaterawfee() { - RPCHelpMan{"estimaterawfee", + return RPCHelpMan{"estimaterawfee", "\nWARNING: This interface is unstable and may disappear or change!\n" "\nWARNING: This is an advanced API call that is tightly coupled to the specific\n" " implementation of fee estimation. The parameters it can be called with\n" @@ -1126,8 +1145,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) RPCExamples{ HelpExampleCli("estimaterawfee", "6 0.9") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); @@ -1186,6 +1205,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) result.pushKV(StringForFeeEstimateHorizon(horizon), horizon_result); } return result; +}, + }; } void RegisterMiningRPCCommands(CRPCTable &t) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e9343b3348..7e2052c7cb 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -197,7 +197,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) if (fStateStats) { if (IsDeprecatedRPCEnabled("banscore")) { // banscore is deprecated in v0.21 for removal in v0.22 - obj.pushKV("banscore", statestats.nMisbehavior); + obj.pushKV("banscore", statestats.m_misbehavior_score); } obj.pushKV("synced_headers", statestats.nSyncHeight); obj.pushKV("synced_blocks", statestats.nCommonHeight); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9c8e7fe04a..f32d9abac6 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -261,13 +261,11 @@ CRPCTable::CRPCTable() } } -bool CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) +void CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) { - if (IsRPCRunning()) - return false; + CHECK_NONFATAL(!IsRPCRunning()); // Only add commands before rpc is running mapCommands[name].push_back(pcmd); - return true; } bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd) diff --git a/src/rpc/server.h b/src/rpc/server.h index 6da3e94ea2..b2358ac5b2 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -160,7 +160,7 @@ public: /** * Appends a CRPCCommand to the dispatch table. * - * Returns false if RPC server is already running (dump concurrency protection). + * Precondition: RPC server is not running * * Commands with different method names but the same unique_id will * be considered aliases, and only the first registered method name will @@ -169,7 +169,7 @@ public: * between calls based on method name, and aliased commands can also * register different names, types, and numbers of parameters. */ - bool appendCommand(const std::string& name, const CRPCCommand* pcmd); + void appendCommand(const std::string& name, const CRPCCommand* pcmd); bool removeCommand(const std::string& name, const CRPCCommand* pcmd); }; diff --git a/src/sync.cpp b/src/sync.cpp index 4be13a3c48..322198a852 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -238,12 +238,15 @@ void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { if (!LockHeld(cs)) return; tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockNotHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockNotHeldInternal(const char*, const char*, int, RecursiveMutex*); void DeleteLock(void* cs) { diff --git a/src/sync.h b/src/sync.h index 05ff2ee8a9..7b397a8003 100644 --- a/src/sync.h +++ b/src/sync.h @@ -53,8 +53,9 @@ void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); template <typename MutexType> -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); -void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs); +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs); void DeleteLock(void* cs); bool LockStackEmpty(); @@ -69,8 +70,9 @@ inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, v inline void LeaveCritical() {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} template <typename MutexType> -inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} -inline void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} +inline void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs) {} +template <typename MutexType> +void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) {} inline void DeleteLock(void* cs) {} inline bool LockStackEmpty() { return true; } #endif diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index c0a2fca9ca..bf0659587c 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); - CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND); + CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY); dummyNode1.SetSendVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(&dummyNode1); @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic, CConnmanTest* connman) { CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); - vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND)); + vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY)); CNode &node = *vNodes.back(); node.SetSendVersion(PROTOCOL_VERSION); @@ -232,10 +232,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(&dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged - } + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged { LOCK(dummyNode1.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); @@ -249,20 +246,14 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(&dummyNode2); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); - } + Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); { LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet... BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be - { - LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold - } + Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold { LOCK(dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); @@ -292,10 +283,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); - } + Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); { LOCK(dummyNode.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp new file mode 100644 index 0000000000..cd0c93b8d0 --- /dev/null +++ b/src/test/fuzz/net.cpp @@ -0,0 +1,156 @@ +// Copyright (c) 2020 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 <chainparams.h> +#include <chainparamsbase.h> +#include <net.h> +#include <net_permissions.h> +#include <netaddress.h> +#include <optional.h> +#include <protocol.h> +#include <random.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> + +#include <cstdint> +#include <string> +#include <vector> + +void initialize() +{ + static const BasicTestingSetup basic_testing_setup; +} + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + + const std::optional<CAddress> address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!address) { + return; + } + const std::optional<CAddress> address_bind = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!address_bind) { + return; + } + + CNode node{fuzzed_data_provider.ConsumeIntegral<NodeId>(), + static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()), + fuzzed_data_provider.ConsumeIntegral<int>(), + INVALID_SOCKET, + *address, + fuzzed_data_provider.ConsumeIntegral<uint64_t>(), + fuzzed_data_provider.ConsumeIntegral<uint64_t>(), + *address_bind, + fuzzed_data_provider.ConsumeRandomLengthString(32), + fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH})}; + while (fuzzed_data_provider.ConsumeBool()) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 12)) { + case 0: { + node.CloseSocketDisconnect(); + break; + } + case 1: { + node.MaybeSetAddrName(fuzzed_data_provider.ConsumeRandomLengthString(32)); + break; + } + case 2: { + node.SetSendVersion(fuzzed_data_provider.ConsumeIntegral<int>()); + break; + } + case 3: { + const std::vector<bool> asmap = ConsumeRandomLengthIntegralVector<bool>(fuzzed_data_provider, 128); + if (!SanityCheckASMap(asmap)) { + break; + } + CNodeStats stats; + node.copyStats(stats, asmap); + break; + } + case 4: { + node.SetRecvVersion(fuzzed_data_provider.ConsumeIntegral<int>()); + break; + } + case 5: { + const CNode* add_ref_node = node.AddRef(); + assert(add_ref_node == &node); + break; + } + case 6: { + if (node.GetRefCount() > 0) { + node.Release(); + } + break; + } + case 7: { + if (node.m_addr_known == nullptr) { + break; + } + const std::optional<CAddress> addr_opt = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!addr_opt) { + break; + } + node.AddAddressKnown(*addr_opt); + break; + } + case 8: { + if (node.m_addr_known == nullptr) { + break; + } + const std::optional<CAddress> addr_opt = ConsumeDeserializable<CAddress>(fuzzed_data_provider); + if (!addr_opt) { + break; + } + FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; + node.PushAddress(*addr_opt, fast_random_context); + break; + } + case 9: { + const std::optional<CInv> inv_opt = ConsumeDeserializable<CInv>(fuzzed_data_provider); + if (!inv_opt) { + break; + } + node.AddKnownTx(inv_opt->hash); + break; + } + case 10: { + node.PushTxInventory(ConsumeUInt256(fuzzed_data_provider)); + break; + } + case 11: { + const std::optional<CService> service_opt = ConsumeDeserializable<CService>(fuzzed_data_provider); + if (!service_opt) { + break; + } + node.SetAddrLocal(*service_opt); + break; + } + case 12: { + const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); + bool complete; + node.ReceiveMsgBytes((const char*)b.data(), b.size(), complete); + break; + } + } + } + + (void)node.GetAddrLocal(); + (void)node.GetAddrName(); + (void)node.GetId(); + (void)node.GetLocalNonce(); + (void)node.GetLocalServices(); + (void)node.GetMyStartingHeight(); + (void)node.GetRecvVersion(); + const int ref_count = node.GetRefCount(); + assert(ref_count >= 0); + (void)node.GetSendVersion(); + (void)node.RelayAddrsWithConn(); + + const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ? + fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({NetPermissionFlags::PF_NONE, NetPermissionFlags::PF_BLOOMFILTER, NetPermissionFlags::PF_RELAY, NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL}) : + static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>()); + (void)node.HasPermission(net_permission_flags); +} diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index ec09acc6c6..52efe5ddfa 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -68,7 +68,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) return; } CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION}; - CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND).release(); + CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY).release(); p2p_node.fSuccessfullyConnected = true; p2p_node.nVersion = PROTOCOL_VERSION; p2p_node.SetSendVersion(PROTOCOL_VERSION); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index ef427442e9..33385c06cf 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -44,7 +44,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3); for (int i = 0; i < num_peers_to_add; ++i) { const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); - const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH}); + const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH}); peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release()); CNode& p2p_node = *peers.back(); diff --git a/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp b/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp new file mode 100644 index 0000000000..d4f302a8d3 --- /dev/null +++ b/src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp @@ -0,0 +1,38 @@ +// Copyright (c) 2020 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 <key.h> +#include <secp256k1.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <vector> + +int ec_seckey_import_der(const secp256k1_context* ctx, unsigned char* out32, const unsigned char* seckey, size_t seckeylen); +int ec_seckey_export_der(const secp256k1_context* ctx, unsigned char* seckey, size_t* seckeylen, const unsigned char* key32, bool compressed); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + secp256k1_context* secp256k1_context_sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN); + { + std::vector<uint8_t> out32(32); + (void)ec_seckey_import_der(secp256k1_context_sign, out32.data(), ConsumeFixedLengthByteVector(fuzzed_data_provider, CKey::SIZE).data(), CKey::SIZE); + } + { + std::vector<uint8_t> seckey(CKey::SIZE); + const std::vector<uint8_t> key32 = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32); + size_t seckeylen = CKey::SIZE; + const bool compressed = fuzzed_data_provider.ConsumeBool(); + const bool exported = ec_seckey_export_der(secp256k1_context_sign, seckey.data(), &seckeylen, key32.data(), compressed); + if (exported) { + std::vector<uint8_t> out32(32); + const bool imported = ec_seckey_import_der(secp256k1_context_sign, out32.data(), seckey.data(), seckey.size()) == 1; + assert(imported && key32 == out32); + } + } + secp256k1_context_destroy(secp256k1_context_sign); +} diff --git a/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp b/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp new file mode 100644 index 0000000000..ed8c7aba89 --- /dev/null +++ b/src/test/fuzz/secp256k1_ecdsa_signature_parse_der_lax.cpp @@ -0,0 +1,33 @@ +// Copyright (c) 2020 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 <key.h> +#include <secp256k1.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <vector> + +bool SigHasLowR(const secp256k1_ecdsa_signature* sig); +int ecdsa_signature_parse_der_lax(const secp256k1_context* ctx, secp256k1_ecdsa_signature* sig, const unsigned char* input, size_t inputlen); + +void test_one_input(const std::vector<uint8_t>& buffer) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const std::vector<uint8_t> signature_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); + if (signature_bytes.data() == nullptr) { + return; + } + secp256k1_context* secp256k1_context_verify = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); + secp256k1_ecdsa_signature sig_der_lax; + const bool parsed_der_lax = ecdsa_signature_parse_der_lax(secp256k1_context_verify, &sig_der_lax, signature_bytes.data(), signature_bytes.size()) == 1; + if (parsed_der_lax) { + ECC_Start(); + (void)SigHasLowR(&sig_der_lax); + ECC_Stop(); + } + secp256k1_context_destroy(secp256k1_context_verify); +} diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 917ae571f5..85ebc89673 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -183,10 +183,20 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) CAddress addr = CAddress(CService(ipv4Addr, 7777), NODE_NETWORK); std::string pszDest; - std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND); + std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY); + BOOST_CHECK(pnode1->IsFullOutboundConn() == true); + BOOST_CHECK(pnode1->IsManualConn() == false); + BOOST_CHECK(pnode1->IsBlockOnlyConn() == false); + BOOST_CHECK(pnode1->IsFeelerConn() == false); + BOOST_CHECK(pnode1->IsAddrFetchConn() == false); BOOST_CHECK(pnode1->IsInboundConn() == false); std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, ConnectionType::INBOUND); + BOOST_CHECK(pnode2->IsFullOutboundConn() == false); + BOOST_CHECK(pnode2->IsManualConn() == false); + BOOST_CHECK(pnode2->IsBlockOnlyConn() == false); + BOOST_CHECK(pnode2->IsFeelerConn() == false); + BOOST_CHECK(pnode2->IsAddrFetchConn() == false); BOOST_CHECK(pnode2->IsInboundConn() == true); } @@ -283,7 +293,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) in_addr ipv4AddrPeer; ipv4AddrPeer.s_addr = 0xa0b0c001; CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK); - std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND); + std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY); pnode->fSuccessfullyConnected.store(true); // the peer claims to be reaching us via IPv6 diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index d9a00c2205..8a88e75892 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -141,8 +141,11 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const pblocktree.reset(new CBlockTreeDB(1 << 20, true)); + m_node.mempool = &::mempool; + m_node.mempool->setSanityCheck(1.0); + m_node.chainman = &::g_chainman; - m_node.chainman->InitializeChainstate(); + m_node.chainman->InitializeChainstate(*m_node.mempool); ::ChainstateActive().InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); assert(!::ChainstateActive().CanFlushToDisk()); @@ -164,8 +167,6 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const } g_parallel_script_checks = true; - m_node.mempool = &::mempool; - m_node.mempool->setSanityCheck(1.0); m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests. m_node.peer_logic = MakeUnique<PeerLogicValidation>(*m_node.connman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 2076a1096a..c8a375275f 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -20,6 +20,7 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup) BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) { ChainstateManager manager; + CTxMemPool mempool; //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view. auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint { @@ -34,7 +35,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) return outp; }; - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool)); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 887a48124f..36badafc4e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -23,12 +23,13 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, TestingSetup) BOOST_AUTO_TEST_CASE(chainstatemanager) { ChainstateManager manager; + CTxMemPool mempool; std::vector<CChainState*> chainstates; const CChainParams& chainparams = Params(); // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -54,7 +55,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // - CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(GetRandHash())); + CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(mempool, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -104,6 +105,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) { ChainstateManager manager; + CTxMemPool mempool; size_t max_cache = 10000; manager.m_total_coinsdb_cache = max_cache; manager.m_total_coinstip_cache = max_cache; @@ -112,7 +114,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a legacy (IBD) chainstate. // - CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool)); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -129,7 +131,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(GetRandHash())); + CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(mempool, GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -147,7 +149,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1); BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1); BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1); - } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 8bac914f05..a3b344d2c9 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -18,8 +18,9 @@ BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, BasicTestingSetup) //! BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { + CTxMemPool mempool; BlockManager blockman{}; - CChainState chainstate{blockman}; + CChainState chainstate{mempool, blockman}; chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false); WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10)); CTxMemPool tx_pool{}; diff --git a/src/threadsafety.h b/src/threadsafety.h index 5f2c40bac6..52bf83b676 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -18,9 +18,7 @@ #define LOCKABLE __attribute__((lockable)) #define SCOPED_LOCKABLE __attribute__((scoped_lockable)) #define GUARDED_BY(x) __attribute__((guarded_by(x))) -#define GUARDED_VAR __attribute__((guarded_var)) #define PT_GUARDED_BY(x) __attribute__((pt_guarded_by(x))) -#define PT_GUARDED_VAR __attribute__((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__))) #define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__((exclusive_lock_function(__VA_ARGS__))) @@ -33,14 +31,12 @@ #define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_required(__VA_ARGS__))) #define SHARED_LOCKS_REQUIRED(...) __attribute__((shared_locks_required(__VA_ARGS__))) #define NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis)) -#define ASSERT_EXCLUSIVE_LOCK(...) __attribute((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__((assert_exclusive_lock(__VA_ARGS__))) #else #define LOCKABLE #define SCOPED_LOCKABLE #define GUARDED_BY(x) -#define GUARDED_VAR #define PT_GUARDED_BY(x) -#define PT_GUARDED_VAR #define ACQUIRED_AFTER(...) #define ACQUIRED_BEFORE(...) #define EXCLUSIVE_LOCK_FUNCTION(...) diff --git a/src/util/system.cpp b/src/util/system.cpp index 00aa53df70..999937d906 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1019,7 +1019,7 @@ bool FileCommit(FILE *file) return false; } #else - #if defined(HAVE_FDATASYNC) + #if HAVE_FDATASYNC if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync LogPrintf("%s: fdatasync failed: %d\n", __func__, errno); return false; diff --git a/src/validation.cpp b/src/validation.cpp index cf2f9dde62..58af8744d9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -370,9 +370,10 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) +static void UpdateMempoolForReorg(CTxMemPool& mempool, DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, mempool.cs) { AssertLockHeld(cs_main); + AssertLockHeld(mempool.cs); std::vector<uint256> vHashUpdate; // disconnectpool's insertion_order index sorts the entries from // oldest to newest, but the oldest entry will be the last tx from the @@ -1254,8 +1255,9 @@ void CoinsViews::InitCache() m_cacheview = MakeUnique<CCoinsViewCache>(&m_catcherview); } -CChainState::CChainState(BlockManager& blockman, uint256 from_snapshot_blockhash) +CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash) : m_blockman(blockman), + m_mempool(mempool), m_from_snapshot_blockhash(from_snapshot_blockhash) {} void CChainState::InitCoinsDB( @@ -2280,7 +2282,7 @@ bool CChainState::FlushStateToDisk( { bool fFlushForPrune = false; bool fDoFullFlush = false; - CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&::mempool); + CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (nManualPruneHeight > 0) { @@ -2426,7 +2428,7 @@ static void AppendWarning(bilingual_str& res, const bilingual_str& warn) } /** Check warning conditions and do some notifications on new chain tip set. */ -void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainParams) +static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const CChainParams& chainParams) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { // New best block @@ -2472,7 +2474,6 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar FormatISO8601DateTime(pindexNew->GetBlockTime()), GuessVerificationProgress(chainParams.TxData(), pindexNew), ::ChainstateActive().CoinsTip().DynamicMemoryUsage() * (1.0 / (1<<20)), ::ChainstateActive().CoinsTip().GetCacheSize(), !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages.original) : ""); - } /** Disconnect m_chain's tip. @@ -2485,8 +2486,11 @@ void static UpdateTip(const CBlockIndex* pindexNew, const CChainParams& chainPar * disconnectpool (note that the caller is responsible for mempool consistency * in any case). */ -bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) +bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) { + AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); + CBlockIndex *pindexDelete = m_chain.Tip(); assert(pindexDelete); // Read block from disk. @@ -2517,14 +2521,14 @@ bool CChainState::DisconnectTip(BlockValidationState& state, const CChainParams& while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { // Drop the earliest entry, and remove its children from the mempool. auto it = disconnectpool->queuedTx.get<insertion_order>().begin(); - mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + m_mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); disconnectpool->removeEntry(it); } } m_chain.SetTip(pindexDelete->pprev); - UpdateTip(pindexDelete->pprev, chainparams); + UpdateTip(m_mempool, pindexDelete->pprev, chainparams); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: GetMainSignals().BlockDisconnected(pblock, pindexDelete); @@ -2585,6 +2589,9 @@ public: */ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) { + AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); + assert(pindexNew->pprev == m_chain.Tip()); // Read block from disk. int64_t nTime1 = GetTimeMicros(); @@ -2625,11 +2632,11 @@ bool CChainState::ConnectTip(BlockValidationState& state, const CChainParams& ch int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime5 - nTime4) * MILLI, nTimeChainState * MICRO, nTimeChainState * MILLI / nBlocksTotal); // Remove conflicting transactions from the mempool.; - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + m_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); disconnectpool.removeForBlock(blockConnecting.vtx); // Update m_chain & related variables. m_chain.SetTip(pindexNew); - UpdateTip(pindexNew, chainparams); + UpdateTip(m_mempool, pindexNew, chainparams); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime6 - nTime5) * MILLI, nTimePostConnect * MICRO, nTimePostConnect * MILLI / nBlocksTotal); @@ -2719,6 +2726,7 @@ void CChainState::PruneBlockIndexCandidates() { bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); + AssertLockHeld(m_mempool.cs); const CBlockIndex *pindexOldTip = m_chain.Tip(); const CBlockIndex *pindexFork = m_chain.FindFork(pindexMostWork); @@ -2730,7 +2738,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai if (!DisconnectTip(state, chainparams, &disconnectpool)) { // This is likely a fatal error, but keep the mempool consistent, // just in case. Only remove from the mempool in this case. - UpdateMempoolForReorg(disconnectpool, false); + UpdateMempoolForReorg(m_mempool, disconnectpool, false); // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort @@ -2774,7 +2782,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai // A system error occurred (disk space, database error, ...). // Make the mempool consistent with the current tip, just in case // any observers try to use it before shutdown. - UpdateMempoolForReorg(disconnectpool, false); + UpdateMempoolForReorg(m_mempool, disconnectpool, false); return false; } } else { @@ -2791,9 +2799,9 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai if (fBlocksDisconnected) { // If any blocks were disconnected, disconnectpool may be non empty. Add // any disconnected transactions back to the mempool. - UpdateMempoolForReorg(disconnectpool, true); + UpdateMempoolForReorg(m_mempool, disconnectpool, true); } - mempool.check(&CoinsTip()); + m_mempool.check(&CoinsTip()); // Callbacks/notifications for a new best chain. if (fInvalidFound) @@ -2867,7 +2875,8 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar LimitValidationInterfaceQueue(); { - LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed + LOCK(cs_main); + LOCK(m_mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -3020,7 +3029,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParam LimitValidationInterfaceQueue(); LOCK(cs_main); - LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between + LOCK(m_mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -3034,7 +3043,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParam // transactions back to the mempool if disconnecting was successful, // and we're not doing a very deep invalidation (in which case // keeping the mempool up to date is probably futile anyway). - UpdateMempoolForReorg(disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); + UpdateMempoolForReorg(m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret); if (!ret) return false; assert(invalid_walk_tip->pprev == m_chain.Tip()); @@ -4517,7 +4526,8 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK2(cs_main, ::mempool.cs); + LOCK(cs_main); + LOCK(m_mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; @@ -5246,7 +5256,7 @@ std::vector<CChainState*> ChainstateManager::GetAll() return out; } -CChainState& ChainstateManager::InitializeChainstate(const uint256& snapshot_blockhash) +CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash) { bool is_snapshot = !snapshot_blockhash.IsNull(); std::unique_ptr<CChainState>& to_modify = @@ -5255,8 +5265,7 @@ CChainState& ChainstateManager::InitializeChainstate(const uint256& snapshot_blo if (to_modify) { throw std::logic_error("should not be overwriting a chainstate"); } - - to_modify.reset(new CChainState(m_blockman, snapshot_blockhash)); + to_modify.reset(new CChainState(mempool, m_blockman, snapshot_blockhash)); // Snapshot chainstates and initial IBD chaintates always become active. if (is_snapshot || (!is_snapshot && !m_active_chainstate)) { diff --git a/src/validation.h b/src/validation.h index 534162d64a..cac9473c7a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -245,7 +245,7 @@ bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_mai * * See consensus/consensus.h for flag definitions. */ -bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, pool.cs); /** * Closure representing one script verification @@ -511,11 +511,14 @@ private: //! easily as opposed to referencing a global. BlockManager& m_blockman; + //! mempool that is kept in sync with the chain + CTxMemPool& m_mempool; + //! Manages the UTXO set, which is a reflection of the contents of `m_chain`. std::unique_ptr<CoinsViews> m_coins_views; public: - explicit CChainState(BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); + explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, uint256 from_snapshot_blockhash = uint256()); /** * Initialize the CoinsViews UTXO set database management data structures. The in-memory @@ -642,7 +645,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. - bool DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool DisconnectTip(BlockValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); // Manual block validity manipulation: bool PreciousBlock(BlockValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -685,8 +688,8 @@ public: std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); private: - bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ActivateBestChainStep(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); + bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs); void InvalidBlockFound(CBlockIndex *pindex, const BlockValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -818,9 +821,11 @@ public: //! Instantiate a new chainstate and assign it based upon whether it is //! from a snapshot. //! + //! @param[in] mempool The mempool to pass to the chainstate + // constructor //! @param[in] snapshot_blockhash If given, signify that this chainstate //! is based on a snapshot. - CChainState& InitializeChainstate(const uint256& snapshot_blockhash = uint256()) + CChainState& InitializeChainstate(CTxMemPool& mempool, const uint256& snapshot_blockhash = uint256()) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! Get all chainstates currently being used. diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index bf05ef844a..3910599ca7 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -5,6 +5,7 @@ #include <init.h> #include <interfaces/chain.h> +#include <interfaces/wallet.h> #include <net.h> #include <node/context.h> #include <node/ui_interface.h> @@ -66,13 +67,13 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-zapwallettxes=<mode>", "Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup" - " (1 = keep tx meta data e.g. payment request information, 2 = drop tx meta data)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); + + argsman.AddHiddenArgs({"-zapwallettxes"}); } bool WalletInit::ParameterInteraction() const @@ -85,26 +86,12 @@ bool WalletInit::ParameterInteraction() const return true; } - const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; - if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - bool zapwallettxes = gArgs.GetBoolArg("-zapwallettxes", false); - // -zapwallettxes implies dropping the mempool on startup - if (zapwallettxes && gArgs.SoftSetBoolArg("-persistmempool", false)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -persistmempool=0\n", __func__); - } - - // -zapwallettxes implies a rescan - if (zapwallettxes) { - if (is_multiwallet) { - return InitError(strprintf(Untranslated("%s is only allowed with a single wallet file"), "-zapwallettxes")); - } - if (gArgs.SoftSetBoolArg("-rescan", true)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -rescan=1\n", __func__); - } + if (gArgs.IsArgSet("-zapwallettxes")) { + return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); } if (gArgs.GetBoolArg("-sysperms", false)) @@ -129,5 +116,7 @@ void WalletInit::Construct(NodeContext& node) const settings.rw_settings["wallet"] = wallets; }); } - node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"))); + auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet")); + node.wallet_client = wallet_client.get(); + node.chain_clients.emplace_back(std::move(wallet_client)); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e0c3a1287a..9e36a09780 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -90,9 +90,9 @@ static void RescanWallet(CWallet& wallet, const WalletRescanReserver& reserver, } } -UniValue importprivkey(const JSONRPCRequest& request) +RPCHelpMan importprivkey() { - RPCHelpMan{"importprivkey", + return RPCHelpMan{"importprivkey", "\nAdds a private key (as returned by dumpprivkey) to your wallet. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one private key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" @@ -116,8 +116,8 @@ UniValue importprivkey(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importprivkey", "\"mykey\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -189,11 +189,13 @@ UniValue importprivkey(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue abortrescan(const JSONRPCRequest& request) +RPCHelpMan abortrescan() { - RPCHelpMan{"abortrescan", + return RPCHelpMan{"abortrescan", "\nStops current wallet rescan triggered by an RPC call, e.g. by an importprivkey call.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", {}, @@ -206,8 +208,8 @@ UniValue abortrescan(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("abortrescan", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -215,11 +217,13 @@ UniValue abortrescan(const JSONRPCRequest& request) if (!pwallet->IsScanning() || pwallet->IsAbortingRescan()) return false; pwallet->AbortRescan(); return true; +}, + }; } -UniValue importaddress(const JSONRPCRequest& request) +RPCHelpMan importaddress() { - RPCHelpMan{"importaddress", + return RPCHelpMan{"importaddress", "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" @@ -243,8 +247,8 @@ UniValue importaddress(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importaddress", "\"myaddress\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -315,11 +319,13 @@ UniValue importaddress(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importprunedfunds(const JSONRPCRequest& request) +RPCHelpMan importprunedfunds() { - RPCHelpMan{"importprunedfunds", + return RPCHelpMan{"importprunedfunds", "\nImports funds without rescan. Corresponding address or script must previously be included in wallet. Aimed towards pruned wallets. The end-user is responsible to import additional transactions that subsequently spend the imported outputs or rescan after the point in the blockchain the transaction is included.\n", { {"rawtransaction", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A raw transaction in hex funding an already-existing address in wallet"}, @@ -327,8 +333,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request) }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{""}, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -371,11 +377,13 @@ UniValue importprunedfunds(const JSONRPCRequest& request) } throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "No addresses in wallet correspond to included transaction"); +}, + }; } -UniValue removeprunedfunds(const JSONRPCRequest& request) +RPCHelpMan removeprunedfunds() { - RPCHelpMan{"removeprunedfunds", + return RPCHelpMan{"removeprunedfunds", "\nDeletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will affect wallet balances.\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex-encoded id of the transaction you are deleting"}, @@ -386,8 +394,8 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("removeprunedfunds", "\"a8d0c0184dde994a09ec054286f1ce581bebf46446a512166eae7628734ea0a5\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -408,11 +416,13 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importpubkey(const JSONRPCRequest& request) +RPCHelpMan importpubkey() { - RPCHelpMan{"importpubkey", + return RPCHelpMan{"importpubkey", "\nAdds a public key (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "Hint: use importmulti to import more than one public key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" @@ -432,8 +442,8 @@ UniValue importpubkey(const JSONRPCRequest& request) "\nAs a JSON-RPC call\n" + HelpExampleRpc("importpubkey", "\"mypubkey\", \"testing\", false") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -492,12 +502,14 @@ UniValue importpubkey(const JSONRPCRequest& request) } return NullUniValue; +}, + }; } -UniValue importwallet(const JSONRPCRequest& request) +RPCHelpMan importwallet() { - RPCHelpMan{"importwallet", + return RPCHelpMan{"importwallet", "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { @@ -512,8 +524,8 @@ UniValue importwallet(const JSONRPCRequest& request) "\nImport using the json rpc call\n" + HelpExampleRpc("importwallet", "\"test\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -649,11 +661,13 @@ UniValue importwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys/scripts to wallet"); return NullUniValue; +}, + }; } -UniValue dumpprivkey(const JSONRPCRequest& request) +RPCHelpMan dumpprivkey() { - RPCHelpMan{"dumpprivkey", + return RPCHelpMan{"dumpprivkey", "\nReveals the private key corresponding to 'address'.\n" "Then the importprivkey can be used with this output\n", { @@ -667,8 +681,8 @@ UniValue dumpprivkey(const JSONRPCRequest& request) + HelpExampleCli("importprivkey", "\"mykey\"") + HelpExampleRpc("dumpprivkey", "\"myaddress\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; const CWallet* const pwallet = wallet.get(); @@ -693,12 +707,14 @@ UniValue dumpprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Private key for address " + strAddress + " is not known"); } return EncodeSecret(vchSecret); +}, + }; } -UniValue dumpwallet(const JSONRPCRequest& request) +RPCHelpMan dumpwallet() { - RPCHelpMan{"dumpwallet", + return RPCHelpMan{"dumpwallet", "\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n" "Imported scripts are included in the dumpfile, but corresponding BIP173 addresses, etc. may not be added automatically by importwallet.\n" "Note that if your wallet contains keys which are not derived from your HD seed (e.g. imported keys), these are not covered by\n" @@ -716,8 +732,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) HelpExampleCli("dumpwallet", "\"test\"") + HelpExampleRpc("dumpwallet", "\"test\"") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return NullUniValue; @@ -829,6 +845,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) reply.pushKV("filename", filepath.string()); return reply; +}, + }; } struct ImportData @@ -1239,9 +1257,9 @@ static int64_t GetImportTimestamp(const UniValue& data, int64_t now) throw JSONRPCError(RPC_TYPE_ERROR, "Missing required timestamp field for key"); } -UniValue importmulti(const JSONRPCRequest& mainRequest) +RPCHelpMan importmulti() { - RPCHelpMan{"importmulti", + return RPCHelpMan{"importmulti", "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), optionally rescanning the blockchain from the earliest creation time of the imported scripts. Requires a new wallet backup.\n" "If an address/script is imported without all of the private keys required to spend from that address, it will be watchonly. The 'watchonly' option must be set to true in this case or a warning will be returned.\n" "Conversely, if all the private keys are provided and the address/script is spendable, the watchonly option must be set to false, or a warning will be returned.\n" @@ -1314,8 +1332,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) "{ \"scriptPubKey\": { \"address\": \"<my 2nd address>\" }, \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") + HelpExampleCli("importmulti", "'[{ \"scriptPubKey\": { \"address\": \"<my address>\" }, \"timestamp\":1455191478 }]' '{ \"rescan\": false}'") }, - }.Check(mainRequest); - + [&](const RPCHelpMan& self, const JSONRPCRequest& mainRequest) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(mainRequest); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -1423,6 +1441,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) } return response; +}, + }; } static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& data, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) @@ -1564,9 +1584,9 @@ static UniValue ProcessDescriptorImport(CWallet * const pwallet, const UniValue& return result; } -UniValue importdescriptors(const JSONRPCRequest& main_request) +RPCHelpMan importdescriptors() { - RPCHelpMan{"importdescriptors", + return RPCHelpMan{"importdescriptors", "\nImport descriptors. This will trigger a rescan of the blockchain based on the earliest timestamp of all descriptors being imported. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if using an early timestamp; during that time, other rpc calls\n" "may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n", @@ -1615,8 +1635,8 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) "{ \"desc\": \"<my desccriptor 2>\", \"label\": \"example 2\", \"timestamp\": 1455191480 }]'") + HelpExampleCli("importdescriptors", "'[{ \"desc\": \"<my descriptor>\", \"timestamp\":1455191478, \"active\": true, \"range\": [0,100], \"label\": \"<my bech32 wallet>\" }]'") }, - }.Check(main_request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& main_request) -> UniValue +{ std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(main_request); if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); @@ -1713,4 +1733,6 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) } return response; +}, + }; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7a03c4f544..7ef0b2129b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2061,6 +2061,7 @@ static UniValue lockunspent(const JSONRPCRequest& request) "Temporarily lock (unlock=false) or unlock (unlock=true) specified transaction outputs.\n" "If no transaction outputs are specified when unlocking then all current locked transaction outputs are unlocked.\n" "A locked transaction output will not be chosen by automatic coin selection, when spending bitcoins.\n" + "Manually selected coins are automatically unlocked.\n" "Locks are stored in memory only. Nodes start with zero locked outputs, and the locked output list\n" "is always cleared (by virtue of process exit) when a node stops or fails.\n" "Also see the listunspent call\n", @@ -2494,7 +2495,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) RPCHelpMan{"loadwallet", "\nLoads a wallet from a wallet file or directory." "\nNote that all wallet command-line options used when starting bitcoind will be" - "\napplied to the new wallet (eg -zapwallettxes, rescan, etc).\n", + "\napplied to the new wallet (eg -rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, @@ -2836,6 +2837,15 @@ static UniValue listunspent(const JSONRPCRequest& request) if (!request.params[4].isNull()) { const UniValue& options = request.params[4].get_obj(); + RPCTypeCheckObj(options, + { + {"minimumAmount", UniValueType()}, + {"maximumAmount", UniValueType()}, + {"minimumSumAmount", UniValueType()}, + {"maximumCount", UniValueType(UniValue::VNUM)}, + }, + true, true); + if (options.exists("minimumAmount")) nMinimumAmount = AmountFromValue(options["minimumAmount"]); @@ -4161,17 +4171,17 @@ static UniValue upgradewallet(const JSONRPCRequest& request) return error.original; } -UniValue abortrescan(const JSONRPCRequest& request); // in rpcdump.cpp -UniValue dumpprivkey(const JSONRPCRequest& request); // in rpcdump.cpp -UniValue importprivkey(const JSONRPCRequest& request); -UniValue importaddress(const JSONRPCRequest& request); -UniValue importpubkey(const JSONRPCRequest& request); -UniValue dumpwallet(const JSONRPCRequest& request); -UniValue importwallet(const JSONRPCRequest& request); -UniValue importprunedfunds(const JSONRPCRequest& request); -UniValue removeprunedfunds(const JSONRPCRequest& request); -UniValue importmulti(const JSONRPCRequest& request); -UniValue importdescriptors(const JSONRPCRequest& request); +RPCHelpMan abortrescan(); +RPCHelpMan dumpprivkey(); +RPCHelpMan importprivkey(); +RPCHelpMan importaddress(); +RPCHelpMan importpubkey(); +RPCHelpMan dumpwallet(); +RPCHelpMan importwallet(); +RPCHelpMan importprunedfunds(); +RPCHelpMan removeprunedfunds(); +RPCHelpMan importmulti(); +RPCHelpMan importdescriptors(); Span<const CRPCCommand> GetWalletRPCCommands() { diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index c0755db751..934e3d5c86 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -16,6 +16,11 @@ static const char *HEADER_END = "HEADER=END"; static const char *DATA_END = "DATA=END"; typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; +static bool KeyFilter(const std::string& type) +{ + return WalletBatch::IsKeyType(type) || type == DBKeys::HDCHAIN; +} + bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings) { std::string filename; @@ -129,9 +134,9 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v { // Required in LoadKeyMetadata(): LOCK(dummyWallet.cs_wallet); - fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr); + fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue, strType, strErr, KeyFilter); } - if (!WalletBatch::IsKeyType(strType) && strType != DBKeys::HDCHAIN) { + if (!KeyFilter(strType)) { continue; } if (!fReadOK) diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index a96d971734..14fb1fa89f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -535,7 +535,7 @@ private: //! keeps track of whether Unlock has run a thorough check before bool m_decryption_thoroughly_checked = false; - bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey); + bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 35bd965673..c23dea1338 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -10,7 +10,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - m_chain_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {}); std::string sep; sep += fs::path::preferred_separator; diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h index c95b4f1f6e..f5bade77df 100644 --- a/src/wallet/test/init_test_fixture.h +++ b/src/wallet/test/init_test_fixture.h @@ -6,6 +6,7 @@ #define BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H #include <interfaces/chain.h> +#include <interfaces/wallet.h> #include <node/context.h> #include <test/util/setup_common.h> @@ -19,7 +20,7 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup { fs::path m_cwd; std::map<std::string, fs::path> m_walletdir_path_cases; std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::ChainClient> m_chain_client; + std::unique_ptr<interfaces::WalletClient> m_wallet_client; }; #endif // BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index c228e06009..9b905569fc 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -15,7 +15,7 @@ BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default) { SetWalletDir(m_walletdir_path_cases["default"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); @@ -25,7 +25,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom) { SetWalletDir(m_walletdir_path_cases["custom"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]); @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist) SetWalletDir(m_walletdir_path_cases["nonexistent"]); { ASSERT_DEBUG_LOG("does not exist"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory) SetWalletDir(m_walletdir_path_cases["file"]); { ASSERT_DEBUG_LOG("is not a directory"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -57,7 +57,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative) SetWalletDir(m_walletdir_path_cases["relative"]); { ASSERT_DEBUG_LOG("is a relative path"); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == false); } } @@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) { SetWalletDir(m_walletdir_path_cases["trailing"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); @@ -75,7 +75,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) { SetWalletDir(m_walletdir_path_cases["trailing2"]); - bool result = m_chain_client->verify(); + bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 44f9eb5ddd..9241470745 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -11,5 +11,5 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) bool fFirstRun; m_wallet.LoadWallet(fFirstRun); m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); - m_chain_client->registerRpcs(); + m_wallet_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 99d7cfe921..12ad13b49b 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -21,7 +21,7 @@ struct WalletTestingSetup : public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node); - std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); CWallet m_wallet; std::unique_ptr<interfaces::Handler> m_chain_notifications_handler; }; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7ef06663b5..c46e90d64b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -24,9 +24,9 @@ #include <boost/test/unit_test.hpp> #include <univalue.h> -extern UniValue importmulti(const JSONRPCRequest& request); -extern UniValue dumpwallet(const JSONRPCRequest& request); -extern UniValue importwallet(const JSONRPCRequest& request); +RPCHelpMan importmulti(); +RPCHelpMan dumpwallet(); +RPCHelpMan importwallet(); // Ensure that fee levels defined in the wallet are at least as high // as the default levels for node policy. @@ -219,7 +219,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(keys); - UniValue response = importmulti(request); + UniValue response = importmulti().HandleRequest(request); BOOST_CHECK_EQUAL(response.write(), strprintf("[{\"success\":false,\"error\":{\"code\":-1,\"message\":\"Rescan failed for key with creation " "timestamp %d. There was an error reading a block from time %d, which is after or within %d " @@ -274,7 +274,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(backup_file); - ::dumpwallet(request); + ::dumpwallet().HandleRequest(request); RemoveWallet(wallet); } @@ -291,7 +291,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.push_back(backup_file); AddWallet(wallet); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - ::importwallet(request); + ::importwallet().HandleRequest(request); RemoveWallet(wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c132a4be42..839e74a2be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -183,11 +183,6 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati return wallet; } -std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) -{ - return LoadWallet(chain, WalletLocation(name), error, warnings); -} - WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result) { // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted @@ -276,7 +271,7 @@ std::string COutput::ToString() const const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const { - LOCK(cs_wallet); + AssertLockHeld(cs_wallet); std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(hash); if (it == mapWallet.end()) return nullptr; @@ -1210,15 +1205,13 @@ void CWallet::BlockUntilSyncedToCurrentChain() const { isminetype CWallet::IsMine(const CTxIn &txin) const { + AssertLockHeld(cs_wallet); + std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash); + if (mi != mapWallet.end()) { - LOCK(cs_wallet); - std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash); - if (mi != mapWallet.end()) - { - const CWalletTx& prev = (*mi).second; - if (txin.prevout.n < prev.tx->vout.size()) - return IsMine(prev.tx->vout[txin.prevout.n]); - } + const CWalletTx& prev = (*mi).second; + if (txin.prevout.n < prev.tx->vout.size()) + return IsMine(prev.tx->vout[txin.prevout.n]); } return ISMINE_NO; } @@ -1243,16 +1236,19 @@ CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const isminetype CWallet::IsMine(const CTxOut& txout) const { + AssertLockHeld(cs_wallet); return IsMine(txout.scriptPubKey); } isminetype CWallet::IsMine(const CTxDestination& dest) const { + AssertLockHeld(cs_wallet); return IsMine(GetScriptForDestination(dest)); } isminetype CWallet::IsMine(const CScript& script) const { + AssertLockHeld(cs_wallet); isminetype result = ISMINE_NO; for (const auto& spk_man_pair : m_spk_managers) { result = std::max(result, spk_man_pair.second->IsMine(script)); @@ -1264,6 +1260,7 @@ CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) cons { if (!MoneyRange(txout.nValue)) throw std::runtime_error(std::string(__func__) + ": value out of range"); + LOCK(cs_wallet); return ((IsMine(txout) & filter) ? txout.nValue : 0); } @@ -1281,13 +1278,12 @@ bool CWallet::IsChange(const CScript& script) const // a better way of identifying which outputs are 'the send' and which are // 'the change' will need to be implemented (maybe extend CWalletTx to remember // which output, if any, was change). + AssertLockHeld(cs_wallet); if (IsMine(script)) { CTxDestination address; if (!ExtractDestination(script, address)) return true; - - LOCK(cs_wallet); if (!FindAddressBookEntry(address)) { return true; } @@ -1297,6 +1293,7 @@ bool CWallet::IsChange(const CScript& script) const CAmount CWallet::GetChange(const CTxOut& txout) const { + AssertLockHeld(cs_wallet); if (!MoneyRange(txout.nValue)) throw std::runtime_error(std::string(__func__) + ": value out of range"); return (IsChange(txout) ? txout.nValue : 0); @@ -1304,6 +1301,7 @@ CAmount CWallet::GetChange(const CTxOut& txout) const bool CWallet::IsMine(const CTransaction& tx) const { + AssertLockHeld(cs_wallet); for (const CTxOut& txout : tx.vout) if (IsMine(txout)) return true; @@ -1362,6 +1360,7 @@ CAmount CWallet::GetCredit(const CTransaction& tx, const isminefilter& filter) c CAmount CWallet::GetChange(const CTransaction& tx) const { + LOCK(cs_wallet); CAmount nChange = 0; for (const CTxOut& txout : tx.vout) { @@ -1597,6 +1596,7 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, nFee = nDebit - nValueOut; } + LOCK(pwallet->cs_wallet); // Sent/received. for (unsigned int i = 0; i < tx->vout.size(); ++i) { @@ -1965,36 +1965,38 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted() const { - std::set<uint256> s; - return IsTrusted(s); + std::set<uint256> trusted_parents; + LOCK(pwallet->cs_wallet); + return pwallet->IsTrusted(*this, trusted_parents); } -bool CWalletTx::IsTrusted(std::set<uint256>& trusted_parents) const +bool CWallet::IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const { + AssertLockHeld(cs_wallet); // Quick answer in most cases - if (!pwallet->chain().checkFinalTx(*tx)) return false; - int nDepth = GetDepthInMainChain(); + if (!chain().checkFinalTx(*wtx.tx)) return false; + int nDepth = wtx.GetDepthInMainChain(); if (nDepth >= 1) return true; if (nDepth < 0) return false; // using wtx's cached debit - if (!pwallet->m_spend_zero_conf_change || !IsFromMe(ISMINE_ALL)) return false; + if (!m_spend_zero_conf_change || !wtx.IsFromMe(ISMINE_ALL)) return false; // Don't trust unconfirmed transactions from us unless they are in the mempool. - if (!InMempool()) return false; + if (!wtx.InMempool()) return false; // Trusted if all inputs are from us and are in the mempool: - for (const CTxIn& txin : tx->vin) + for (const CTxIn& txin : wtx.tx->vin) { // Transactions not sent by us: not trusted - const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); + const CWalletTx* parent = GetWalletTx(txin.prevout.hash); if (parent == nullptr) return false; const CTxOut& parentOut = parent->tx->vout[txin.prevout.n]; // Check that this specific input being spent is trusted - if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE) return false; + if (IsMine(parentOut) != ISMINE_SPENDABLE) return false; // If we've already trusted this parent, continue if (trusted_parents.count(parent->GetHash())) continue; // Recurse to check that the parent is also trusted - if (!parent->IsTrusted(trusted_parents)) return false; + if (!IsTrusted(*parent, trusted_parents)) return false; trusted_parents.insert(parent->GetHash()); } return true; @@ -2080,7 +2082,7 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; - const bool is_trusted{wtx.IsTrusted(trusted_parents)}; + const bool is_trusted{IsTrusted(wtx, trusted_parents)}; const int tx_depth{wtx.GetDepthInMainChain()}; const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; @@ -2148,7 +2150,7 @@ void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const if (nDepth == 0 && !wtx.InMempool()) continue; - bool safeTx = wtx.IsTrusted(trusted_parents); + bool safeTx = IsTrusted(wtx, trusted_parents); // We should not consider coins from transactions that are replacing // other transactions. @@ -2580,10 +2582,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); - if (lockUnspents) { - LockCoin(txin.prevout); - } } + if (lockUnspents) { + LockCoin(txin.prevout); + } + } return true; @@ -3172,28 +3175,10 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 return DBErrors::LOAD_OK; } -DBErrors CWallet::ZapWalletTx(std::list<CWalletTx>& vWtx) -{ - DBErrors nZapWalletTxRet = WalletBatch(*database,"cr+").ZapWalletTx(vWtx); - if (nZapWalletTxRet == DBErrors::NEED_REWRITE) - { - if (database->Rewrite("\x04pool")) - { - for (const auto& spk_man_pair : m_spk_managers) { - spk_man_pair.second->RewriteDB(); - } - } - } - - if (nZapWalletTxRet != DBErrors::LOAD_OK) - return nZapWalletTxRet; - - return DBErrors::LOAD_OK; -} - bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { bool fUpdated = false; + bool is_mine; { LOCK(cs_wallet); std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address); @@ -3201,8 +3186,9 @@ bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& add m_address_book[address].SetLabel(strName); if (!strPurpose.empty()) /* update purpose only if requested */ m_address_book[address].purpose = strPurpose; + is_mine = IsMine(address) != ISMINE_NO; } - NotifyAddressBookChanged(this, address, strName, IsMine(address) != ISMINE_NO, + NotifyAddressBookChanged(this, address, strName, is_mine, strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); if (!strPurpose.empty() && !batch.WritePurpose(EncodeDestination(address), strPurpose)) return false; @@ -3217,17 +3203,16 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s bool CWallet::DelAddressBook(const CTxDestination& address) { - // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) - // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! - // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). - if (IsMine(address)) { - WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); - return false; - } - + bool is_mine; { LOCK(cs_wallet); - + // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) + // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! + // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). + if (IsMine(address)) { + WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); + return false; + } // Delete destdata tuples associated with address std::string strAddress = EncodeDestination(address); for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata) @@ -3235,9 +3220,10 @@ bool CWallet::DelAddressBook(const CTxDestination& address) WalletBatch(*database).EraseDestData(strAddress, item.first); } m_address_book.erase(address); + is_mine = IsMine(address) != ISMINE_NO; } - NotifyAddressBookChanged(this, address, "", IsMine(address) != ISMINE_NO, "", CT_DELETED); + NotifyAddressBookChanged(this, address, "", is_mine, "", CT_DELETED); WalletBatch(*database).ErasePurpose(EncodeDestination(address)); return WalletBatch(*database).EraseName(EncodeDestination(address)); @@ -3345,7 +3331,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const { const CWalletTx& wtx = walletEntry.second; - if (!wtx.IsTrusted(trusted_parents)) + if (!IsTrusted(wtx, trusted_parents)) continue; if (wtx.IsImmatureCoinBase()) @@ -3364,9 +3350,6 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const continue; CAmount n = IsSpent(walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue; - - if (!balances.count(addr)) - balances[addr] = 0; balances[addr] += n; } } @@ -3770,20 +3753,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, { const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); - // needed to restore wallet transaction meta data after -zapwallettxes - std::list<CWalletTx> vWtx; - - if (gArgs.GetBoolArg("-zapwallettxes", false)) { - chain.initMessage(_("Zapping all transactions from wallet...").translated); - - std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(&chain, location, CreateWalletDatabase(location.GetPath())); - DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); - if (nZapWalletRet != DBErrors::LOAD_OK) { - error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); - return nullptr; - } - } - chain.initMessage(_("Loading wallet...").translated); int64_t nStart = GetTimeMillis(); @@ -4060,30 +4029,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->database->IncrementUpdateCounter(); - - // Restore wallet transaction metadata after -zapwallettxes=1 - if (gArgs.GetBoolArg("-zapwallettxes", false) && gArgs.GetArg("-zapwallettxes", "1") != "2") - { - WalletBatch batch(*walletInstance->database); - - for (const CWalletTx& wtxOld : vWtx) - { - uint256 hash = wtxOld.GetHash(); - std::map<uint256, CWalletTx>::iterator mi = walletInstance->mapWallet.find(hash); - if (mi != walletInstance->mapWallet.end()) - { - const CWalletTx* copyFrom = &wtxOld; - CWalletTx* copyTo = &mi->second; - copyTo->mapValue = copyFrom->mapValue; - copyTo->vOrderForm = copyFrom->vOrderForm; - copyTo->nTimeReceived = copyFrom->nTimeReceived; - copyTo->nTimeSmart = copyFrom->nTimeSmart; - copyTo->fFromMe = copyFrom->fFromMe; - copyTo->nOrderPos = copyFrom->nOrderPos; - batch.WriteTx(*copyTo); - } - } - } } { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2f9d301000..42622c9312 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -275,7 +275,7 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, class CWalletTx { private: - const CWallet* pwallet; + const CWallet* const pwallet; /** Constant used in hashBlock to indicate tx has been abandoned, only used at * serialization/deserialization to avoid ambiguity with conflicted. @@ -502,7 +502,6 @@ public: bool InMempool() const; bool IsTrusted() const; - bool IsTrusted(std::set<uint256>& trusted_parents) const; int64_t GetTxTime() const; @@ -805,7 +804,8 @@ public: /** Interface for accessing chain state. */ interfaces::Chain& chain() const { assert(m_chain); return *m_chain; } - const CWalletTx* GetWalletTx(const uint256& hash) const; + const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! check whether we are allowed to upgrade (or already support) to the named feature bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } @@ -1051,20 +1051,20 @@ public: bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error); bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error); - isminetype IsMine(const CTxDestination& dest) const; - isminetype IsMine(const CScript& script) const; - isminetype IsMine(const CTxIn& txin) const; + isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Returns amount of debit if the input matches the * filter, otherwise returns 0 */ CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; - isminetype IsMine(const CTxOut& txout) const; + isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; - bool IsChange(const CTxOut& txout) const; - bool IsChange(const CScript& script) const; - CAmount GetChange(const CTxOut& txout) const; - bool IsMine(const CTransaction& tx) const; + bool IsChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsChange(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + CAmount GetChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** should probably be renamed to IsRelevantToMe */ bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; @@ -1075,7 +1075,6 @@ public: void chainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); - DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& purpose); @@ -1179,7 +1178,7 @@ public: * Obviously holding cs_main/cs_wallet when going into this call may cause * deadlock */ - void BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(cs_main, cs_wallet); + void BlockUntilSyncedToCurrentChain() const EXCLUSIVE_LOCKS_REQUIRED(!::cs_main, !cs_wallet); /** set a single wallet flag */ void SetWalletFlag(uint64_t flags); @@ -1285,7 +1284,7 @@ public: void LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal); //! Create new DescriptorScriptPubKeyMans and add them to the wallet - void SetupDescriptorScriptPubKeyMans(); + void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet DescriptorScriptPubKeyMan* GetDescriptorScriptPubKeyMan(const WalletDescriptor& desc) const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index fa6814d0d3..962ea66fa0 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -263,13 +263,17 @@ public: static bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, - CWalletScanState &wss, std::string& strType, std::string& strErr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) + CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { try { // Unserialize // Taking advantage of the fact that pair serialization // is just the two items serialized one after the other ssKey >> strType; + // If we have a filter, check if this matches the filter + if (filter_fn && !filter_fn(strType)) { + return true; + } if (strType == DBKeys::NAME) { std::string strAddress; ssKey >> strAddress; @@ -668,11 +672,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return true; } -bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr) +bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn) { CWalletScanState dummy_wss; LOCK(pwallet->cs_wallet); - return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr); + return ReadKeyValue(pwallet, ssKey, ssValue, dummy_wss, strType, strErr, filter_fn); } bool WalletBatch::IsKeyType(const std::string& strType) @@ -926,23 +930,6 @@ DBErrors WalletBatch::ZapSelectTx(std::vector<uint256>& vTxHashIn, std::vector<u return DBErrors::LOAD_OK; } -DBErrors WalletBatch::ZapWalletTx(std::list<CWalletTx>& vWtx) -{ - // build list of wallet TXs - std::vector<uint256> vTxHash; - DBErrors err = FindWalletTx(vTxHash, vWtx); - if (err != DBErrors::LOAD_OK) - return err; - - // erase each wallet TX - for (const uint256& hash : vTxHash) { - if (!EraseTx(hash)) - return DBErrors::CORRUPT; - } - - return DBErrors::LOAD_OK; -} - void MaybeCompactWalletDB() { static std::atomic<bool> fOneThread(false); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 64d60b1f44..2548c17508 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -257,7 +257,6 @@ public: DBErrors LoadWallet(CWallet* pwallet); DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx); - DBErrors ZapWalletTx(std::list<CWalletTx>& vWtx); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); @@ -280,8 +279,11 @@ private: //! Compacts BDB state so that wallet.dat is self-contained (if there are changes) void MaybeCompactWalletDB(); +//! Callback for filtering key types to deserialize in ReadKeyValue +using KeyFilterFn = std::function<bool(const std::string&)>; + //! Unserialize a given Key-Value pair and load it into the wallet -bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr); +bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); /** Return whether a wallet database is currently loaded. */ bool IsWalletLoaded(const fs::path& wallet_path); diff --git a/src/zmq/zmqpublishnotifier.cpp b/src/zmq/zmqpublishnotifier.cpp index 04806903c2..e2431cbbb7 100644 --- a/src/zmq/zmqpublishnotifier.cpp +++ b/src/zmq/zmqpublishnotifier.cpp @@ -86,6 +86,14 @@ bool CZMQAbstractPublishNotifier::Initialize(void *pcontext) return false; } + const int so_keepalive_option {1}; + rc = zmq_setsockopt(psocket, ZMQ_TCP_KEEPALIVE, &so_keepalive_option, sizeof(so_keepalive_option)); + if (rc != 0) { + zmqError("Failed to set SO_KEEPALIVE"); + zmq_close(psocket); + return false; + } + rc = zmq_bind(psocket, address.c_str()); if (rc != 0) { diff --git a/src/zmq/zmqrpc.cpp b/src/zmq/zmqrpc.cpp index cce6210129..1dd751b493 100644 --- a/src/zmq/zmqrpc.cpp +++ b/src/zmq/zmqrpc.cpp @@ -13,9 +13,9 @@ namespace { -UniValue getzmqnotifications(const JSONRPCRequest& request) +static RPCHelpMan getzmqnotifications() { - RPCHelpMan{"getzmqnotifications", + return RPCHelpMan{"getzmqnotifications", "\nReturns information about the active ZeroMQ notifications.\n", {}, RPCResult{ @@ -33,8 +33,8 @@ UniValue getzmqnotifications(const JSONRPCRequest& request) HelpExampleCli("getzmqnotifications", "") + HelpExampleRpc("getzmqnotifications", "") }, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ UniValue result(UniValue::VARR); if (g_zmq_notification_interface != nullptr) { for (const auto* n : g_zmq_notification_interface->GetActiveNotifiers()) { @@ -47,6 +47,8 @@ UniValue getzmqnotifications(const JSONRPCRequest& request) } return result; +}, + }; } const CRPCCommand commands[] = |