diff options
Diffstat (limited to 'src')
112 files changed, 1735 insertions, 1145 deletions
diff --git a/src/.clang-tidy b/src/.clang-tidy index e9807d4cb7..df2a080075 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,12 +1,14 @@ Checks: ' -*, bugprone-argument-comment, +misc-unused-using-decls, modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, ' WarningsAsErrors: ' bugprone-argument-comment, +misc-unused-using-decls, modernize-use-default-member-init, modernize-use-nullptr, readability-redundant-declaration, diff --git a/src/Makefile.am b/src/Makefile.am index 3fbbe180fc..02ec5c098e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -177,6 +177,7 @@ BITCOIN_CORE_H = \ kernel/context.h \ kernel/mempool_limits.h \ kernel/mempool_options.h \ + kernel/mempool_persist.h \ key.h \ key_io.h \ logging.h \ @@ -198,6 +199,7 @@ BITCOIN_CORE_H = \ node/chainstate.h \ node/coin.h \ node/context.h \ + node/mempool_persist_args.h \ node/miner.h \ node/minisketchwrapper.h \ node/psbt.h \ @@ -261,7 +263,6 @@ BITCOIN_CORE_H = \ util/bip32.h \ util/bytevectorhash.h \ util/check.h \ - util/designator.h \ util/epochguard.h \ util/error.h \ util/fastrange.h \ @@ -277,6 +278,7 @@ BITCOIN_CORE_H = \ util/overloaded.h \ util/rbf.h \ util/readwritefile.h \ + util/result.h \ util/serfloat.h \ util/settings.h \ util/sock.h \ @@ -366,6 +368,7 @@ libbitcoin_node_a_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ mapport.cpp \ mempool_args.cpp \ net.cpp \ @@ -379,6 +382,7 @@ libbitcoin_node_a_SOURCES = \ node/context.cpp \ node/eviction.cpp \ node/interfaces.cpp \ + node/mempool_persist_args.cpp \ node/miner.cpp \ node/minisketchwrapper.cpp \ node/psbt.cpp \ @@ -881,6 +885,7 @@ libbitcoinkernel_la_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ key.cpp \ logging.cpp \ node/blockstorage.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b806b62d5b..d9195ad32e 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -373,7 +373,7 @@ endif $(AM_V_at)$(MAKE) $(AM_MAKEFLAGS) -C secp256k1 check if ENABLE_TESTS -UNIVALUE_TESTS = univalue/test/object univalue/test/unitester univalue/test/no_nul +UNIVALUE_TESTS = univalue/test/object univalue/test/unitester noinst_PROGRAMS += $(UNIVALUE_TESTS) TESTS += $(UNIVALUE_TESTS) @@ -382,11 +382,6 @@ univalue_test_unitester_LDADD = $(LIBUNIVALUE) univalue_test_unitester_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) -DJSON_TEST_SRC=\"$(srcdir)/$(UNIVALUE_TEST_DATA_DIR_INT)\" univalue_test_unitester_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) -univalue_test_no_nul_SOURCES = $(UNIVALUE_TEST_NO_NUL_INT) -univalue_test_no_nul_LDADD = $(LIBUNIVALUE) -univalue_test_no_nul_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) -univalue_test_no_nul_LDFLAGS = -static $(LIBTOOL_APP_LDFLAGS) - univalue_test_object_SOURCES = $(UNIVALUE_TEST_OBJECT_INT) univalue_test_object_LDADD = $(LIBUNIVALUE) univalue_test_object_CPPFLAGS = -I$(srcdir)/$(UNIVALUE_INCLUDE_DIR_INT) diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 8922dda3ad..b43816636f 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -10,6 +10,7 @@ EXTRA_LIBRARIES += \ TEST_FUZZ_H = \ test/fuzz/fuzz.h \ test/fuzz/FuzzedDataProvider.h \ + test/fuzz/mempool_utils.h \ test/fuzz/util.h libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index f611383788..a10f7ff7d1 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -19,8 +19,6 @@ using wallet::CWallet; using wallet::DatabaseFormat; using wallet::DatabaseOptions; -using wallet::ISMINE_SPENDABLE; -using wallet::MakeWalletDatabase; using wallet::TxStateInactive; using wallet::WALLET_FLAG_DESCRIPTORS; using wallet::WalletContext; @@ -47,12 +45,11 @@ static void BenchUnloadWallet(std::shared_ptr<CWallet>&& wallet) static void AddTx(CWallet& wallet) { - bilingual_str error; - CTxDestination dest; - wallet.GetNewDestination(OutputType::BECH32, "", dest, error); + const auto& dest = wallet.GetNewDestination(OutputType::BECH32, ""); + assert(dest.HasRes()); CMutableTransaction mtx; - mtx.vout.push_back({COIN, GetScriptForDestination(dest)}); + mtx.vout.push_back({COIN, GetScriptForDestination(dest.GetObj())}); mtx.vin.push_back(CTxIn()); wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInactive{}); diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 1817aa1a53..d53e917aba 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -78,8 +78,8 @@ int main(int argc, char* argv[]) // SETUP: Chainstate const ChainstateManager::Options chainman_opts{ - chainparams, - static_cast<int64_t(*)()>(GetTime), + .chainparams = chainparams, + .adjusted_time_callback = static_cast<int64_t (*)()>(GetTime), }; ChainstateManager chainman{chainman_opts}; @@ -9,6 +9,7 @@ #include <cstdio> #include <filesystem> +#include <functional> #include <iomanip> #include <ios> #include <ostream> @@ -199,6 +200,7 @@ bool create_directories(const std::filesystem::path& p, std::error_code& ec) = d /** Bridge operations to C stdio */ namespace fsbridge { + using FopenFn = std::function<FILE*(const fs::path&, const char*)>; FILE *fopen(const fs::path& p, const char *mode); /** diff --git a/src/init.cpp b/src/init.cpp index eff37e1a83..5ed1def4db 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -10,6 +10,7 @@ #include <init.h> #include <kernel/checks.h> +#include <kernel/mempool_persist.h> #include <addrman.h> #include <banman.h> @@ -41,6 +42,7 @@ #include <node/chainstate.h> #include <node/context.h> #include <node/interface_ui.h> +#include <node/mempool_persist_args.h> #include <node/miner.h> #include <policy/feerate.h> #include <policy/fees.h> @@ -64,7 +66,6 @@ #include <txorphanage.h> #include <util/asmap.h> #include <util/check.h> -#include <util/designator.h> #include <util/moneystr.h> #include <util/strencodings.h> #include <util/string.h> @@ -103,14 +104,18 @@ #include <zmq/zmqrpc.h> #endif +using kernel::DumpMempool; + using node::CacheSizes; using node::CalculateCacheSizes; using node::ChainstateLoadVerifyError; using node::ChainstateLoadingError; -using node::CleanupBlockRevFiles; +using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINTPRIORITY; using node::DEFAULT_STOPAFTERBLOCKIMPORT; using node::LoadChainstate; +using node::MempoolPath; +using node::ShouldPersistMempool; using node::NodeContext; using node::ThreadImport; using node::VerifyLoadedChainstate; @@ -246,8 +251,8 @@ void Shutdown(NodeContext& node) node.addrman.reset(); node.netgroupman.reset(); - if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - DumpMempool(*node.mempool); + if (node.mempool && node.mempool->GetLoadTried() && ShouldPersistMempool(*node.args)) { + DumpMempool(*node.mempool, MempoolPath(*node.args)); } // Drop transactions we were still watching, and record fee estimations. @@ -1425,8 +1430,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.chainman); CTxMemPool::Options mempool_opts{ - Desig(estimator) node.fee_estimator.get(), - Desig(check_ratio) chainparams.DefaultConsistencyChecks() ? 1 : 0, + .estimator = node.fee_estimator.get(), + .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, }; ApplyArgsManOptions(args, mempool_opts); mempool_opts.check_ratio = std::clamp<int>(mempool_opts.check_ratio, 0, 1'000'000); @@ -1441,8 +1446,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.mempool = std::make_unique<CTxMemPool>(mempool_opts); const ChainstateManager::Options chainman_opts{ - chainparams, - GetAdjustedTime, + .chainparams = chainparams, + .adjusted_time_callback = GetAdjustedTime, }; node.chainman = std::make_unique<ChainstateManager>(chainman_opts); ChainstateManager& chainman = *node.chainman; @@ -1670,7 +1675,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { - ThreadImport(chainman, vImportFiles, args); + ThreadImport(chainman, vImportFiles, args, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); // Wait for genesis block to be processed diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index ddfb4bda95..ff994b8edc 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -111,6 +111,10 @@ public: //! Get locator for the current chain tip. virtual CBlockLocator getTipLocator() = 0; + //! Return a locator that refers to a block in the active chain. + //! If specified block is not in the active chain, return locator for the latest ancestor that is in the chain. + virtual CBlockLocator getActiveChainLocator(const uint256& block_hash) = 0; + //! Return height of the highest block on chain in common with the locator, //! which will either be the original block used to create the locator, //! or one of its ancestors. @@ -283,6 +287,9 @@ public: //! to be prepared to handle this by ignoring notifications about unknown //! removed transactions and already added new transactions. virtual void requestMempoolTransactions(Notifications& notifications) = 0; + + //! Return true if an assumed-valid chain is in use. + virtual bool hasAssumedValidChain() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index b3cb0ae387..e29fefae56 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -12,6 +12,7 @@ #include <script/standard.h> // For CTxDestination #include <support/allocators/secure.h> // For SecureString #include <util/message.h> +#include <util/result.h> #include <util/ui_change_type.h> #include <cstdint> @@ -87,7 +88,7 @@ public: virtual std::string getWalletName() = 0; // Get a new address. - virtual bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) = 0; + virtual BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) = 0; //! Get public key. virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0; @@ -138,12 +139,11 @@ public: virtual void listLockedCoins(std::vector<COutPoint>& outputs) = 0; //! Create transaction. - virtual CTransactionRef createTransaction(const std::vector<wallet::CRecipient>& recipients, + virtual BResult<CTransactionRef> createTransaction(const std::vector<wallet::CRecipient>& recipients, const wallet::CCoinControl& coin_control, bool sign, int& change_pos, - CAmount& fee, - bilingual_str& fail_reason) = 0; + CAmount& fee) = 0; //! Commit transaction. virtual void commitTransaction(CTransactionRef tx, @@ -329,7 +329,7 @@ public: virtual std::string getWalletDir() = 0; //! Restore backup wallet - virtual std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + virtual BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0; //! Return available wallets in wallet directory. virtual std::vector<std::string> listWalletDir() = 0; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 575d94e2e9..510a1f9edc 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -10,6 +10,8 @@ class CChainParams; +namespace kernel { + /** * An options struct for `ChainstateManager`, more ergonomically referred to as * `ChainstateManager::Options` due to the using-declaration in @@ -20,4 +22,6 @@ struct ChainstateManagerOpts { const std::function<int64_t()> adjusted_time_callback{nullptr}; }; +} // namespace kernel + #endif // BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp new file mode 100644 index 0000000000..1a1cf2bbdc --- /dev/null +++ b/src/kernel/mempool_persist.cpp @@ -0,0 +1,189 @@ +// Copyright (c) 2022 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 <kernel/mempool_persist.h> + +#include <clientversion.h> +#include <consensus/amount.h> +#include <fs.h> +#include <logging.h> +#include <primitives/transaction.h> +#include <serialize.h> +#include <shutdown.h> +#include <streams.h> +#include <sync.h> +#include <txmempool.h> +#include <uint256.h> +#include <util/system.h> +#include <util/time.h> +#include <validation.h> + +#include <chrono> +#include <cstdint> +#include <cstdio> +#include <exception> +#include <functional> +#include <map> +#include <memory> +#include <set> +#include <stdexcept> +#include <utility> +#include <vector> + +using fsbridge::FopenFn; + +namespace kernel { + +static const uint64_t MEMPOOL_DUMP_VERSION = 1; + +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function) +{ + if (load_path.empty()) return false; + + FILE* filestr{mockable_fopen_function(load_path, "rb")}; + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + if (file.IsNull()) { + LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); + return false; + } + + int64_t count = 0; + int64_t expired = 0; + int64_t failed = 0; + int64_t already_there = 0; + int64_t unbroadcast = 0; + auto now = NodeClock::now(); + + try { + uint64_t version; + file >> version; + if (version != MEMPOOL_DUMP_VERSION) { + return false; + } + uint64_t num; + file >> num; + while (num) { + --num; + CTransactionRef tx; + int64_t nTime; + int64_t nFeeDelta; + file >> tx; + file >> nTime; + file >> nFeeDelta; + + CAmount amountdelta = nFeeDelta; + if (amountdelta) { + pool.PrioritiseTransaction(tx->GetHash(), amountdelta); + } + if (nTime > TicksSinceEpoch<std::chrono::seconds>(now - pool.m_expiry)) { + LOCK(cs_main); + const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); + if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { + ++count; + } else { + // mempool may contain the transaction already, e.g. from + // wallet(s) having loaded it while we were processing + // mempool transactions; consider these as valid, instead of + // failed, but mark them as 'already there' + if (pool.exists(GenTxid::Txid(tx->GetHash()))) { + ++already_there; + } else { + ++failed; + } + } + } else { + ++expired; + } + if (ShutdownRequested()) + return false; + } + std::map<uint256, CAmount> mapDeltas; + file >> mapDeltas; + + for (const auto& i : mapDeltas) { + pool.PrioritiseTransaction(i.first, i.second); + } + + std::set<uint256> unbroadcast_txids; + file >> unbroadcast_txids; + unbroadcast = unbroadcast_txids.size(); + for (const auto& txid : unbroadcast_txids) { + // Ensure transactions were accepted to mempool then add to + // unbroadcast set. + if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); + } + } catch (const std::exception& e) { + LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); + return false; + } + + LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); + return true; +} + +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit) +{ + auto start = SteadyClock::now(); + + std::map<uint256, CAmount> mapDeltas; + std::vector<TxMempoolInfo> vinfo; + std::set<uint256> unbroadcast_txids; + + static Mutex dump_mutex; + LOCK(dump_mutex); + + { + LOCK(pool.cs); + for (const auto &i : pool.mapDeltas) { + mapDeltas[i.first] = i.second; + } + vinfo = pool.infoAll(); + unbroadcast_txids = pool.GetUnbroadcastTxs(); + } + + auto mid = SteadyClock::now(); + + try { + FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; + if (!filestr) { + return false; + } + + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + + uint64_t version = MEMPOOL_DUMP_VERSION; + file << version; + + file << (uint64_t)vinfo.size(); + for (const auto& i : vinfo) { + file << *(i.tx); + file << int64_t{count_seconds(i.m_time)}; + file << int64_t{i.nFeeDelta}; + mapDeltas.erase(i.tx->GetHash()); + } + + file << mapDeltas; + + LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); + file << unbroadcast_txids; + + if (!skip_file_commit && !FileCommit(file.Get())) + throw std::runtime_error("FileCommit failed"); + file.fclose(); + if (!RenameOver(dump_path + ".new", dump_path)) { + throw std::runtime_error("Rename failed"); + } + auto last = SteadyClock::now(); + + LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + Ticks<SecondsDouble>(mid - start), + Ticks<SecondsDouble>(last - mid)); + } catch (const std::exception& e) { + LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + return false; + } + return true; +} + +} // namespace kernel diff --git a/src/kernel/mempool_persist.h b/src/kernel/mempool_persist.h new file mode 100644 index 0000000000..9a15ec6dca --- /dev/null +++ b/src/kernel/mempool_persist.h @@ -0,0 +1,28 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_KERNEL_MEMPOOL_PERSIST_H +#define BITCOIN_KERNEL_MEMPOOL_PERSIST_H + +#include <fs.h> + +class CChainState; +class CTxMemPool; + +namespace kernel { + +/** Dump the mempool to disk. */ +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen, + bool skip_file_commit = false); + +/** Load the mempool from disk. */ +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, + CChainState& active_chainstate, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); + +} // namespace kernel + + +#endif // BITCOIN_KERNEL_MEMPOOL_PERSIST_H diff --git a/src/net.cpp b/src/net.cpp index c37d90519c..2c4a1e4bae 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -26,7 +26,6 @@ #include <protocol.h> #include <random.h> #include <scheduler.h> -#include <util/designator.h> #include <util/sock.h> #include <util/strencodings.h> #include <util/syscall_sandbox.h> @@ -207,15 +206,13 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn) // Otherwise, return the unroutable 0.0.0.0 but filled in with // the normal parameters, since the IP may be changed to a useful // one by discovery. -CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices) +CService GetLocalAddress(const CNetAddr& addrPeer) { - CAddress ret(CService(CNetAddr(),GetListenPort()), nLocalServices); + CService ret{CNetAddr(), GetListenPort()}; CService addr; - if (GetLocal(addr, paddrPeer)) - { - ret = CAddress(addr, nLocalServices); + if (GetLocal(addr, &addrPeer)) { + ret = CService{addr}; } - ret.nTime = GetAdjustedTime(); return ret; } @@ -234,35 +231,35 @@ bool IsPeerAddrLocalGood(CNode *pnode) IsReachable(addrLocal.GetNetwork()); } -std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode) +std::optional<CService> GetLocalAddrForPeer(CNode& node) { - CAddress addrLocal = GetLocalAddress(&pnode->addr, pnode->GetLocalServices()); + CService addrLocal{GetLocalAddress(node.addr)}; if (gArgs.GetBoolArg("-addrmantest", false)) { // use IPv4 loopback during addrmantest - addrLocal = CAddress(CService(LookupNumeric("127.0.0.1", GetListenPort())), pnode->GetLocalServices()); + addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort())); } // If discovery is enabled, sometimes give our peer the address it // tells us that it sees us as in case it has a better idea of our // address than we do. FastRandomContext rng; - if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() || + if (IsPeerAddrLocalGood(&node) && (!addrLocal.IsRoutable() || rng.randbits((GetnScore(addrLocal) > LOCAL_MANUAL) ? 3 : 1) == 0)) { - if (pnode->IsInboundConn()) { + if (node.IsInboundConn()) { // For inbound connections, assume both the address and the port // as seen from the peer. - addrLocal = CAddress{pnode->GetAddrLocal(), addrLocal.nServices, addrLocal.nTime}; + addrLocal = CService{node.GetAddrLocal()}; } else { // For outbound connections, assume just the address as seen from // the peer and leave the port in `addrLocal` as returned by // `GetLocalAddress()` above. The peer has no way to observe our // listening port when we have initiated the connection. - addrLocal.SetIP(pnode->GetAddrLocal()); + addrLocal.SetIP(node.GetAddrLocal()); } } if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false)) { - LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), pnode->GetId()); + LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToString(), node.GetId()); return addrLocal; } // Address is unroutable. Don't advertise. @@ -544,7 +541,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo addr_bind = GetBindAddress(*sock); } CNode* pnode = new CNode(id, - nLocalServices, std::move(sock), addrConnect, CalculateKeyedNetGroup(addrConnect), @@ -604,7 +600,6 @@ Network CNode::ConnectedThroughNetwork() const void CNode::CopyStats(CNodeStats& stats) { stats.nodeid = this->GetId(); - X(nServices); X(addr); X(addrBind); stats.m_network = ConnectedThroughNetwork(); @@ -876,20 +871,20 @@ bool CConnman::AttemptToEvictConnection() if (node->fDisconnect) continue; NodeEvictionCandidate candidate{ - Desig(id) node->GetId(), - Desig(m_connected) node->m_connected, - Desig(m_min_ping_time) node->m_min_ping_time, - Desig(m_last_block_time) node->m_last_block_time, - Desig(m_last_tx_time) node->m_last_tx_time, - Desig(fRelevantServices) HasAllDesirableServiceFlags(node->nServices), - Desig(m_relay_txs) node->m_relays_txs.load(), - Desig(fBloomFilter) node->m_bloom_filter_loaded.load(), - Desig(nKeyedNetGroup) node->nKeyedNetGroup, - Desig(prefer_evict) node->m_prefer_evict, - Desig(m_is_local) node->addr.IsLocal(), - Desig(m_network) node->ConnectedThroughNetwork(), - Desig(m_noban) node->HasPermission(NetPermissionFlags::NoBan), - Desig(m_conn_type) node->m_conn_type, + .id = node->GetId(), + .m_connected = node->m_connected, + .m_min_ping_time = node->m_min_ping_time, + .m_last_block_time = node->m_last_block_time, + .m_last_tx_time = node->m_last_tx_time, + .fRelevantServices = node->m_has_all_wanted_services, + .m_relay_txs = node->m_relays_txs.load(), + .fBloomFilter = node->m_bloom_filter_loaded.load(), + .nKeyedNetGroup = node->nKeyedNetGroup, + .prefer_evict = node->m_prefer_evict, + .m_is_local = node->addr.IsLocal(), + .m_network = node->ConnectedThroughNetwork(), + .m_noban = node->HasPermission(NetPermissionFlags::NoBan), + .m_conn_type = node->m_conn_type, }; vEvictionCandidates.push_back(candidate); } @@ -1015,7 +1010,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); CNode* pnode = new CNode(id, - nodeServices, std::move(sock), addr, CalculateKeyedNetGroup(addr), @@ -1027,7 +1021,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock, pnode->AddRef(); pnode->m_permissionFlags = permissionFlags; pnode->m_prefer_evict = discouraged; - m_msgproc->InitializeNode(pnode); + m_msgproc->InitializeNode(*pnode, nodeServices); LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString()); @@ -1965,7 +1959,7 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai if (grantOutbound) grantOutbound->MoveTo(pnode->grantOutbound); - m_msgproc->InitializeNode(pnode); + m_msgproc->InitializeNode(*pnode, nLocalServices); { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); @@ -2709,7 +2703,10 @@ ServiceFlags CConnman::GetLocalServices() const unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } -CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion) +CNode::CNode(NodeId idIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, + uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, + const CAddress& addrBindIn, const std::string& addrNameIn, + ConnectionType conn_type_in, bool inbound_onion) : m_sock{sock}, m_connected{GetTime<std::chrono::seconds>()}, addr(addrIn), @@ -2719,8 +2716,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> s nKeyedNetGroup(nKeyedNetGroupIn), id(idIn), nLocalHostNonce(nLocalHostNonceIn), - m_conn_type(conn_type_in), - nLocalServices(nLocalServicesIn) + m_conn_type(conn_type_in) { if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND); @@ -144,8 +144,8 @@ enum }; bool IsPeerAddrLocalGood(CNode *pnode); -/** Returns a local address that we should advertise to this peer */ -std::optional<CAddress> GetLocalAddrForPeer(CNode *pnode); +/** Returns a local address that we should advertise to this peer. */ +std::optional<CService> GetLocalAddrForPeer(CNode& node); /** * Mark a network as reachable or unreachable (no automatic connects to it) @@ -163,7 +163,7 @@ void RemoveLocal(const CService& addr); bool SeenLocal(const CService& addr); bool IsLocal(const CService& addr); bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr); -CAddress GetLocalAddress(const CNetAddr *paddrPeer, ServiceFlags nLocalServices); +CService GetLocalAddress(const CNetAddr& addrPeer); extern bool fDiscover; @@ -187,7 +187,6 @@ class CNodeStats { public: NodeId nodeid; - ServiceFlags nServices; std::chrono::seconds m_last_send; std::chrono::seconds m_last_recv; std::chrono::seconds m_last_tx_time; @@ -346,7 +345,6 @@ public: std::unique_ptr<TransportSerializer> m_serializer; NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; - std::atomic<ServiceFlags> nServices{NODE_NONE}; /** * Socket used for communication with the node. @@ -399,8 +397,6 @@ public: bool HasPermission(NetPermissionFlags permission) const { return NetPermissions::HasFlag(m_permissionFlags, permission); } - bool fClient{false}; // set by version message - bool m_limited_node{false}; //after BIP159, set by version message /** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */ std::atomic_bool fSuccessfullyConnected{false}; // Setting fDisconnect to true will cause the node to be disconnected the @@ -484,6 +480,9 @@ public: // Peer selected us as (compact blocks) high-bandwidth peer (BIP152) std::atomic<bool> m_bip152_highbandwidth_from{false}; + /** Whether this peer provides all services that we want. Used for eviction decisions */ + std::atomic_bool m_has_all_wanted_services{false}; + /** Whether we should relay transactions to this peer (their version * message did not include fRelay=false and this is not a block-relay-only * connection). This only changes from false to true. It will never change @@ -514,7 +513,10 @@ public: * criterium in CConnman::AttemptToEvictConnection. */ std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()}; - CNode(NodeId id, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion); + CNode(NodeId id, std::shared_ptr<Sock> sock, const CAddress& addrIn, + uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, + const CAddress& addrBindIn, const std::string& addrNameIn, + ConnectionType conn_type_in, bool inbound_onion); CNode(const CNode&) = delete; CNode& operator=(const CNode&) = delete; @@ -572,11 +574,6 @@ public: void CopyStats(CNodeStats& stats) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv); - ServiceFlags GetLocalServices() const - { - return nLocalServices; - } - std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */ @@ -591,23 +588,6 @@ private: const ConnectionType m_conn_type; std::atomic<int> m_greatest_common_version{INIT_PROTO_VERSION}; - //! Services offered to this peer. - //! - //! This is supplied by the parent CConnman during peer connection - //! (CConnman::ConnectNode()) from its attribute of the same name. - //! - //! This is const because there is no protocol defined for renegotiating - //! services initially offered to a peer. The set of local services we - //! offer should not change after initialization. - //! - //! An interesting example of this is NODE_NETWORK and initial block - //! download: a node which starts up from scratch doesn't have any blocks - //! to serve, but still advertises NODE_NETWORK because it will eventually - //! fulfill this role after IBD completes. P2P code is written in such a - //! way that it can gracefully handle peers who don't make good on their - //! service advertisements. - const ServiceFlags nLocalServices; - std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread // Our address, as reported by the peer @@ -625,7 +605,7 @@ class NetEventsInterface { public: /** Initialize a peer (setup state, queue any initial messages) */ - virtual void InitializeNode(CNode* pnode) = 0; + virtual void InitializeNode(CNode& node, ServiceFlags our_services) = 0; /** Handle removal of a peer (clear state) */ virtual void FinalizeNode(const CNode& node) = 0; @@ -1035,16 +1015,14 @@ private: std::map<uint64_t, CachedAddrResponse> m_addr_response_caches; /** - * Services this instance offers. + * Services this node offers. * - * This data is replicated in each CNode instance we create during peer - * connection (in ConnectNode()) under a member also called - * nLocalServices. + * This data is replicated in each Peer instance we create. * * This data is not marked const, but after being set it should not - * change. See the note in CNode::nLocalServices documentation. + * change. * - * \sa CNode::nLocalServices + * \sa Peer::our_services */ ServiceFlags nLocalServices; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c33dd29923..74d1bf44d2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -207,6 +207,23 @@ struct Peer { /** Same id as the CNode object for this peer */ const NodeId m_id{0}; + /** Services we offered to this peer. + * + * This is supplied by CConnman during peer initialization. It's const + * because there is no protocol defined for renegotiating services + * initially offered to a peer. The set of local services we offer should + * not change after initialization. + * + * An interesting example of this is NODE_NETWORK and initial block + * download: a node which starts up from scratch doesn't have any blocks + * to serve, but still advertises NODE_NETWORK because it will eventually + * fulfill this role after IBD completes. P2P code is written in such a + * way that it can gracefully handle peers who don't make good on their + * service advertisements. */ + const ServiceFlags m_our_services; + /** Services this peer offered to us. */ + std::atomic<ServiceFlags> m_their_services{NODE_NONE}; + /** Protects misbehavior data members */ Mutex m_misbehavior_mutex; /** Accumulated misbehavior score for this peer */ @@ -358,10 +375,11 @@ struct Peer { std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); /** Time of the last getheaders message to this peer */ - std::atomic<NodeClock::time_point> m_last_getheaders_timestamp{NodeSeconds{}}; + NodeClock::time_point m_last_getheaders_timestamp{}; - Peer(NodeId id) + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} + , m_our_services{our_services} {} private: @@ -410,8 +428,6 @@ struct CNodeState { bool m_requested_hb_cmpctblocks{false}; /** Whether this peer will send us cmpctblocks if we request them. */ bool m_provides_cmpctblocks{false}; - //! Whether this peer can give us witnesses - bool fHaveWitness{false}; /** State used to enforce CHAIN_SYNC_TIMEOUT and EXTRA_PEER_CHECK_INTERVAL logic. * @@ -482,7 +498,7 @@ public: EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); /** Implement NetEventsInterface */ - void InitializeNode(CNode* pnode) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex); @@ -492,7 +508,8 @@ public: /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; void CheckForStaleTipAndEvictPeers() override; - std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override; + std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -578,7 +595,7 @@ private: */ bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer); /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ - void HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* pindexLast); + void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast); /** Update peer state based on received headers message */ void UpdatePeerStateForReceivedHeaders(CNode& pfrom, const CBlockIndex *pindexLast, bool received_new_header, bool may_have_more_headers); @@ -657,7 +674,7 @@ private: /** Get a pointer to a mutable CNodeState. */ CNodeState* State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - uint32_t GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + uint32_t GetFetchFlags(const Peer& peer) const; std::atomic<std::chrono::microseconds> m_next_inv_to_inbounds{0us}; @@ -778,7 +795,7 @@ private: /** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has * at most count entries. */ - void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) EXCLUSIVE_LOCKS_REQUIRED(cs_main); std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> > mapBlocksInFlight GUARDED_BY(cs_main); @@ -848,6 +865,7 @@ private: * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] filter_type The filter type the request is for. Must be basic filters. * @param[in] start_height The start height for the request @@ -857,7 +875,7 @@ private: * @param[out] filter_index The filter index, if the request can be serviced. * @return True if the request can be serviced. */ - bool PrepareBlockFilterRequest(CNode& peer, + bool PrepareBlockFilterRequest(CNode& node, Peer& peer, BlockFilterType filter_type, uint32_t start_height, const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, @@ -868,30 +886,33 @@ private: * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFilters(CNode& peer, CDataStream& vRecv); + void ProcessGetCFilters(CNode& node, Peer& peer, CDataStream& vRecv); /** * Handle a cfheaders request. * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv); + void ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv); /** * Handle a getcfcheckpt request. * * May disconnect from the peer in the case of a bad request. * + * @param[in] node The node that we received the request from * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv); + void ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv); /** Checks if address relay is permitted with peer. If needed, initializes * the m_addr_known bloom filter and sets m_addr_relay_enabled to true. @@ -955,6 +976,26 @@ static void AddKnownTx(Peer& peer, const uint256& hash) tx_relay->m_tx_inventory_known_filter.insert(hash); } +/** Whether this peer can serve us blocks. */ +static bool CanServeBlocks(const Peer& peer) +{ + return peer.m_their_services & (NODE_NETWORK|NODE_NETWORK_LIMITED); +} + +/** Whether this peer can only serve limited recent blocks (e.g. because + * it prunes old blocks) */ +static bool IsLimitedPeer(const Peer& peer) +{ + return (!(peer.m_their_services & NODE_NETWORK) && + (peer.m_their_services & NODE_NETWORK_LIMITED)); +} + +/** Whether this peer can serve us witness data */ +static bool CanServeWitnesses(const Peer& peer) +{ + return peer.m_their_services & NODE_WITNESS; +} + std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, std::chrono::seconds average_interval) { @@ -1148,17 +1189,17 @@ void PeerManagerImpl::UpdateBlockAvailability(NodeId nodeid, const uint256 &hash } } -void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) +void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) { if (count == 0) return; vBlocks.reserve(vBlocks.size() + count); - CNodeState *state = State(nodeid); + CNodeState *state = State(peer.m_id); assert(state != nullptr); // Make sure pindexBestKnownBlock is up to date, we'll need it. - ProcessBlockAvailability(nodeid); + ProcessBlockAvailability(peer.m_id); if (state->pindexBestKnownBlock == nullptr || state->pindexBestKnownBlock->nChainWork < m_chainman.ActiveChain().Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < nMinimumChainWork) { // This peer has nothing interesting. @@ -1206,7 +1247,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // We consider the chain that this peer is on invalid. return; } - if (!State(nodeid)->fHaveWitness && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { + if (!CanServeWitnesses(peer) && DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_SEGWIT)) { // We wouldn't download this block or its descendants from this peer. return; } @@ -1217,7 +1258,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count // The block is not already downloaded, and not yet in flight. if (pindex->nHeight > nWindowEnd) { // We reached the end of the window. - if (vBlocks.size() == 0 && waitingfor != nodeid) { + if (vBlocks.size() == 0 && waitingfor != peer.m_id) { // We aren't able to fetch anything, but we would be if the download window was one larger. nodeStaller = waitingfor; } @@ -1239,10 +1280,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(NodeId nodeid, unsigned int count void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) { - // Note that pnode->GetLocalServices() is a reflection of the local - // services we were offering when the CNode object was created for this - // peer. - uint64_t my_services{pnode.GetLocalServices()}; + uint64_t my_services{peer.m_our_services}; const int64_t nTime{count_seconds(GetTime<std::chrono::seconds>())}; uint64_t nonce = pnode.GetLocalNonce(); const int nNodeStartingHeight{m_best_height}; @@ -1299,21 +1337,21 @@ void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_s if (state) state->m_last_block_announcement = time_in_seconds; } -void PeerManagerImpl::InitializeNode(CNode *pnode) +void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) { - NodeId nodeid = pnode->GetId(); + NodeId nodeid = node.GetId(); { LOCK(cs_main); - m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(pnode->IsInboundConn())); + m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); assert(m_txrequest.Count(nodeid) == 0); } - PeerRef peer = std::make_shared<Peer>(nodeid); + PeerRef peer = std::make_shared<Peer>(nodeid, our_services); { LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); } - if (!pnode->IsInboundConn()) { - PushNodeVersion(*pnode, *peer); + if (!node.IsInboundConn()) { + PushNodeVersion(node, *peer); } } @@ -1431,6 +1469,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c PeerRef peer = GetPeerRef(nodeid); if (peer == nullptr) return false; + stats.their_services = peer->m_their_services; stats.m_starting_height = peer->m_starting_height; // It is common for nodes with good ping times to suddenly become lagged, // due to a new block arriving or other large transfer. @@ -1585,12 +1624,14 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl if (fImporting) return "Importing..."; if (fReindex) return "Reindexing..."; - LOCK(cs_main); // Ensure this peer exists and hasn't been disconnected - CNodeState* state = State(peer_id); - if (state == nullptr) return "Peer does not exist"; + PeerRef peer = GetPeerRef(peer_id); + if (peer == nullptr) return "Peer does not exist"; + // Ignore pre-segwit peers - if (!state->fHaveWitness) return "Pre-SegWit peer"; + if (!CanServeWitnesses(*peer)) return "Pre-SegWit peer"; + + LOCK(cs_main); // Mark block as in-flight unless it already is (for this peer). // If a block was already in-flight for a different peer, its BLOCKTXN @@ -1974,7 +2015,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( - (((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) + (((peer.m_our_services & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((peer.m_our_services & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId()); //disconnect node and prevent it from stalling (would otherwise wait for the missing block) @@ -2191,10 +2232,10 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } } -uint32_t PeerManagerImpl::GetFetchFlags(const CNode& pfrom) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +uint32_t PeerManagerImpl::GetFetchFlags(const Peer& peer) const { uint32_t nFetchFlags = 0; - if (State(pfrom.GetId())->fHaveWitness) { + if (CanServeWitnesses(peer)) { nFetchFlags |= MSG_WITNESS_FLAG; } return nFetchFlags; @@ -2276,7 +2317,7 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& loc // Only allow a new getheaders message to go out if we don't have a recent // one already in-flight - if (current_time - peer.m_last_getheaders_timestamp.load() > HEADERS_RESPONSE_TIME) { + if (current_time - peer.m_last_getheaders_timestamp > HEADERS_RESPONSE_TIME) { m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, locator, uint256())); peer.m_last_getheaders_timestamp = current_time; return true; @@ -2289,7 +2330,7 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& loc * We require that the given tip have at least as much work as our tip, and for * our current tip to be "close to synced" (see CanDirectFetch()). */ -void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* pindexLast) +void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast) { const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); @@ -2304,7 +2345,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* while (pindexWalk && !m_chainman.ActiveChain().Contains(pindexWalk) && vToFetch.size() <= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { if (!(pindexWalk->nStatus & BLOCK_HAVE_DATA) && !IsBlockRequested(pindexWalk->GetBlockHash()) && - (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || State(pfrom.GetId())->fHaveWitness)) { + (!DeploymentActiveAt(*pindexWalk, m_chainman, Consensus::DEPLOYMENT_SEGWIT) || CanServeWitnesses(peer))) { // We don't have this block, and it's not yet in flight. vToFetch.push_back(pindexWalk); } @@ -2326,7 +2367,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const CBlockIndex* // Can't download any more from this peer break; } - uint32_t nFetchFlags = GetFetchFlags(pfrom); + uint32_t nFetchFlags = GetFetchFlags(peer); vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); BlockRequested(pfrom.GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", @@ -2471,7 +2512,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS); // Consider immediately downloading blocks. - HeadersDirectFetchBlocks(pfrom, pindexLast); + HeadersDirectFetchBlocks(pfrom, peer, pindexLast); return; } @@ -2555,7 +2596,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set) } } -bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, +bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, BlockFilterType filter_type, uint32_t start_height, const uint256& stop_hash, uint32_t max_height_diff, const CBlockIndex*& stop_index, @@ -2563,11 +2604,11 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, { const bool supported_filter_type = (filter_type == BlockFilterType::BASIC && - (peer.GetLocalServices() & NODE_COMPACT_FILTERS)); + (peer.m_our_services & NODE_COMPACT_FILTERS)); if (!supported_filter_type) { LogPrint(BCLog::NET, "peer %d requested unsupported block filter type: %d\n", - peer.GetId(), static_cast<uint8_t>(filter_type)); - peer.fDisconnect = true; + node.GetId(), static_cast<uint8_t>(filter_type)); + node.fDisconnect = true; return false; } @@ -2578,8 +2619,8 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, // Check that the stop block exists and the peer would be allowed to fetch it. if (!stop_index || !BlockRequestAllowed(stop_index)) { LogPrint(BCLog::NET, "peer %d requested invalid block hash: %s\n", - peer.GetId(), stop_hash.ToString()); - peer.fDisconnect = true; + node.GetId(), stop_hash.ToString()); + node.fDisconnect = true; return false; } } @@ -2588,14 +2629,14 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, if (start_height > stop_height) { LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ "start height %d and stop height %d\n", - peer.GetId(), start_height, stop_height); - peer.fDisconnect = true; + node.GetId(), start_height, stop_height); + node.fDisconnect = true; return false; } if (stop_height - start_height >= max_height_diff) { LogPrint(BCLog::NET, "peer %d requested too many cfilters/cfheaders: %d / %d\n", - peer.GetId(), stop_height - start_height + 1, max_height_diff); - peer.fDisconnect = true; + node.GetId(), stop_height - start_height + 1, max_height_diff); + node.fDisconnect = true; return false; } @@ -2608,7 +2649,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& peer, return true; } -void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFilters(CNode& node,Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -2620,7 +2661,7 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, start_height, stop_hash, MAX_GETCFILTERS_SIZE, stop_index, filter_index)) { return; } @@ -2633,13 +2674,13 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& peer, CDataStream& vRecv) } for (const auto& filter : filters) { - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFILTER, filter); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } } -void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -2651,7 +2692,7 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, start_height, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, start_height, stop_hash, MAX_GETCFHEADERS_SIZE, stop_index, filter_index)) { return; } @@ -2674,16 +2715,16 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& peer, CDataStream& vRecv) return; } - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFHEADERS, filter_type_ser, stop_index->GetBlockHash(), prev_header, filter_hashes); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } -void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv) { uint8_t filter_type_ser; uint256 stop_hash; @@ -2694,7 +2735,7 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) const CBlockIndex* stop_index; BlockFilterIndex* filter_index; - if (!PrepareBlockFilterRequest(peer, filter_type, /*start_height=*/0, stop_hash, + if (!PrepareBlockFilterRequest(node, peer, filter_type, /*start_height=*/0, stop_hash, /*max_height_diff=*/std::numeric_limits<uint32_t>::max(), stop_index, filter_index)) { return; @@ -2715,12 +2756,12 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& peer, CDataStream& vRecv) } } - CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) + CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) .Make(NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); - m_connman.PushMessage(&peer, std::move(msg)); + m_connman.PushMessage(&node, std::move(msg)); } void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr<const CBlock>& block, bool force_processing) @@ -2842,7 +2883,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); - pfrom.nServices = nServices; + pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices); + peer->m_their_services = nServices; pfrom.SetAddrLocal(addrMe); { LOCK(pfrom.m_subver_mutex); @@ -2850,18 +2892,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } peer->m_starting_height = starting_height; - // set nodes not relaying blocks and tx and not serving (parts) of the historical blockchain as "clients" - pfrom.fClient = (!(nServices & NODE_NETWORK) && !(nServices & NODE_NETWORK_LIMITED)); - - // set nodes not capable of serving the complete blockchain history as "limited nodes" - pfrom.m_limited_node = (!(nServices & NODE_NETWORK) && (nServices & NODE_NETWORK_LIMITED)); - // We only initialize the m_tx_relay data structure if: // - this isn't an outbound block-relay-only connection; and // - fRelay=true or we're offering NODE_BLOOM to this peer // (NODE_BLOOM means that the peer may turn on tx relay later) if (!pfrom.IsBlockOnlyConn() && - (fRelay || (pfrom.GetLocalServices() & NODE_BLOOM))) { + (fRelay || (peer->m_our_services & NODE_BLOOM))) { auto* const tx_relay = peer->SetTxRelay(); { LOCK(tx_relay->m_bloom_filter_mutex); @@ -2870,17 +2906,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (fRelay) pfrom.m_relays_txs = true; } - if((nServices & NODE_WITNESS)) - { - LOCK(cs_main); - State(pfrom.GetId())->fHaveWitness = true; - } - // Potentially mark this peer as a preferred download peer. { LOCK(cs_main); CNodeState* state = State(pfrom.GetId()); - state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && !pfrom.fClient; + state->fPreferredDownload = (!pfrom.IsInboundConn() || pfrom.HasPermission(NetPermissionFlags::NoBan)) && !pfrom.IsAddrFetchConn() && CanServeBlocks(*peer); m_num_preferred_download_peers += state->fPreferredDownload; } @@ -2899,7 +2929,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // indicate to the peer that we will participate in addr relay. if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { - CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); + CAddress addr{GetLocalAddress(pfrom.addr), peer->m_our_services, (uint32_t)GetAdjustedTime()}; FastRandomContext insecure_rand; if (addr.IsRoutable()) { @@ -3758,7 +3788,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We requested this block for some reason, but our mempool will probably be useless // so we just grab the block via normal getdata std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); } return; @@ -3794,7 +3824,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } @@ -3837,7 +3867,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We requested this block, but its far into the future, so our // mempool will probably be useless - request the block normally std::vector<CInv> vInv(1); - vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom), cmpctblock.header.GetHash()); + vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); return; } else { @@ -3921,7 +3951,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( std::vector<CInv> invs; - invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom), resp.blockhash)); + invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(*peer), resp.blockhash)); m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); } else { // Block is either okay, or possibly we received @@ -3974,7 +4004,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Assume that this is in response to any outstanding getheaders // request we may have sent, and clear out the time of our last request - peer->m_last_getheaders_timestamp.store(NodeSeconds{}); + peer->m_last_getheaders_timestamp = {}; std::vector<CBlockHeader> headers; @@ -4061,7 +4091,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::MEMPOOL) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) + if (!(peer->m_our_services & NODE_BLOOM) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) { if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { @@ -4164,7 +4194,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERLOAD) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filterload received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4189,7 +4219,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERADD) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filteradd received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4217,7 +4247,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::FILTERCLEAR) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM)) { + if (!(peer->m_our_services & NODE_BLOOM)) { LogPrint(BCLog::NET, "filterclear received despite not offering bloom services from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -4248,17 +4278,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::GETCFILTERS) { - ProcessGetCFilters(pfrom, vRecv); + ProcessGetCFilters(pfrom, *peer, vRecv); return; } if (msg_type == NetMsgType::GETCFHEADERS) { - ProcessGetCFHeaders(pfrom, vRecv); + ProcessGetCFHeaders(pfrom, *peer, vRecv); return; } if (msg_type == NetMsgType::GETCFCHECKPT) { - ProcessGetCFCheckPt(pfrom, vRecv); + ProcessGetCFCheckPt(pfrom, *peer, vRecv); return; } @@ -4654,9 +4684,10 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros if (peer.m_next_local_addr_send != 0us) { peer.m_addr_known->reset(); } - if (std::optional<CAddress> local_addr = GetLocalAddrForPeer(&node)) { + if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) { + CAddress local_addr{*local_service, peer.m_our_services, (uint32_t)GetAdjustedTime()}; FastRandomContext insecure_rand; - PushAddress(peer, *local_addr, insecure_rand); + PushAddress(peer, local_addr, insecure_rand); } peer.m_next_local_addr_send = GetExponentialRand(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } @@ -4840,7 +4871,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) bool sync_blocks_and_headers_from_peer = false; if (state.fPreferredDownload) { sync_blocks_and_headers_from_peer = true; - } else if (!pto->fClient && !pto->IsAddrFetchConn()) { + } else if (CanServeBlocks(*peer) && !pto->IsAddrFetchConn()) { // Typically this is an inbound peer. If we don't have any outbound // peers, or if we aren't downloading any blocks from such peers, // then allow block downloads from this peer, too. @@ -4855,7 +4886,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } - if (!state.fSyncStarted && !pto->fClient && !fImporting && !fReindex) { + if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex) { // Only actively request headers from a single peer, unless we're close to today. if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) { const CBlockIndex* pindexStart = m_chainman.m_best_header; @@ -5232,12 +5263,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Message: getdata (blocks) // std::vector<CInv> vGetData; - if (!pto->fClient && ((sync_blocks_and_headers_from_peer && !pto->m_limited_node) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (CanServeBlocks(*peer) && ((sync_blocks_and_headers_from_peer && !IsLimitedPeer(*peer)) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { std::vector<const CBlockIndex*> vToDownload; NodeId staller = -1; - FindNextBlocksToDownload(pto->GetId(), MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); + FindNextBlocksToDownload(*peer, MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.nBlocksInFlight, vToDownload, staller); for (const CBlockIndex *pindex : vToDownload) { - uint32_t nFetchFlags = GetFetchFlags(*pto); + uint32_t nFetchFlags = GetFetchFlags(*peer); vGetData.push_back(CInv(MSG_BLOCK | nFetchFlags, pindex->GetBlockHash())); BlockRequested(pto->GetId(), *pindex); LogPrint(BCLog::NET, "Requesting block %s (%d) peer=%d\n", pindex->GetBlockHash().ToString(), @@ -5264,7 +5295,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!AlreadyHaveTx(gtxid)) { LogPrint(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", gtxid.GetHash().ToString(), pto->GetId()); - vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), gtxid.GetHash()); + vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); if (vGetData.size() >= MAX_GETDATA_SZ) { m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); vGetData.clear(); diff --git a/src/net_processing.h b/src/net_processing.h index 5fbae98c27..bcda9614d4 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -34,6 +34,7 @@ struct CNodeStateStats { uint64_t m_addr_processed = 0; uint64_t m_addr_rate_limited = 0; bool m_addr_relay_enabled{false}; + ServiceFlags their_services; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index cadafcaa8d..103f4f0d7f 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -823,7 +823,7 @@ struct CImportingNow { } }; -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args) +void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path) { SetSyscallSandboxPolicy(SyscallSandboxPolicy::INITIALIZATION_LOAD_BLOCKS); ScheduleBatchPriority(); @@ -893,6 +893,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile return; } } // End scope of CImportingNow - chainman.ActiveChainstate().LoadMempool(args); + chainman.ActiveChainstate().LoadMempool(mempool_path); } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index e017f3f427..9b76371aae 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -211,7 +211,7 @@ bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, c bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); -void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args); +void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 3c085ae6fb..46e0efb9b6 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -524,20 +524,27 @@ public: } bool haveBlockOnDisk(int height) override { - LOCK(cs_main); + LOCK(::cs_main); const CChain& active = Assert(m_node.chainman)->ActiveChain(); CBlockIndex* block = active[height]; return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0; } CBlockLocator getTipLocator() override { - LOCK(cs_main); + LOCK(::cs_main); const CChain& active = Assert(m_node.chainman)->ActiveChain(); return active.GetLocator(); } + CBlockLocator getActiveChainLocator(const uint256& block_hash) override + { + LOCK(::cs_main); + const CBlockIndex* index = chainman().m_blockman.LookupBlockIndex(block_hash); + if (!index) return {}; + return chainman().ActiveChain().GetLocator(index); + } std::optional<int> findLocatorFork(const CBlockLocator& locator) override { - LOCK(cs_main); + LOCK(::cs_main); const CChainState& active = Assert(m_node.chainman)->ActiveChainstate(); if (const CBlockIndex* fork = active.FindForkInGlobalIndex(locator)) { return fork->nHeight; @@ -593,7 +600,7 @@ public: void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); } double guessVerificationProgress(const uint256& block_hash) override { - LOCK(cs_main); + LOCK(::cs_main); return GuessVerificationProgress(chainman().GetParams().TxData(), chainman().m_blockman.LookupBlockIndex(block_hash)); } bool hasBlocks(const uint256& block_hash, int min_height, std::optional<int> max_height) override @@ -693,7 +700,7 @@ public: CFeeRate relayDustFee() override { return ::dustRelayFee; } bool havePruned() override { - LOCK(cs_main); + LOCK(::cs_main); return m_node.chainman->m_blockman.m_have_pruned; } bool isReadyToBroadcast() override { return !node::fImporting && !node::fReindex && !isInitialBlockDownload(); } @@ -768,6 +775,11 @@ public: notifications.transactionAddedToMempool(entry.GetSharedTx(), 0 /* mempool_sequence */); } } + bool hasAssumedValidChain() override + { + return Assert(m_node.chainman)->IsSnapshotActive(); + } + NodeContext& m_node; }; } // namespace diff --git a/src/node/mempool_persist_args.cpp b/src/node/mempool_persist_args.cpp new file mode 100644 index 0000000000..4e775869c6 --- /dev/null +++ b/src/node/mempool_persist_args.cpp @@ -0,0 +1,23 @@ +// Copyright (c) 2022 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 <node/mempool_persist_args.h> + +#include <fs.h> +#include <util/system.h> +#include <validation.h> + +namespace node { + +bool ShouldPersistMempool(const ArgsManager& argsman) +{ + return argsman.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL); +} + +fs::path MempoolPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / "mempool.dat"; +} + +} // namespace node diff --git a/src/node/mempool_persist_args.h b/src/node/mempool_persist_args.h new file mode 100644 index 0000000000..f719ec62ab --- /dev/null +++ b/src/node/mempool_persist_args.h @@ -0,0 +1,25 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H +#define BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H + +#include <fs.h> + +class ArgsManager; + +namespace node { + +/** + * Default for -persistmempool, indicating whether the node should attempt to + * automatically load the mempool on start and save to disk on shutdown + */ +static constexpr bool DEFAULT_PERSIST_MEMPOOL{true}; + +bool ShouldPersistMempool(const ArgsManager& argsman); +fs::path MempoolPath(const ArgsManager& argsman); + +} // namespace node + +#endif // BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 27ee9509e6..bb50b5384a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -370,23 +370,21 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con else if(type == Receive) { // Generate a new address to associate with given label - CTxDestination dest; - if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) - { + auto op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); + if (!op_dest) { WalletModel::UnlockContext ctx(walletModel->requestUnlock()); - if(!ctx.isValid()) - { + if (!ctx.isValid()) { // Unlock wallet failed or was cancelled editStatus = WALLET_UNLOCK_FAILURE; return QString(); } - if(!walletModel->wallet().getNewDestination(address_type, strLabel, dest)) - { + op_dest = walletModel->wallet().getNewDestination(address_type, strLabel); + if (!op_dest) { editStatus = KEY_GENERATION_FAILURE; return QString(); } } - strAddress = EncodeDestination(dest); + strAddress = EncodeDestination(op_dest.GetObj()); } else { diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 27d3a1b9e2..33c60deafb 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -77,8 +77,6 @@ Q_DECLARE_METATYPE(CAmount) Q_DECLARE_METATYPE(SynchronizationState) Q_DECLARE_METATYPE(uint256) -using node::NodeContext; - static void RegisterMetaTypes() { // Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 462b923d61..42c8b07763 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -286,11 +286,19 @@ void OptionsDialog::on_resetButton_clicked() { if (model) { // confirmation dialog + /*: Text explaining that the settings changed will not come into effect + until the client is restarted. */ + QString reset_dialog_text = tr("Client restart required to activate changes.") + "<br><br>"; + /*: Text explaining to the user that the client's current settings + will be backed up at a specific location. %1 is a stand-in + argument for the backup location's path. */ + reset_dialog_text.append(tr("Current settings will be backed up at \"%1\".").arg(m_client_model->dataDir()) + "<br><br>"); + /*: Text asking the user to confirm if they would like to proceed + with a client shutdown. */ + reset_dialog_text.append(tr("Client will be shut down. Do you want to proceed?")); + //: Window title text of pop-up window shown when the user has chosen to reset options. QMessageBox::StandardButton btnRetVal = QMessageBox::question(this, tr("Confirm options reset"), - tr("Client restart required to activate changes.") + "<br><br>" + - tr("Current settings will be backed up at \"%1\".").arg(m_client_model->dataDir()) + "<br><br>" + - tr("Client will be shut down. Do you want to proceed?"), - QMessageBox::Yes | QMessageBox::Cancel, QMessageBox::Cancel); + reset_dialog_text, QMessageBox::Yes | QMessageBox::Cancel, QMessageBox::Cancel); if (btnRetVal == QMessageBox::Cancel) return; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b791fd30c4..70fccdef1c 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1162,7 +1162,6 @@ void RPCConsole::updateDetailWidget() if (!stats->nodeStats.addrLocal.empty()) peerAddrDetails += "<br />" + tr("via %1").arg(QString::fromStdString(stats->nodeStats.addrLocal)); ui->peerHeading->setText(peerAddrDetails); - ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStats.nServices)); QString bip152_hb_settings; if (stats->nodeStats.m_bip152_highbandwidth_to) bip152_hb_settings = ts.to; if (stats->nodeStats.m_bip152_highbandwidth_from) bip152_hb_settings += (bip152_hb_settings.isEmpty() ? ts.from : QLatin1Char('/') + ts.from); @@ -1197,6 +1196,7 @@ void RPCConsole::updateDetailWidget() // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { + ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services)); // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { ui->peerSyncHeight->setText(QString("%1").arg(stats->nodeStateStats.nSyncHeight)); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index de23f51c92..846fa519ee 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -43,8 +43,6 @@ Q_IMPORT_PLUGIN(QAndroidPlatformIntegrationPlugin) #endif #endif -using node::NodeContext; - const std::function<void(const std::string&)> G_TEST_LOG_FUN{}; const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{}; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 7671bfc739..b8ae62ecab 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -173,7 +173,7 @@ void TestGUI(interfaces::Node& node) { WalletRescanReserver reserver(*wallet); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, 0 /* block height */, {} /* max height */, reserver, true /* fUpdate */); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/false); QCOMPARE(result.status, CWallet::ScanResult::SUCCESS); QCOMPARE(result.last_scanned_block, node.context()->chainman->ActiveChain().Tip()->GetBlockHash()); QVERIFY(result.last_failed_block.IsNull()); diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 11140c5da9..01d84624e8 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -391,9 +391,10 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri tr("Restoring Wallet <b>%1</b>…").arg(name.toHtmlEscaped())); QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] { - std::unique_ptr<interfaces::Wallet> wallet = node().walletLoader().restoreWallet(backup_file, wallet_name, m_error_message, m_warning_message); + auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)}; - if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet)); + m_error_message = wallet ? bilingual_str{} : wallet.GetError(); + if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(wallet.ReleaseObj()); QTimer::singleShot(0, this, &RestoreWalletActivity::finish); }); diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 6b38e207d3..8dc97e66a2 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -213,6 +213,14 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard) } std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary}; data.assign(std::istream_iterator<unsigned char>{in}, {}); + + // Some psbt files may be base64 strings in the file rather than binary data + std::string b64_str{data.begin(), data.end()}; + b64_str.erase(b64_str.find_last_not_of(" \t\n\r\f\v") + 1); // Trim trailing whitespace + auto b64_dec = DecodeBase64(b64_str); + if (b64_dec.has_value()) { + data = b64_dec.value(); + } } std::string error; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index ab4d1cae3f..bb6079afee 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -204,10 +204,10 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact { CAmount nFeeRequired = 0; int nChangePosRet = -1; - bilingual_str error; auto& newTx = transaction.getWtx(); - newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error); + const auto& res = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired); + newTx = res ? res.GetObj() : nullptr; transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && newTx) transaction.reassignAmounts(nChangePosRet); @@ -218,7 +218,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact { return SendCoinsReturn(AmountWithFeeExceedsBalance); } - Q_EMIT message(tr("Send Coins"), QString::fromStdString(error.translated), + Q_EMIT message(tr("Send Coins"), QString::fromStdString(res.GetError().translated), CClientUIInterface::MSG_ERROR); return TransactionCreationFailed; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6846e992d4..52d5eaaa50 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -440,6 +440,11 @@ static RPCHelpMan getblockfrompeer() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + RPCTypeCheck(request.params, { + UniValue::VSTR, // blockhash + UniValue::VNUM, // peer_id + }); + const NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); PeerManager& peerman = EnsurePeerman(node); @@ -598,6 +603,29 @@ static CBlockUndo GetUndoChecked(BlockManager& blockman, const CBlockIndex* pblo return blockUndo; } +const RPCResult getblock_vin{ + RPCResult::Type::ARR, "vin", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ELISION, "", "The same output as verbosity = 2"}, + {RPCResult::Type::OBJ, "prevout", "(Only if undo information is available)", + { + {RPCResult::Type::BOOL, "generated", "Coinbase or not"}, + {RPCResult::Type::NUM, "height", "The height of the prevout"}, + {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, + {RPCResult::Type::OBJ, "scriptPubKey", "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, + {RPCResult::Type::STR, "type", "The type (one of: " + GetAllOutputTypes() + ")"}, + }}, + }}, + }}, + } +}; + static RPCHelpMan getblock() { return RPCHelpMan{"getblock", @@ -657,26 +685,7 @@ static RPCHelpMan getblock() { {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::ARR, "vin", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::ELISION, "", "The same output as verbosity = 2"}, - {RPCResult::Type::OBJ, "prevout", "(Only if undo information is available)", - { - {RPCResult::Type::BOOL, "generated", "Coinbase or not"}, - {RPCResult::Type::NUM, "height", "The height of the prevout"}, - {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, - {RPCResult::Type::OBJ, "scriptPubKey", "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR, "hex", "The hex"}, - {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, - {RPCResult::Type::STR, "type", "The type (one of: " + GetAllOutputTypes() + ")"}, - }}, - }}, - }}, - }}, + getblock_vin, }}, }}, }}, @@ -1005,9 +1014,9 @@ static RPCHelpMan gettxout() {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, {RPCResult::Type::OBJ, "scriptPubKey", "", { - {RPCResult::Type::STR, "asm", ""}, + {RPCResult::Type::STR, "asm", "The asm"}, {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, - {RPCResult::Type::STR_HEX, "hex", ""}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, {RPCResult::Type::STR, "type", "The type, eg pubkeyhash"}, {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, }}, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 84d43e7818..d59ff3f75c 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -5,9 +5,12 @@ #include <rpc/blockchain.h> +#include <kernel/mempool_persist.h> + #include <chainparams.h> #include <core_io.h> #include <fs.h> +#include <node/mempool_persist_args.h> #include <policy/rbf.h> #include <policy/settings.h> #include <primitives/transaction.h> @@ -18,7 +21,10 @@ #include <univalue.h> #include <util/moneystr.h> +using kernel::DumpMempool; + using node::DEFAULT_MAX_RAW_TX_FEE_RATE; +using node::MempoolPath; using node::NodeContext; static RPCHelpMan sendrawtransaction() @@ -653,7 +659,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) // Make sure this call is atomic in the pool. LOCK(pool.cs); UniValue ret(UniValue::VOBJ); - ret.pushKV("loaded", pool.IsLoaded()); + ret.pushKV("loaded", pool.GetLoadTried()); ret.pushKV("size", (int64_t)pool.size()); ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize()); ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); @@ -717,16 +723,18 @@ static RPCHelpMan savemempool() const ArgsManager& args{EnsureAnyArgsman(request.context)}; const CTxMemPool& mempool = EnsureAnyMemPool(request.context); - if (!mempool.IsLoaded()) { + if (!mempool.GetLoadTried()) { throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } - if (!DumpMempool(mempool)) { + const fs::path& dump_path = MempoolPath(args); + + if (!DumpMempool(mempool, dump_path)) { throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump mempool to disk"); } UniValue ret(UniValue::VOBJ); - ret.pushKV("filename", fs::path((args.GetDataDirNet() / "mempool.dat")).u8string()); + ret.pushKV("filename", dump_path.u8string()); return ret; }, diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index fad92629c5..27eea824bc 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -195,8 +195,9 @@ static RPCHelpMan getpeerinfo() if (stats.m_mapped_as != 0) { obj.pushKV("mapped_as", uint64_t(stats.m_mapped_as)); } - obj.pushKV("services", strprintf("%016x", stats.nServices)); - obj.pushKV("servicesnames", GetServicesNames(stats.nServices)); + ServiceFlags services{fStateStats ? statestats.their_services : ServiceFlags::NODE_NONE}; + obj.pushKV("services", strprintf("%016x", services)); + obj.pushKV("servicesnames", GetServicesNames(services)); obj.pushKV("lastsend", count_seconds(stats.m_last_send)); obj.pushKV("lastrecv", count_seconds(stats.m_last_recv)); obj.pushKV("last_transaction", count_seconds(stats.m_last_tx_time)); diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index f4bb76f50f..744f809814 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -26,11 +26,6 @@ #include <tuple> #include <vector> -namespace node { -struct NodeContext; -} -using node::NodeContext; - static RPCHelpMan validateaddress() { return RPCHelpMan{ diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 792a1e13b0..16105a85d5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -46,12 +46,10 @@ #include <univalue.h> using node::AnalyzePSBT; -using node::BroadcastTransaction; using node::FindCoins; using node::GetTransaction; using node::NodeContext; using node::PSBTAnalysis; -using node::ReadBlockFromDisk; static void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry, CChainState& active_chainstate) { @@ -680,6 +678,200 @@ static RPCHelpMan signrawtransactionwithkey() }; } +const RPCResult decodepsbt_inputs{ + RPCResult::Type::ARR, "inputs", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::OBJ, "non_witness_utxo", /*optional=*/true, "Decoded network transaction for non-witness UTXOs", + { + {RPCResult::Type::ELISION, "",""}, + }}, + {RPCResult::Type::OBJ, "witness_utxo", /*optional=*/true, "Transaction output for witness UTXOs", + { + {RPCResult::Type::NUM, "amount", "The value in " + CURRENCY_UNIT}, + {RPCResult::Type::OBJ, "scriptPubKey", "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, + }}, + }}, + {RPCResult::Type::OBJ_DYN, "partial_signatures", /*optional=*/true, "", + { + {RPCResult::Type::STR, "pubkey", "The public key and signature that corresponds to it."}, + }}, + {RPCResult::Type::STR, "sighash", /*optional=*/true, "The sighash type to be used"}, + {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + }}, + {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + }}, + {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "pubkey", "The public key with the derivation path as the value."}, + {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, + {RPCResult::Type::STR, "path", "The path"}, + }}, + }}, + {RPCResult::Type::OBJ, "final_scriptSig", /*optional=*/true, "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + }}, + {RPCResult::Type::ARR, "final_scriptwitness", /*optional=*/true, "", + { + {RPCResult::Type::STR_HEX, "", "hex-encoded witness data (if any)"}, + }}, + {RPCResult::Type::OBJ_DYN, "ripemd160_preimages", /*optional=*/ true, "", + { + {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, + }}, + {RPCResult::Type::OBJ_DYN, "sha256_preimages", /*optional=*/ true, "", + { + {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, + }}, + {RPCResult::Type::OBJ_DYN, "hash160_preimages", /*optional=*/ true, "", + { + {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, + }}, + {RPCResult::Type::OBJ_DYN, "hash256_preimages", /*optional=*/ true, "", + { + {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, + }}, + {RPCResult::Type::STR_HEX, "taproot_key_path_sig", /*optional=*/ true, "hex-encoded signature for the Taproot key path spend"}, + {RPCResult::Type::ARR, "taproot_script_path_sigs", /*optional=*/ true, "", + { + {RPCResult::Type::OBJ, "signature", /*optional=*/ true, "The signature for the pubkey and leaf hash combination", + { + {RPCResult::Type::STR, "pubkey", "The x-only pubkey for this signature"}, + {RPCResult::Type::STR, "leaf_hash", "The leaf hash for this signature"}, + {RPCResult::Type::STR, "sig", "The signature itself"}, + }}, + }}, + {RPCResult::Type::ARR, "taproot_scripts", /*optional=*/ true, "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "script", "A leaf script"}, + {RPCResult::Type::NUM, "leaf_ver", "The version number for the leaf script"}, + {RPCResult::Type::ARR, "control_blocks", "The control blocks for this script", + { + {RPCResult::Type::STR_HEX, "control_block", "A hex-encoded control block for this script"}, + }}, + }}, + }}, + {RPCResult::Type::ARR, "taproot_bip32_derivs", /*optional=*/ true, "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "pubkey", "The x-only public key this path corresponds to"}, + {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, + {RPCResult::Type::STR, "path", "The path"}, + {RPCResult::Type::ARR, "leaf_hashes", "The hashes of the leaves this pubkey appears in", + { + {RPCResult::Type::STR_HEX, "hash", "The hash of a leaf this pubkey appears in"}, + }}, + }}, + }}, + {RPCResult::Type::STR_HEX, "taproot_internal_key", /*optional=*/ true, "The hex-encoded Taproot x-only internal key"}, + {RPCResult::Type::STR_HEX, "taproot_merkle_root", /*optional=*/ true, "The hex-encoded Taproot merkle root"}, + {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/ true, "The unknown input fields", + { + {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, + }}, + {RPCResult::Type::ARR, "proprietary", /*optional=*/true, "The input proprietary map", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "identifier", "The hex string for the proprietary identifier"}, + {RPCResult::Type::NUM, "subtype", "The number for the subtype"}, + {RPCResult::Type::STR_HEX, "key", "The hex for the key"}, + {RPCResult::Type::STR_HEX, "value", "The hex for the value"}, + }}, + }}, + }}, + } +}; + +const RPCResult decodepsbt_outputs{ + RPCResult::Type::ARR, "outputs", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + }}, + {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR_HEX, "hex", "The hex"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + }}, + {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "pubkey", "The public key this path corresponds to"}, + {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, + {RPCResult::Type::STR, "path", "The path"}, + }}, + }}, + {RPCResult::Type::STR_HEX, "taproot_internal_key", /*optional=*/ true, "The hex-encoded Taproot x-only internal key"}, + {RPCResult::Type::ARR, "taproot_tree", /*optional=*/ true, "The tuples that make up the Taproot tree, in depth first search order", + { + {RPCResult::Type::OBJ, "tuple", /*optional=*/ true, "A single leaf script in the taproot tree", + { + {RPCResult::Type::NUM, "depth", "The depth of this element in the tree"}, + {RPCResult::Type::NUM, "leaf_ver", "The version of this leaf"}, + {RPCResult::Type::STR, "script", "The hex-encoded script itself"}, + }}, + }}, + {RPCResult::Type::ARR, "taproot_bip32_derivs", /*optional=*/ true, "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "pubkey", "The x-only public key this path corresponds to"}, + {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, + {RPCResult::Type::STR, "path", "The path"}, + {RPCResult::Type::ARR, "leaf_hashes", "The hashes of the leaves this pubkey appears in", + { + {RPCResult::Type::STR_HEX, "hash", "The hash of a leaf this pubkey appears in"}, + }}, + }}, + }}, + {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/true, "The unknown output fields", + { + {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, + }}, + {RPCResult::Type::ARR, "proprietary", /*optional=*/true, "The output proprietary map", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "identifier", "The hex string for the proprietary identifier"}, + {RPCResult::Type::NUM, "subtype", "The number for the subtype"}, + {RPCResult::Type::STR_HEX, "key", "The hex for the key"}, + {RPCResult::Type::STR_HEX, "value", "The hex for the value"}, + }}, + }}, + }}, + } +}; + static RPCHelpMan decodepsbt() { return RPCHelpMan{ @@ -719,194 +911,8 @@ static RPCHelpMan decodepsbt() { {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, }}, - {RPCResult::Type::ARR, "inputs", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::OBJ, "non_witness_utxo", /*optional=*/true, "Decoded network transaction for non-witness UTXOs", - { - {RPCResult::Type::ELISION, "",""}, - }}, - {RPCResult::Type::OBJ, "witness_utxo", /*optional=*/true, "Transaction output for witness UTXOs", - { - {RPCResult::Type::NUM, "amount", "The value in " + CURRENCY_UNIT}, - {RPCResult::Type::OBJ, "scriptPubKey", "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, - {RPCResult::Type::STR_HEX, "hex", "The hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, - }}, - }}, - {RPCResult::Type::OBJ_DYN, "partial_signatures", /*optional=*/true, "", - { - {RPCResult::Type::STR, "pubkey", "The public key and signature that corresponds to it."}, - }}, - {RPCResult::Type::STR, "sighash", /*optional=*/true, "The sighash type to be used"}, - {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR_HEX, "hex", "The hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - }}, - {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR_HEX, "hex", "The hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - }}, - {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR, "pubkey", "The public key with the derivation path as the value."}, - {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, - {RPCResult::Type::STR, "path", "The path"}, - }}, - }}, - {RPCResult::Type::OBJ, "final_scriptSig", /*optional=*/true, "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR, "hex", "The hex"}, - }}, - {RPCResult::Type::ARR, "final_scriptwitness", /*optional=*/true, "", - { - {RPCResult::Type::STR_HEX, "", "hex-encoded witness data (if any)"}, - }}, - {RPCResult::Type::OBJ_DYN, "ripemd160_preimages", /*optional=*/ true, "", - { - {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, - }}, - {RPCResult::Type::OBJ_DYN, "sha256_preimages", /*optional=*/ true, "", - { - {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, - }}, - {RPCResult::Type::OBJ_DYN, "hash160_preimages", /*optional=*/ true, "", - { - {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, - }}, - {RPCResult::Type::OBJ_DYN, "hash256_preimages", /*optional=*/ true, "", - { - {RPCResult::Type::STR, "hash", "The hash and preimage that corresponds to it."}, - }}, - {RPCResult::Type::STR_HEX, "taproot_key_path_sig", /*optional=*/ true, "hex-encoded signature for the Taproot key path spend"}, - {RPCResult::Type::ARR, "taproot_script_path_sigs", /*optional=*/ true, "", - { - {RPCResult::Type::OBJ, "signature", /*optional=*/ true, "The signature for the pubkey and leaf hash combination", - { - {RPCResult::Type::STR, "pubkey", "The x-only pubkey for this signature"}, - {RPCResult::Type::STR, "leaf_hash", "The leaf hash for this signature"}, - {RPCResult::Type::STR, "sig", "The signature itself"}, - }}, - }}, - {RPCResult::Type::ARR, "taproot_scripts", /*optional=*/ true, "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "script", "A leaf script"}, - {RPCResult::Type::NUM, "leaf_ver", "The version number for the leaf script"}, - {RPCResult::Type::ARR, "control_blocks", "The control blocks for this script", - { - {RPCResult::Type::STR_HEX, "control_block", "A hex-encoded control block for this script"}, - }}, - }}, - }}, - {RPCResult::Type::ARR, "taproot_bip32_derivs", /*optional=*/ true, "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR, "pubkey", "The x-only public key this path corresponds to"}, - {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, - {RPCResult::Type::STR, "path", "The path"}, - {RPCResult::Type::ARR, "leaf_hashes", "The hashes of the leaves this pubkey appears in", - { - {RPCResult::Type::STR_HEX, "hash", "The hash of a leaf this pubkey appears in"}, - }}, - }}, - }}, - {RPCResult::Type::STR_HEX, "taproot_internal_key", /*optional=*/ true, "The hex-encoded Taproot x-only internal key"}, - {RPCResult::Type::STR_HEX, "taproot_merkle_root", /*optional=*/ true, "The hex-encoded Taproot merkle root"}, - {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/ true, "The unknown input fields", - { - {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, - }}, - {RPCResult::Type::ARR, "proprietary", /*optional=*/true, "The input proprietary map", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "identifier", "The hex string for the proprietary identifier"}, - {RPCResult::Type::NUM, "subtype", "The number for the subtype"}, - {RPCResult::Type::STR_HEX, "key", "The hex for the key"}, - {RPCResult::Type::STR_HEX, "value", "The hex for the value"}, - }}, - }}, - }}, - }}, - {RPCResult::Type::ARR, "outputs", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::OBJ, "redeem_script", /*optional=*/true, "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR_HEX, "hex", "The hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - }}, - {RPCResult::Type::OBJ, "witness_script", /*optional=*/true, "", - { - {RPCResult::Type::STR, "asm", "The asm"}, - {RPCResult::Type::STR_HEX, "hex", "The hex"}, - {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, - }}, - {RPCResult::Type::ARR, "bip32_derivs", /*optional=*/true, "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR, "pubkey", "The public key this path corresponds to"}, - {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, - {RPCResult::Type::STR, "path", "The path"}, - }}, - }}, - {RPCResult::Type::STR_HEX, "taproot_internal_key", /*optional=*/ true, "The hex-encoded Taproot x-only internal key"}, - {RPCResult::Type::ARR, "taproot_tree", /*optional=*/ true, "The tuples that make up the Taproot tree, in depth first search order", - { - {RPCResult::Type::OBJ, "tuple", /*optional=*/ true, "A single leaf script in the taproot tree", - { - {RPCResult::Type::NUM, "depth", "The depth of this element in the tree"}, - {RPCResult::Type::NUM, "leaf_ver", "The version of this leaf"}, - {RPCResult::Type::STR, "script", "The hex-encoded script itself"}, - }}, - }}, - {RPCResult::Type::ARR, "taproot_bip32_derivs", /*optional=*/ true, "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR, "pubkey", "The x-only public key this path corresponds to"}, - {RPCResult::Type::STR, "master_fingerprint", "The fingerprint of the master key"}, - {RPCResult::Type::STR, "path", "The path"}, - {RPCResult::Type::ARR, "leaf_hashes", "The hashes of the leaves this pubkey appears in", - { - {RPCResult::Type::STR_HEX, "hash", "The hash of a leaf this pubkey appears in"}, - }}, - }}, - }}, - {RPCResult::Type::OBJ_DYN, "unknown", /*optional=*/true, "The unknown output fields", - { - {RPCResult::Type::STR_HEX, "key", "(key-value pair) An unknown key-value pair"}, - }}, - {RPCResult::Type::ARR, "proprietary", /*optional=*/true, "The output proprietary map", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "identifier", "The hex string for the proprietary identifier"}, - {RPCResult::Type::NUM, "subtype", "The number for the subtype"}, - {RPCResult::Type::STR_HEX, "key", "The hex for the key"}, - {RPCResult::Type::STR_HEX, "value", "The hex for the value"}, - }}, - }}, - }}, - }}, + decodepsbt_inputs, + decodepsbt_outputs, {RPCResult::Type::STR_AMOUNT, "fee", /*optional=*/true, "The transaction fee paid if all UTXOs slots in the PSBT have been filled."}, } }, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ca0170c84b..34a4da74f8 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -6,6 +6,7 @@ #include <key_io.h> #include <pubkey.h> +#include <script/miniscript.h> #include <script/script.h> #include <script/standard.h> @@ -161,6 +162,20 @@ public: virtual ~PubkeyProvider() = default; + /** Compare two public keys represented by this provider. + * Used by the Miniscript descriptors to check for duplicate keys in the script. + */ + bool operator<(PubkeyProvider& other) const { + CPubKey a, b; + SigningProvider dummy; + KeyOriginInfo dummy_info; + + GetPubKey(0, dummy, a, dummy_info); + other.GetPubKey(0, dummy, b, dummy_info); + + return a < b; + } + /** Derive a public key. * read_cache is the cache to read keys from (if not nullptr) * write_cache is the cache to write keys to (if not nullptr) @@ -493,12 +508,12 @@ public: /** Base class for all Descriptor implementations. */ class DescriptorImpl : public Descriptor { - //! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size for Multisig). +protected: + //! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size for WSH and Multisig). const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args; //! The string name of the descriptor function. const std::string m_name; -protected: //! The sub-descriptor arguments (empty for everything but SH and WSH). //! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT) //! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions. @@ -563,7 +578,7 @@ public: return true; } - bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const + virtual bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const { std::string extra = ToStringExtra(); size_t pos = extra.size() > 0 ? 1 : 0; @@ -917,6 +932,89 @@ public: bool IsSingleType() const final { return true; } }; +/* We instantiate Miniscript here with a simple integer as key type. + * The value of these key integers are an index in the + * DescriptorImpl::m_pubkey_args vector. + */ + +/** + * The context for converting a Miniscript descriptor into a Script. + */ +class ScriptMaker { + //! Keys contained in the Miniscript (the evaluation of DescriptorImpl::m_pubkey_args). + const std::vector<CPubKey>& m_keys; + +public: + ScriptMaker(const std::vector<CPubKey>& keys LIFETIMEBOUND) : m_keys(keys) {} + + std::vector<unsigned char> ToPKBytes(uint32_t key) const { + return {m_keys[key].begin(), m_keys[key].end()}; + } + + std::vector<unsigned char> ToPKHBytes(uint32_t key) const { + auto id = m_keys[key].GetID(); + return {id.begin(), id.end()}; + } +}; + +/** + * The context for converting a Miniscript descriptor to its textual form. + */ +class StringMaker { + //! To convert private keys for private descriptors. + const SigningProvider* m_arg; + //! Keys contained in the Miniscript (a reference to DescriptorImpl::m_pubkey_args). + const std::vector<std::unique_ptr<PubkeyProvider>>& m_pubkeys; + //! Whether to serialize keys as private or public. + bool m_private; + +public: + StringMaker(const SigningProvider* arg LIFETIMEBOUND, const std::vector<std::unique_ptr<PubkeyProvider>>& pubkeys LIFETIMEBOUND, bool priv) + : m_arg(arg), m_pubkeys(pubkeys), m_private(priv) {} + + std::optional<std::string> ToString(uint32_t key) const + { + std::string ret; + if (m_private) { + if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {}; + } else { + ret = m_pubkeys[key]->ToString(); + } + return ret; + } +}; + +class MiniscriptDescriptor final : public DescriptorImpl +{ +private: + miniscript::NodeRef<uint32_t> m_node; + +protected: + std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript> scripts, + FlatSigningProvider& provider) const override + { + for (const auto& key : keys) provider.pubkeys.emplace(key.GetID(), key); + return Vector(m_node->ToScript(ScriptMaker(keys))); + } + +public: + MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node) + : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {} + + bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, + const DescriptorCache* cache = nullptr) const override + { + if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type == StringType::PRIVATE))) { + out = *res; + return true; + } + return false; + } + + bool IsSolvable() const override { return false; } // For now, mark these descriptors as non-solvable (as we don't have signing logic for them). + bool IsSingleType() const final { return true; } +}; + //////////////////////////////////////////////////////////////////////////// // Parser // //////////////////////////////////////////////////////////////////////////// @@ -1058,6 +1156,94 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider)); } +std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider) +{ + std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false); + KeyOriginInfo info; + if (provider.GetKeyOrigin(pubkey.GetID(), info)) { + return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); + } + return key_provider; +} + +std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseScriptContext ctx, const SigningProvider& provider) +{ + unsigned char full_key[CPubKey::COMPRESSED_SIZE] = {0x02}; + std::copy(xkey.begin(), xkey.end(), full_key + 1); + CPubKey pubkey(full_key); + std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true); + KeyOriginInfo info; + if (provider.GetKeyOriginByXOnly(xkey, info)) { + return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); + } + return key_provider; +} + +/** + * The context for parsing a Miniscript descriptor (either from Script or from its textual representation). + */ +struct KeyParser { + //! The Key type is an index in DescriptorImpl::m_pubkey_args + using Key = uint32_t; + //! Must not be nullptr if parsing from string. + FlatSigningProvider* m_out; + //! Must not be nullptr if parsing from Script. + const SigningProvider* m_in; + //! List of keys contained in the Miniscript. + mutable std::vector<std::unique_ptr<PubkeyProvider>> m_keys; + //! Used to detect key parsing errors within a Miniscript. + mutable std::string m_key_parsing_error; + + KeyParser(FlatSigningProvider* out LIFETIMEBOUND, const SigningProvider* in LIFETIMEBOUND) : m_out(out), m_in(in) {} + + bool KeyCompare(const Key& a, const Key& b) const { + return *m_keys.at(a) < *m_keys.at(b); + } + + template<typename I> std::optional<Key> FromString(I begin, I end) const + { + assert(m_out); + Key key = m_keys.size(); + auto pk = ParsePubkey(key, {&*begin, &*end}, ParseScriptContext::P2WSH, *m_out, m_key_parsing_error); + if (!pk) return {}; + m_keys.push_back(std::move(pk)); + return key; + } + + std::optional<std::string> ToString(const Key& key) const + { + return m_keys.at(key)->ToString(); + } + + template<typename I> std::optional<Key> FromPKBytes(I begin, I end) const + { + assert(m_in); + CPubKey pubkey(begin, end); + if (pubkey.IsValid()) { + Key key = m_keys.size(); + m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2WSH, *m_in)); + return key; + } + return {}; + } + + template<typename I> std::optional<Key> FromPKHBytes(I begin, I end) const + { + assert(end - begin == 20); + assert(m_in); + uint160 hash; + std::copy(begin, end, hash.begin()); + CKeyID keyid(hash); + CPubKey pubkey; + if (m_in->GetPubKey(keyid, pubkey)) { + Key key = m_keys.size(); + m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2WSH, *m_in)); + return key; + } + return {}; + } +}; + /** Parse a script in a particular context. */ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { @@ -1279,6 +1465,45 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const error = "Can only have raw() at top level"; return nullptr; } + // Process miniscript expressions. + { + KeyParser parser(&out, nullptr); + auto node = miniscript::FromString(std::string(expr.begin(), expr.end()), parser); + if (node) { + if (ctx != ParseScriptContext::P2WSH) { + error = "Miniscript expressions can only be used in wsh"; + return nullptr; + } + if (parser.m_key_parsing_error != "") { + error = std::move(parser.m_key_parsing_error); + return nullptr; + } + if (!node->IsSane()) { + // Try to find the first insane sub for better error reporting. + auto insane_node = node.get(); + if (const auto sub = node->FindInsaneSub()) insane_node = sub; + if (const auto str = insane_node->ToString(parser)) error = *str; + if (!insane_node->IsValid()) { + error += " is invalid"; + } else { + error += " is not sane"; + if (!insane_node->IsNonMalleable()) { + error += ": malleable witnesses exist"; + } else if (insane_node == node.get() && !insane_node->NeedsSignature()) { + error += ": witnesses without signature exist"; + } else if (!insane_node->CheckTimeLocksMix()) { + error += ": contains mixes of timelocks expressed in blocks and seconds"; + } else if (!insane_node->CheckDuplicateKey()) { + error += ": contains duplicate public keys"; + } else if (!insane_node->ValidSatisfactions()) { + error += ": needs witnesses that may exceed resource limits"; + } + } + return nullptr; + } + return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node)); + } + } if (ctx == ParseScriptContext::P2SH) { error = "A function is needed within P2SH"; return nullptr; @@ -1290,29 +1515,6 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const return nullptr; } -std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider) -{ - std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false); - KeyOriginInfo info; - if (provider.GetKeyOrigin(pubkey.GetID(), info)) { - return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); - } - return key_provider; -} - -std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseScriptContext ctx, const SigningProvider& provider) -{ - unsigned char full_key[CPubKey::COMPRESSED_SIZE] = {0x02}; - std::copy(xkey.begin(), xkey.end(), full_key + 1); - CPubKey pubkey(full_key); - std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true); - KeyOriginInfo info; - if (provider.GetKeyOriginByXOnly(xkey, info)) { - return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider)); - } - return key_provider; -} - std::unique_ptr<DescriptorImpl> InferMultiA(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider) { auto match = MatchMultiA(script); @@ -1426,6 +1628,14 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo } } + if (ctx == ParseScriptContext::P2WSH) { + KeyParser parser(nullptr, &provider); + auto node = miniscript::FromScript(script, parser); + if (node && node->IsSane()) { + return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node)); + } + } + CTxDestination dest; if (ExtractDestination(script, dest)) { if (GetScriptForDestination(dest) == script) { diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 2c239c2678..f783d1dafc 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -429,6 +429,21 @@ private: )); } + /** Like TreeEval, but without downfn or State type. + * upfn takes (const Node&, Span<Result>) and returns Result. */ + template<typename Result, typename UpFn> + Result TreeEval(UpFn upfn) const + { + struct DummyState {}; + return std::move(*TreeEvalMaybe<Result>(DummyState{}, + [](DummyState, const Node&, size_t) { return DummyState{}; }, + [&upfn](DummyState, const Node& node, Span<Result> subs) { + Result res{upfn(node, subs)}; + return std::optional<Result>(std::move(res)); + } + )); + } + /** Compare two miniscript subtrees, using a non-recursive algorithm. */ friend int Compare(const Node<Key>& node1, const Node<Key>& node2) { @@ -818,6 +833,15 @@ public: //! Return the expression type. Type GetType() const { return typ; } + //! Find an insane subnode which has no insane children. Nullptr if there is none. + const Node* FindInsaneSub() const { + return TreeEval<const Node*>([](const Node& node, Span<const Node*> subs) -> const Node* { + for (auto& sub: subs) if (sub) return sub; + if (!node.IsSaneSubexpression()) return &node; + return nullptr; + }); + } + //! Check whether this node is valid at all. bool IsValid() const { return !(GetType() == ""_mst) && ScriptSize() <= MAX_STANDARD_P2WSH_SCRIPT_SIZE; } @@ -953,7 +977,11 @@ void BuildBack(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>>& construct } } -//! Parse a miniscript from its textual descriptor form. +/** + * Parse a miniscript from its textual descriptor form. + * This does not check whether the script is valid, let alone sane. The caller is expected to use + * the `IsValidTopLevel()` and `IsSaneTopLevel()` to check for these properties on the node. + */ template<typename Key, typename Ctx> inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) { @@ -1255,9 +1283,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) // Sanity checks on the produced miniscript assert(constructed.size() == 1); if (in.size() > 0) return {}; - const NodeRef<Key> tl_node = std::move(constructed.front()); - if (!tl_node->IsValidTopLevel()) return {}; - return tl_node; + return std::move(constructed.front()); } /** Decode a script into opcode/push pairs. diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 50eb479035..1d9a037a66 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -13,9 +13,6 @@ #include <chrono> -using kernel::CCoinsStats; -using kernel::CoinStatsHashType; - BOOST_AUTO_TEST_SUITE(coinstatsindex_tests) static void IndexWaitSynced(BaseIndex& index) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index c87ed82c88..66c4605afa 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -15,6 +15,7 @@ #include <serialize.h> #include <test/util/net.h> #include <test/util/setup_common.h> +#include <timedata.h> #include <util/string.h> #include <util/system.h> #include <util/time.h> @@ -44,17 +45,15 @@ BOOST_FIXTURE_TEST_SUITE(denialofservice_tests, TestingSetup) // work. BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) { - auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + ConnmanTestMsg& connman = static_cast<ConnmanTestMsg&>(*m_node.connman); // Disable inactivity checks for this test to avoid interference - static_cast<ConnmanTestMsg*>(connman.get())->SetPeerConnectTimeout(99999s); - auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, - *m_node.chainman, *m_node.mempool, false); + connman.SetPeerConnectTimeout(99999s); + PeerManager& peerman = *m_node.peerman; // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); NodeId id{0}; CNode dummyNode1{id++, - ServiceFlags(NODE_NETWORK | NODE_WITNESS), /*sock=*/nullptr, addr1, /*nKeyedNetGroupIn=*/0, @@ -63,10 +62,16 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) /*addrNameIn=*/"", ConnectionType::OUTBOUND_FULL_RELAY, /*inbound_onion=*/false}; - dummyNode1.SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode1); - dummyNode1.fSuccessfullyConnected = true; + connman.Handshake( + /*node=*/dummyNode1, + /*successfully_connected=*/true, + /*remote_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*local_services=*/ServiceFlags(NODE_NETWORK | NODE_WITNESS), + /*permission_flags=*/NetPermissionFlags::None, + /*version=*/PROTOCOL_VERSION, + /*relay_txs=*/true); + TestOnlyResetTimeData(); // This test requires that we have a chain with non-zero work. { @@ -78,7 +83,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) // Test starts here { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders } { LOCK(dummyNode1.cs_vSend); @@ -91,7 +96,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) SetMockTime(nStartTime+21*60); { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in getheaders + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders } { LOCK(dummyNode1.cs_vSend); @@ -101,18 +106,17 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) SetMockTime(nStartTime+24*60); { LOCK(dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in disconnect + BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect } BOOST_CHECK(dummyNode1.fDisconnect == true); - peerLogic->FinalizeNode(dummyNode1); + peerman.FinalizeNode(dummyNode1); } static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType) { CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); vNodes.emplace_back(new CNode{id++, - ServiceFlags(NODE_NETWORK | NODE_WITNESS), /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -124,7 +128,7 @@ static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerM CNode &node = *vNodes.back(); node.SetCommonVersion(PROTOCOL_VERSION); - peerLogic.InitializeNode(&node); + peerLogic.InitializeNode(node, ServiceFlags(NODE_NETWORK | NODE_WITNESS)); node.fSuccessfullyConnected = true; connman.AddTestNode(node); @@ -292,7 +296,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) banman->ClearBanned(); NodeId id{0}; nodes[0] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[0], /*nKeyedNetGroupIn=*/0, @@ -302,7 +305,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::INBOUND, /*inbound_onion=*/false}; nodes[0]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[0]); + peerLogic->InitializeNode(*nodes[0], NODE_NETWORK); nodes[0]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[0]); peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged @@ -315,7 +318,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) BOOST_CHECK(!banman->IsDiscouraged(other_addr)); // Different address, not discouraged nodes[1] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[1], /*nKeyedNetGroupIn=*/1, @@ -325,7 +327,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::INBOUND, /*inbound_onion=*/false}; nodes[1]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[1]); + peerLogic->InitializeNode(*nodes[1], NODE_NETWORK); nodes[1]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[1]); peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1); @@ -353,7 +355,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) // Make sure non-IP peers are discouraged and disconnected properly. nodes[2] = new CNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr[2], /*nKeyedNetGroupIn=*/1, @@ -363,7 +364,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) ConnectionType::OUTBOUND_FULL_RELAY, /*inbound_onion=*/false}; nodes[2]->SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(nodes[2]); + peerLogic->InitializeNode(*nodes[2], NODE_NETWORK); nodes[2]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[2]); peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD); @@ -398,7 +399,6 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) CAddress addr(ip(0xa0b0c001), NODE_NONE); NodeId id{0}; CNode dummyNode{id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/4, @@ -408,7 +408,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) ConnectionType::INBOUND, /*inbound_onion=*/false}; dummyNode.SetCommonVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&dummyNode); + peerLogic->InitializeNode(dummyNode, NODE_NETWORK); dummyNode.fSuccessfullyConnected = true; peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 63c86a896d..a8c666079d 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -141,14 +141,13 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& } else { parse_priv = Parse(prv, keys_priv, error); } + BOOST_CHECK_MESSAGE(parse_priv, error); if (replace_apostrophe_with_h_in_pub) { parse_pub = Parse(UseHInsteadOfApostrophe(pub), keys_pub, error); } else { parse_pub = Parse(pub, keys_pub, error); } - - BOOST_CHECK(parse_priv); - BOOST_CHECK(parse_pub); + BOOST_CHECK_MESSAGE(parse_pub, error); // Check that the correct OutputType is inferred BOOST_CHECK(parse_priv->GetOutputType() == type); @@ -161,8 +160,8 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& // Check that both versions serialize back to the public version. std::string pub1 = parse_priv->ToString(); std::string pub2 = parse_pub->ToString(); - BOOST_CHECK(EqualDescriptor(pub, pub1)); - BOOST_CHECK(EqualDescriptor(pub, pub2)); + BOOST_CHECK_MESSAGE(EqualDescriptor(pub, pub1), "Private ser: " + pub1 + " Public desc: " + pub); + BOOST_CHECK_MESSAGE(EqualDescriptor(pub, pub2), "Public ser: " + pub2 + " Public desc: " + pub); // Check that both can be serialized with private key back to the private version, but not without private key. if (!(flags & MISSING_PRIVKEYS)) { @@ -486,6 +485,29 @@ Check("sh(wsh(multi(20,KzoAz5CanayRKex3fSLQ2BwJpN7U52gZvxMyk78nDMHuqrUxuSJy,KwGN } nonminimalmultisig << std::vector<unsigned char>{4} << OP_CHECKMULTISIG; CheckInferRaw(nonminimalmultisig); + + // Miniscript tests + + // Invalid checksum + CheckUnparsable("wsh(and_v(vc:andor(pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))#abcdef12", "wsh(and_v(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))#abcdef12", "Provided checksum 'abcdef12' does not match computed checksum 'tyzp6a7p'"); + // Only p2wsh context is valid + CheckUnparsable("sh(and_v(vc:andor(pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))", "sh(and_v(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))", "Miniscript expressions can only be used in wsh"); + CheckUnparsable("tr(and_v(vc:andor(pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))", "tr(and_v(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))", "tr(): key 'and_v(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10))' is not valid"); + CheckUnparsable("raw(and_v(vc:andor(pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))", "sh(and_v(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))", "Miniscript expressions can only be used in wsh"); + CheckUnparsable("", "tr(034D2224bbbbbbbbbbcbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb40,{{{{{{{{{{{{{{{{{{{{{{multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/967808'/9,xprvA1RpRA33e1JQ7ifknakTFNpgXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/968/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/3/4/5/58/55/2/5/58/58/2/5/5/5/8/5/2/8/5/85/2/8/2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/8/5/8/5/4/5/585/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/8/2/5/8/5/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/58/58/2/0/8/5/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/5/8/5/8/24/5/58/52/5/8/5/2/8/24/5/58/588/246/8/5/2/8/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/5/4/5/58/55/58/2/5/8/55/2/5/8/58/555/58/2/5/8/4//2/5/58/5w/2/5/8/5/2/4/5/58/5558'/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/8/2/5/8/5/5/8/58/2/5/58/58/2/5/8/9/588/2/58/2/5/8/5/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/82/5/8/5/5/58/52/6/8/5/2/8/{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{}{{{{{{{{{DDD2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8588/246/8/5/2DLDDDDDDDbbD3DDDD/8/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/3/4/5/58/55/2/5/58/58/2/5/5/5/8/5/2/8/5/85/2/8/2/5/8D)/5/2/5/58/58/2/5/58/58/58/588/2/58/2/5/8/5/25/58/58/2/5/58/58/2/5/8/9/588/2/58/2/6780,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFW/8/5/2/5/58678008')", "'multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/967808'/9,xprvA1RpRA33e1JQ7ifknakTFNpgXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/968/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/3/4/5/58/55/2/5/58/58/2/5/5/5/8/5/2/8/5/85/2/8/2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/8/5/8/5/4/5/585/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/8/2/5/8/5/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/58/58/2/0/8/5/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/5/8/5/8/24/5/58/52/5/8/5/2/8/24/5/58/588/246/8/5/2/8/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/5/4/5/58/55/58/2/5/8/55/2/5/8/58/555/58/2/5/8/4//2/5/58/5w/2/5/8/5/2/4/5/58/5558'/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/8/2/5/8/5/5/8/58/2/5/58/58/2/5/8/9/588/2/58/2/5/8/5/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8/5/2/5/58/58/2/5/5/58/588/2/58/2/5/8/5/2/82/5/8/5/5/58/52/6/8/5/2/8/{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{}{{{{{{{{{DDD2/5/8/5/2/5/58/58/2/5/58/58/588/2/58/2/8/5/8/5/4/5/58/588/2/6/8/5/2/8/2/5/8588/246/8/5/2DLDDDDDDDbbD3DDDD/8/2/5/8/5/2/5/58/58/2/5/5/5/58/588/2/6/8/5/2/8/2/5/8/2/58/2/5/8/5/2/8/5/8/3/4/5/58/55/2/5/58/58/2/5/5/5/8/5/2/8/5/85/2/8/2/5/8D)/5/2/5/58/58/2/5/58/58/58/588/2/58/2/5/8/5/25/58/58/2/5/58/58/2/5/8/9/588/2/58/2/6780,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFW/8/5/2/5/58678008'' is not a valid descriptor function"); + // Insane at top level + CheckUnparsable("wsh(and_b(vc:andor(pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))", "wsh(and_b(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))", "and_b(vc:andor(pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)) is invalid"); + // Invalid sub + CheckUnparsable("wsh(and_v(vc:andor(v:pk_k(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),pk_k(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn),and_v(v:older(1),pk_k(L4o2kDvXXDRH2VS9uBnouScLduWt4dZnM25se7kvEjJeQ285en2A))),after(10)))", "wsh(and_v(vc:andor(v:pk_k(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),pk_k(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0),and_v(v:older(1),pk_k(02aa27e5eb2c185e87cd1dbc3e0efc9cb1175235e0259df1713424941c3cb40402))),after(10)))", "v:pk_k(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204) is invalid"); + // Insane subs + CheckUnparsable("wsh(or_i(older(1),pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd)))", "wsh(or_i(older(1),pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "or_i(older(1),pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)) is not sane: witnesses without signature exist"); + CheckUnparsable("wsh(or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "wsh(or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)) is not sane: malleable witnesses exist"); + CheckUnparsable("wsh(and_b(and_b(older(1),a:older(100000000)),s:pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd)))", "wsh(and_b(and_b(older(1),a:older(100000000)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "and_b(older(1),a:older(100000000)) is not sane: contains mixes of timelocks expressed in blocks and seconds"); + CheckUnparsable("wsh(and_b(or_b(pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),s:pk(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn)),s:pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd)))", "wsh(and_b(or_b(pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "and_b(or_b(pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)) is not sane: contains duplicate public keys"); + // Valid with extended keys. + Check("wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0)))", "wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)))", "wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0)))", "wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)))", UNSOLVABLE, {{"0020acf425291b98a1d7e0d4690139442abc289175be32ef1f75945e339924246d73"}}, OutputType::BECH32, {{},{0}}); + // Valid under sh(wsh()) and with a mix of xpubs and raw keys + Check("sh(wsh(thresh(1,pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),a:and_n(multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0),n:older(2)))))", "sh(wsh(thresh(1,pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),a:and_n(multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0),n:older(2)))))", "sh(wsh(thresh(1,pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),a:and_n(multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0),n:older(2)))))", "sh(wsh(thresh(1,pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),a:and_n(multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0),n:older(2)))))", UNSOLVABLE | MIXED_PUBKEYS, {{"a914767e9119ff3b3ac0cb6dcfe21de1842ccf85f1c487"}}, OutputType::P2SH_SEGWIT, {{},{0}}); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/mempool_utils.h b/src/test/fuzz/mempool_utils.h new file mode 100644 index 0000000000..bfe12e30ba --- /dev/null +++ b/src/test/fuzz/mempool_utils.h @@ -0,0 +1,19 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H +#define BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H + +#include <validation.h> + +class DummyChainState final : public CChainState +{ +public: + void SetMempool(CTxMemPool* mempool) + { + m_mempool = mempool; + } +}; + +#endif // BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 4981287152..741810f6a2 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -68,7 +68,6 @@ FUZZ_TARGET_INIT(net, initialize_net) (void)node.GetAddrLocal(); (void)node.GetId(); (void)node.GetLocalNonce(); - (void)node.GetLocalServices(); const int ref_count = node.GetRefCount(); assert(ref_count >= 0); (void)node.GetCommonVersion(); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 1763cd8af3..272c9e6cdc 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -80,8 +80,7 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release(); connman.AddTestNode(p2p_node); - g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); + FillNode(fuzzed_data_provider, connman, p2p_node); const auto mock_time = ConsumeTime(fuzzed_data_provider); SetMockTime(mock_time); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index e1c11e1afd..12e682416c 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -46,8 +46,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release()); CNode& p2p_node = *peers.back(); - g_setup->m_node.peerman->InitializeNode(&p2p_node); - FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node); + FillNode(fuzzed_data_provider, connman, p2p_node); connman.AddTestNode(p2p_node); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 2d88ee295b..63fbf0516a 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -8,6 +8,7 @@ #include <node/miner.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> #include <test/util/mining.h> #include <test/util/script.h> @@ -34,15 +35,6 @@ struct MockedTxPool : public CTxMemPool { } }; -class DummyChainState final : public CChainState -{ -public: - void SetMempool(CTxMemPool* mempool) - { - m_mempool = mempool; - } -}; - void initialize_tx_pool() { static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index d318baa6a2..b18d783259 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -3,8 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <consensus/amount.h> -#include <net.h> +#include <consensus/validation.h> #include <net_processing.h> +#include <node/eviction.h> +#include <policy/policy.h> #include <primitives/transaction.h> #include <script/script.h> #include <sync.h> @@ -99,16 +101,20 @@ FUZZ_TARGET_INIT(txorphan, initialize_orphanage) [&] { bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); // AddTx should return false if tx is too big or already have it + // tx weight is unknown, we only check when tx is already in orphanage { LOCK(g_cs_orphans); - Assert(have_tx != orphanage.AddTx(tx, peer_id)); + bool add_tx = orphanage.AddTx(tx, peer_id); + // have_tx == true -> add_tx == false + Assert(!have_tx || !add_tx); } have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); - // tx should already be added since it will not be too big in the test - // have_tx should be true and AddTx should fail { LOCK(g_cs_orphans); - Assert(have_tx && !orphanage.AddTx(tx, peer_id)); + bool add_tx = orphanage.AddTx(tx, peer_id); + // if have_tx is still false, it must be too big + Assert(!have_tx == GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT); + Assert(!have_tx || !add_tx); } }, [&] { diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 4b893c648e..fabcea22c3 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -289,57 +289,15 @@ bool FuzzedSock::IsConnected(std::string& errmsg) const return false; } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept { - const bool successfully_connected{fuzzed_data_provider.ConsumeBool()}; - const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); - const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); - const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()); - const bool relay_txs{fuzzed_data_provider.ConsumeBool()}; - - const CNetMsgMaker mm{0}; - - CSerializedNetMsg msg_version{ - mm.Make(NetMsgType::VERSION, - version, // - Using<CustomUintFormatter<8>>(remote_services), // - int64_t{}, // dummy time - int64_t{}, // ignored service bits - CService{}, // dummy - int64_t{}, // ignored service bits - CService{}, // ignored - uint64_t{1}, // dummy nonce - std::string{}, // dummy subver - int32_t{}, // dummy starting_height - relay_txs), - }; - - (void)connman.ReceiveMsgFrom(node, msg_version); - node.fPauseSend = false; - connman.ProcessMessagesOnce(node); - { - LOCK(node.cs_sendProcessing); - peerman.SendMessages(&node); - } - if (node.fDisconnect) return; - assert(node.nVersion == version); - assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); - assert(node.nServices == remote_services); - CNodeStateStats statestats; - assert(peerman.GetNodeStateStats(node.GetId(), statestats)); - assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); - node.m_permissionFlags = permission_flags; - if (successfully_connected) { - CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; - (void)connman.ReceiveMsgFrom(node, msg_verack); - node.fPauseSend = false; - connman.ProcessMessagesOnce(node); - { - LOCK(node.cs_sendProcessing); - peerman.SendMessages(&node); - } - assert(node.fSuccessfullyConnected == true); - } + connman.Handshake(node, + /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(), + /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), + /*local_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), + /*permission_flags=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS), + /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()), + /*relay_txs=*/fuzzed_data_provider.ConsumeBool()); } CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional<CAmount>& max) noexcept diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 4b89ad9bdc..406e11c573 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -296,7 +296,6 @@ template <bool ReturnUniquePtr = false> auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = std::nullopt) noexcept { const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegralInRange<NodeId>(0, std::numeric_limits<NodeId>::max())); - const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const auto sock = std::make_shared<FuzzedSock>(fuzzed_data_provider); const CAddress address = ConsumeAddress(fuzzed_data_provider); const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>(); @@ -307,7 +306,6 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false}; if constexpr (ReturnUniquePtr) { return std::make_unique<CNode>(node_id, - local_services, sock, address, keyed_net_group, @@ -318,7 +316,6 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N inbound_onion); } else { return CNode{node_id, - local_services, sock, address, keyed_net_group, @@ -331,7 +328,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N } inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept; +void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept; class FuzzedFileProvider { diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 9532610f8d..90c1a71d9f 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -2,10 +2,14 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <kernel/mempool_persist.h> + #include <chainparamsbase.h> #include <mempool_args.h> +#include <node/mempool_persist_args.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/mempool_utils.h> #include <test/fuzz/util.h> #include <test/util/setup_common.h> #include <txmempool.h> @@ -15,6 +19,10 @@ #include <cstdint> #include <vector> +using kernel::DumpMempool; + +using node::MempoolPath; + namespace { const TestingSetup* g_setup; } // namespace @@ -33,9 +41,12 @@ FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)}; + auto& chainstate{static_cast<DummyChainState&>(g_setup->m_node.chainman->ActiveChainstate())}; + chainstate.SetMempool(&pool); + auto fuzzed_fopen = [&](const fs::path&, const char*) { return fuzzed_file_provider.open(); }; - (void)LoadMempool(pool, g_setup->m_node.chainman->ActiveChainstate(), fuzzed_fopen); - (void)DumpMempool(pool, fuzzed_fopen, true); + (void)chainstate.LoadMempool(MempoolPath(g_setup->m_args), fuzzed_fopen); + (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 3877fea907..95e8476b77 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -111,11 +111,17 @@ struct KeyConverter { assert(it != g_testdata->pkmap.end()); return it->second; } + + std::optional<std::string> ToString(const Key& key) const { + return HexStr(ToPKBytes(key)); + } }; //! Singleton instance of KeyConverter. const KeyConverter CONVERTER{}; +// https://github.com/llvm/llvm-project/issues/53444 +// NOLINTNEXTLINE(misc-unused-using-decls) using miniscript::operator"" _mst; enum TestMode : int { @@ -276,7 +282,7 @@ BOOST_AUTO_TEST_CASE(fixed_tests) // (for now) have 'd:' be 'u'. This tests we can't use a 'd:' wrapper for a thresh, which requires // its subs to all be 'u' (taken from https://github.com/rust-bitcoin/rust-miniscript/discussions/341). const auto ms_minimalif = miniscript::FromString("thresh(3,c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),sc:pk_k(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),sc:pk_k(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798),sdv:older(32))", CONVERTER); - BOOST_CHECK(!ms_minimalif); + BOOST_CHECK(ms_minimalif && !ms_minimalif->IsValid()); // A Miniscript with duplicate keys is not sane const auto ms_dup1 = miniscript::FromString("and_v(v:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65))", CONVERTER); BOOST_CHECK(ms_dup1); @@ -290,6 +296,15 @@ BOOST_AUTO_TEST_CASE(fixed_tests) // Same when the duplicates are on different levels in the tree const auto ms_dup4 = miniscript::FromString("thresh(2,pkh(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),a:and_b(dv:older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER); BOOST_CHECK(ms_dup4 && !ms_dup4->IsSane() && !ms_dup4->CheckDuplicateKey()); + // Test we find the first insane sub closer to be a leaf node. This fragment is insane for two reasons: + // 1. It can be spent without a signature + // 2. It contains timelock mixes + // We'll report the timelock mix error, as it's "deeper" (closer to be a leaf node) than the "no 's' property" + // error is. + const auto ms_ins = miniscript::FromString("or_i(and_b(after(1),a:after(1000000000)),pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204))", CONVERTER); + BOOST_CHECK(ms_ins && ms_ins->IsValid() && !ms_ins->IsSane()); + const auto insane_sub = ms_ins->FindInsaneSub(); + BOOST_CHECK(insane_sub && *insane_sub->ToString(CONVERTER) == "and_b(after(1),a:after(1000000000))"); // Timelock tests Test("after(100)", "?", TESTMODE_VALID | TESTMODE_NONMAL); // only heightlock diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 115c4b9b24..f2eaa0179f 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -58,7 +58,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) std::string pszDest; std::unique_ptr<CNode> pnode1 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -77,7 +76,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode1->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode2 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/1, @@ -96,7 +94,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode2->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode3 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -115,7 +112,6 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) BOOST_CHECK_EQUAL(pnode3->ConnectedThroughNetwork(), Network::NET_IPV4); std::unique_ptr<CNode> pnode4 = std::make_unique<CNode>(id++, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/1, @@ -629,7 +625,6 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) ipv4AddrPeer.s_addr = 0xa0b0c001; CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK); std::unique_ptr<CNode> pnode = std::make_unique<CNode>(/*id=*/0, - NODE_NETWORK, /*sock=*/nullptr, addr, /*nKeyedNetGroupIn=*/0, @@ -648,7 +643,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test) pnode->SetAddrLocal(addrLocal); // before patch, this causes undefined behavior detectable with clang's -fsanitize=memory - GetLocalAddrForPeer(&*pnode); + GetLocalAddrForPeer(*pnode); // suppress no-checks-run warning; if this test fails, it's by triggering a sanitizer BOOST_CHECK(1); @@ -675,19 +670,15 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) const uint16_t bind_port = 20001; m_node.args->ForceSetArg("-bind", strprintf("3.4.5.6:%u", bind_port)); - const uint32_t current_time = static_cast<uint32_t>(GetAdjustedTime()); - SetMockTime(current_time); - // Our address:port as seen from the peer, completely different from the above. in_addr peer_us_addr; peer_us_addr.s_addr = htonl(0x02030405); - const CAddress peer_us{CService{peer_us_addr, 20002}, NODE_NETWORK, current_time}; + const CService peer_us{peer_us_addr, 20002}; // Create a peer with a routable IPv4 address (outbound). in_addr peer_out_in_addr; peer_out_in_addr.s_addr = htonl(0x01020304); CNode peer_out{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_out_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -700,16 +691,15 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) peer_out.SetAddrLocal(peer_us); // Without the fix peer_us:8333 is chosen instead of the proper peer_us:bind_port. - auto chosen_local_addr = GetLocalAddrForPeer(&peer_out); + auto chosen_local_addr = GetLocalAddrForPeer(peer_out); BOOST_REQUIRE(chosen_local_addr); - const CAddress expected{CService{peer_us_addr, bind_port}, NODE_NETWORK, current_time}; + const CService expected{peer_us_addr, bind_port}; BOOST_CHECK(*chosen_local_addr == expected); // Create a peer with a routable IPv4 address (inbound). in_addr peer_in_in_addr; peer_in_in_addr.s_addr = htonl(0x05060708); CNode peer_in{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_in_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -722,7 +712,7 @@ BOOST_AUTO_TEST_CASE(get_local_addr_for_peer_port) peer_in.SetAddrLocal(peer_us); // Without the fix peer_us:8333 is chosen instead of the proper peer_us:peer_us.GetPort(). - chosen_local_addr = GetLocalAddrForPeer(&peer_in); + chosen_local_addr = GetLocalAddrForPeer(peer_in); BOOST_REQUIRE(chosen_local_addr); BOOST_CHECK(*chosen_local_addr == peer_us); @@ -837,7 +827,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) in_addr peer_in_addr; peer_in_addr.s_addr = htonl(0x01020304); CNode peer{/*id=*/0, - /*nLocalServicesIn=*/NODE_NETWORK, /*sock=*/nullptr, /*addrIn=*/CAddress{CService{peer_in_addr, 8333}, NODE_NETWORK}, /*nKeyedNetGroupIn=*/0, @@ -857,7 +846,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) *static_cast<TestChainState*>(&m_node.chainman->ActiveChainstate()); chainstate.JumpOutOfIbd(); - m_node.peerman->InitializeNode(&peer); + m_node.peerman->InitializeNode(peer, NODE_NETWORK); std::atomic<bool> interrupt_dummy{false}; std::chrono::microseconds time_received_dummy{0}; diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index 071193b550..223db16c6c 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -7,10 +7,69 @@ #include <chainparams.h> #include <node/eviction.h> #include <net.h> +#include <net_processing.h> +#include <netmessagemaker.h> #include <span.h> #include <vector> +void ConnmanTestMsg::Handshake(CNode& node, + bool successfully_connected, + ServiceFlags remote_services, + ServiceFlags local_services, + NetPermissionFlags permission_flags, + int32_t version, + bool relay_txs) +{ + auto& peerman{static_cast<PeerManager&>(*m_msgproc)}; + auto& connman{*this}; + const CNetMsgMaker mm{0}; + + peerman.InitializeNode(node, local_services); + + CSerializedNetMsg msg_version{ + mm.Make(NetMsgType::VERSION, + version, // + Using<CustomUintFormatter<8>>(remote_services), // + int64_t{}, // dummy time + int64_t{}, // ignored service bits + CService{}, // dummy + int64_t{}, // ignored service bits + CService{}, // ignored + uint64_t{1}, // dummy nonce + std::string{}, // dummy subver + int32_t{}, // dummy starting_height + relay_txs), + }; + + (void)connman.ReceiveMsgFrom(node, msg_version); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); + } + if (node.fDisconnect) return; + assert(node.nVersion == version); + assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); + CNodeStateStats statestats; + assert(peerman.GetNodeStateStats(node.GetId(), statestats)); + assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); + assert(statestats.their_services == remote_services); + node.m_permissionFlags = permission_flags; + if (successfully_connected) { + CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; + (void)connman.ReceiveMsgFrom(node, msg_verack); + node.fPauseSend = false; + connman.ProcessMessagesOnce(node); + { + LOCK(node.cs_sendProcessing); + peerman.SendMessages(&node); + } + assert(node.fSuccessfullyConnected == true); + } +} + void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const { assert(node.ReceiveMsgBytes(msg_bytes, complete)); diff --git a/src/test/util/net.h b/src/test/util/net.h index 34ab9958c6..7f61a03d27 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -39,6 +39,14 @@ struct ConnmanTestMsg : public CConnman { m_nodes.clear(); } + void Handshake(CNode& node, + bool successfully_connected, + ServiceFlags remote_services, + ServiceFlags local_services, + NetPermissionFlags permission_flags, + int32_t version, + bool relay_txs); + void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); } void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 0c9e880d67..0fba9258f1 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -36,7 +36,6 @@ #include <timedata.h> #include <txdb.h> #include <txmempool.h> -#include <util/designator.h> #include <util/strencodings.h> #include <util/string.h> #include <util/thread.h> @@ -158,10 +157,10 @@ BasicTestingSetup::~BasicTestingSetup() CTxMemPool::Options MemPoolOptionsForTest(const NodeContext& node) { CTxMemPool::Options mempool_opts{ - Desig(estimator) node.fee_estimator.get(), + .estimator = node.fee_estimator.get(), // Default to always checking mempool regardless of // chainparams.DefaultConsistencyChecks for tests - Desig(check_ratio) 1, + .check_ratio = 1, }; ApplyArgsManOptions(*node.args, mempool_opts); return mempool_opts; @@ -184,8 +183,8 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve m_cache_sizes = CalculateCacheSizes(m_args); const ChainstateManager::Options chainman_opts{ - chainparams, - GetAdjustedTime, + .chainparams = chainparams, + .adjusted_time_callback = GetAdjustedTime, }; m_node.chainman = std::make_unique<ChainstateManager>(chainman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(m_cache_sizes.block_tree_db, true); diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index 52aaeabccf..7a00ac9e1f 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -20,11 +20,10 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; - CTxDestination dest; - bilingual_str error; - if (!w.GetNewDestination(output_type, "", dest, error)) assert(false); + auto op_dest = w.GetNewDestination(output_type, ""); + assert(op_dest.HasRes()); - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); } #endif // ENABLE_WALLET diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 102de74389..78adb521ac 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -24,8 +24,8 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup) BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) { const ChainstateManager::Options chainman_opts{ - Params(), - GetAdjustedTime, + .chainparams = Params(), + .adjusted_time_callback = GetAdjustedTime, }; ChainstateManager manager{chainman_opts}; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index aeaa10034e..7eff6bdbe3 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1210,14 +1210,14 @@ void CTxMemPool::GetTransactionAncestry(const uint256& txid, size_t& ancestors, } } -bool CTxMemPool::IsLoaded() const +bool CTxMemPool::GetLoadTried() const { LOCK(cs); - return m_is_loaded; + return m_load_tried; } -void CTxMemPool::SetIsLoaded(bool loaded) +void CTxMemPool::SetLoadTried(bool load_tried) { LOCK(cs); - m_is_loaded = loaded; + m_load_tried = load_tried; } diff --git a/src/txmempool.h b/src/txmempool.h index 6e37f59f2e..d7d308038c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -451,7 +451,7 @@ protected: void trackPackageRemoved(const CFeeRate& rate) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool m_is_loaded GUARDED_BY(cs){false}; + bool m_load_tried GUARDED_BY(cs){false}; CFeeRate GetMinFee(size_t sizelimit) const; @@ -728,11 +728,17 @@ public: */ void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const; - /** @returns true if the mempool is fully loaded */ - bool IsLoaded() const; + /** + * @returns true if we've made an attempt to load the mempool regardless of + * whether the attempt was successful or not + */ + bool GetLoadTried() const; - /** Sets the current loaded state */ - void SetIsLoaded(bool loaded); + /** + * Set whether or not we've made an attempt to load the mempool (regardless + * of whether the attempt was successful or not) + */ + void SetLoadTried(bool load_tried); unsigned long size() const { diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index 22be0311e8..5cb8268472 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -82,12 +82,14 @@ public: bool isArray() const { return (typ == VARR); } bool isObject() const { return (typ == VOBJ); } - bool push_back(const UniValue& val); - bool push_backV(const std::vector<UniValue>& vec); + void push_back(const UniValue& val); + void push_backV(const std::vector<UniValue>& vec); + template <class It> + void push_backV(It first, It last); void __pushKV(const std::string& key, const UniValue& val); - bool pushKV(const std::string& key, const UniValue& val); - bool pushKVs(const UniValue& obj); + void pushKV(const std::string& key, const UniValue& val); + void pushKVs(const UniValue& obj); std::string write(unsigned int prettyIndent = 0, unsigned int indentLevel = 0) const; @@ -137,6 +139,13 @@ public: friend const UniValue& find_value( const UniValue& obj, const std::string& name); }; +template <class It> +void UniValue::push_backV(It first, It last) +{ + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; + values.insert(values.end(), first, last); +} + enum jtokentype { JTOK_ERR = -1, JTOK_NONE = 0, // eof diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index 3553995c28..ac6f31a5a9 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -108,53 +108,45 @@ bool UniValue::setObject() return true; } -bool UniValue::push_back(const UniValue& val_) +void UniValue::push_back(const UniValue& val_) { - if (typ != VARR) - return false; + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; values.push_back(val_); - return true; } -bool UniValue::push_backV(const std::vector<UniValue>& vec) +void UniValue::push_backV(const std::vector<UniValue>& vec) { - if (typ != VARR) - return false; + if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"}; values.insert(values.end(), vec.begin(), vec.end()); - - return true; } void UniValue::__pushKV(const std::string& key, const UniValue& val_) { + if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; + keys.push_back(key); values.push_back(val_); } -bool UniValue::pushKV(const std::string& key, const UniValue& val_) +void UniValue::pushKV(const std::string& key, const UniValue& val_) { - if (typ != VOBJ) - return false; + if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; size_t idx; if (findKey(key, idx)) values[idx] = val_; else __pushKV(key, val_); - return true; } -bool UniValue::pushKVs(const UniValue& obj) +void UniValue::pushKVs(const UniValue& obj) { - if (typ != VOBJ || obj.typ != VOBJ) - return false; + if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"}; for (size_t i = 0; i < obj.keys.size(); i++) __pushKV(obj.keys[i], obj.values.at(i)); - - return true; } void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const diff --git a/src/univalue/sources.mk b/src/univalue/sources.mk index e156216378..5e4d9c3831 100644 --- a/src/univalue/sources.mk +++ b/src/univalue/sources.mk @@ -1,12 +1,8 @@ -# - All variables are namespaced with UNIVALUE_ to avoid colliding with -# downstream makefiles. # - All Variables ending in _HEADERS or _SOURCES confuse automake, so the # _INT postfix is applied. # - Convenience variables, for example a UNIVALUE_TEST_DIR should not be used # as they interfere with automatic dependency generation -# - The %reldir% is the relative path from the Makefile.am. This allows -# downstreams to use these variables without having to manually account for -# the path change. +# - The %reldir% is the relative path from the Makefile.am. UNIVALUE_INCLUDE_DIR_INT = %reldir%/include @@ -29,9 +25,6 @@ UNIVALUE_TEST_UNITESTER_INT += %reldir%/test/unitester.cpp UNIVALUE_TEST_JSON_INT = UNIVALUE_TEST_JSON_INT += %reldir%/test/test_json.cpp -UNIVALUE_TEST_NO_NUL_INT = -UNIVALUE_TEST_NO_NUL_INT += %reldir%/test/no_nul.cpp - UNIVALUE_TEST_OBJECT_INT = UNIVALUE_TEST_OBJECT_INT += %reldir%/test/object.cpp diff --git a/src/univalue/test/.gitignore b/src/univalue/test/.gitignore index 7b27cf0da2..5812c96b14 100644 --- a/src/univalue/test/.gitignore +++ b/src/univalue/test/.gitignore @@ -2,7 +2,6 @@ object unitester test_json -no_nul *.trs *.log diff --git a/src/univalue/test/no_nul.cpp b/src/univalue/test/no_nul.cpp deleted file mode 100644 index 3a7a727abb..0000000000 --- a/src/univalue/test/no_nul.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include <univalue.h> - -int main (int argc, char *argv[]) -{ - char buf[] = "___[1,2,3]___"; - UniValue val; - return val.read(buf + 3, 7) ? 0 : 1; -} diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp index 8a35bf914d..cf8c29ec67 100644 --- a/src/univalue/test/object.cpp +++ b/src/univalue/test/object.cpp @@ -13,9 +13,6 @@ #include <string> #include <vector> -#define BOOST_FIXTURE_TEST_SUITE(a, b) -#define BOOST_AUTO_TEST_CASE(funcName) void funcName() -#define BOOST_AUTO_TEST_SUITE_END() #define BOOST_CHECK(expr) assert(expr) #define BOOST_CHECK_EQUAL(v1, v2) assert((v1) == (v2)) #define BOOST_CHECK_THROW(stmt, excMatch) { \ @@ -35,9 +32,7 @@ } \ } -BOOST_FIXTURE_TEST_SUITE(univalue_tests, BasicTestingSetup) - -BOOST_AUTO_TEST_CASE(univalue_constructor) +void univalue_constructor() { UniValue v1; BOOST_CHECK(v1.isNull()); @@ -85,7 +80,17 @@ BOOST_AUTO_TEST_CASE(univalue_constructor) BOOST_CHECK_EQUAL(v9.getValStr(), "zappa"); } -BOOST_AUTO_TEST_CASE(univalue_typecheck) +void univalue_push_throw() +{ + UniValue j; + BOOST_CHECK_THROW(j.push_back(1), std::runtime_error); + BOOST_CHECK_THROW(j.push_backV({1}), std::runtime_error); + BOOST_CHECK_THROW(j.__pushKV("k", 1), std::runtime_error); + BOOST_CHECK_THROW(j.pushKV("k", 1), std::runtime_error); + BOOST_CHECK_THROW(j.pushKVs({}), std::runtime_error); +} + +void univalue_typecheck() { UniValue v1; BOOST_CHECK(v1.setNumStr("1")); @@ -134,7 +139,7 @@ BOOST_AUTO_TEST_CASE(univalue_typecheck) BOOST_CHECK_THROW(vals[1].get_bool(), std::runtime_error); } -BOOST_AUTO_TEST_CASE(univalue_set) +void univalue_set() { UniValue v(UniValue::VSTR, "foo"); v.clear(); @@ -193,18 +198,18 @@ BOOST_AUTO_TEST_CASE(univalue_set) BOOST_CHECK(v.isNull()); } -BOOST_AUTO_TEST_CASE(univalue_array) +void univalue_array() { UniValue arr(UniValue::VARR); UniValue v((int64_t)1023LL); - BOOST_CHECK(arr.push_back(v)); + arr.push_back(v); std::string vStr("zippy"); - BOOST_CHECK(arr.push_back(vStr)); + arr.push_back(vStr); const char *s = "pippy"; - BOOST_CHECK(arr.push_back(s)); + arr.push_back(s); std::vector<UniValue> vec; v.setStr("boing"); @@ -213,13 +218,13 @@ BOOST_AUTO_TEST_CASE(univalue_array) v.setStr("going"); vec.push_back(v); - BOOST_CHECK(arr.push_backV(vec)); + arr.push_backV(vec); - BOOST_CHECK(arr.push_back((uint64_t) 400ULL)); - BOOST_CHECK(arr.push_back((int64_t) -400LL)); - BOOST_CHECK(arr.push_back((int) -401)); - BOOST_CHECK(arr.push_back(-40.1)); - BOOST_CHECK(arr.push_back(true)); + arr.push_back(uint64_t{400ULL}); + arr.push_back(int64_t{-400LL}); + arr.push_back(int{-401}); + arr.push_back(-40.1); + arr.push_back(true); BOOST_CHECK_EQUAL(arr.empty(), false); BOOST_CHECK_EQUAL(arr.size(), 10); @@ -252,7 +257,7 @@ BOOST_AUTO_TEST_CASE(univalue_array) BOOST_CHECK_EQUAL(arr.size(), 0); } -BOOST_AUTO_TEST_CASE(univalue_object) +void univalue_object() { UniValue obj(UniValue::VOBJ); std::string strKey, strVal; @@ -260,39 +265,39 @@ BOOST_AUTO_TEST_CASE(univalue_object) strKey = "age"; v.setInt(100); - BOOST_CHECK(obj.pushKV(strKey, v)); + obj.pushKV(strKey, v); strKey = "first"; strVal = "John"; - BOOST_CHECK(obj.pushKV(strKey, strVal)); + obj.pushKV(strKey, strVal); strKey = "last"; - const char *cVal = "Smith"; - BOOST_CHECK(obj.pushKV(strKey, cVal)); + const char* cVal = "Smith"; + obj.pushKV(strKey, cVal); strKey = "distance"; - BOOST_CHECK(obj.pushKV(strKey, (int64_t) 25)); + obj.pushKV(strKey, int64_t{25}); strKey = "time"; - BOOST_CHECK(obj.pushKV(strKey, (uint64_t) 3600)); + obj.pushKV(strKey, uint64_t{3600}); strKey = "calories"; - BOOST_CHECK(obj.pushKV(strKey, (int) 12)); + obj.pushKV(strKey, int{12}); strKey = "temperature"; - BOOST_CHECK(obj.pushKV(strKey, (double) 90.012)); + obj.pushKV(strKey, double{90.012}); strKey = "moon"; - BOOST_CHECK(obj.pushKV(strKey, true)); + obj.pushKV(strKey, true); strKey = "spoon"; - BOOST_CHECK(obj.pushKV(strKey, false)); + obj.pushKV(strKey, false); UniValue obj2(UniValue::VOBJ); - BOOST_CHECK(obj2.pushKV("cat1", 9000)); - BOOST_CHECK(obj2.pushKV("cat2", 12345)); + obj2.pushKV("cat1", 9000); + obj2.pushKV("cat2", 12345); - BOOST_CHECK(obj.pushKVs(obj2)); + obj.pushKVs(obj2); BOOST_CHECK_EQUAL(obj.empty(), false); BOOST_CHECK_EQUAL(obj.size(), 11); @@ -371,7 +376,7 @@ BOOST_AUTO_TEST_CASE(univalue_object) static const char *json1 = "[1.10000000,{\"key1\":\"str\\u0000\",\"key2\":800,\"key3\":{\"name\":\"martian http://test.com\"}}]"; -BOOST_AUTO_TEST_CASE(univalue_readwrite) +void univalue_readwrite() { UniValue v; BOOST_CHECK(v.read(json1)); @@ -414,11 +419,10 @@ BOOST_AUTO_TEST_CASE(univalue_readwrite) BOOST_CHECK(!v.read("{} 42")); } -BOOST_AUTO_TEST_SUITE_END() - int main (int argc, char *argv[]) { univalue_constructor(); + univalue_push_throw(); univalue_typecheck(); univalue_set(); univalue_array(); diff --git a/src/univalue/test/unitester.cpp b/src/univalue/test/unitester.cpp index 94c149b39f..6344a5a0ab 100644 --- a/src/univalue/test/unitester.cpp +++ b/src/univalue/test/unitester.cpp @@ -149,6 +149,13 @@ void unescape_unicode_test() assert(val[0].get_str() == "\xf0\x9d\x85\xa1"); } +void no_nul_test() +{ + char buf[] = "___[1,2,3]___"; + UniValue val; + assert(val.read(buf + 3, 7)); +} + int main (int argc, char *argv[]) { for (const auto& f: filenames) { @@ -156,6 +163,7 @@ int main (int argc, char *argv[]) } unescape_unicode_test(); + no_nul_test(); return 0; } diff --git a/src/util/asmap.cpp b/src/util/asmap.cpp index ceb8379c1c..b1db3b5f0d 100644 --- a/src/util/asmap.cpp +++ b/src/util/asmap.cpp @@ -8,10 +8,13 @@ #include <crypto/common.h> #include <fs.h> #include <logging.h> +#include <serialize.h> #include <streams.h> +#include <algorithm> #include <cassert> -#include <map> +#include <cstdio> +#include <utility> #include <vector> namespace { diff --git a/src/util/bip32.cpp b/src/util/bip32.cpp index 4c7e948368..39e43eeb31 100644 --- a/src/util/bip32.cpp +++ b/src/util/bip32.cpp @@ -2,12 +2,15 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <sstream> -#include <stdio.h> #include <tinyformat.h> #include <util/bip32.h> #include <util/strencodings.h> +#include <algorithm> +#include <cstdint> +#include <cstdio> +#include <sstream> + bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath) { diff --git a/src/util/bip32.h b/src/util/bip32.h index aa4eac3791..0872bc88de 100644 --- a/src/util/bip32.h +++ b/src/util/bip32.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_UTIL_BIP32_H #define BITCOIN_UTIL_BIP32_H +#include <cstdint> #include <string> #include <vector> diff --git a/src/util/bytevectorhash.cpp b/src/util/bytevectorhash.cpp index 9054db4759..6d777613e6 100644 --- a/src/util/bytevectorhash.cpp +++ b/src/util/bytevectorhash.cpp @@ -6,6 +6,8 @@ #include <random.h> #include <util/bytevectorhash.h> +#include <vector> + ByteVectorHash::ByteVectorHash() : m_k0(GetRand<uint64_t>()), m_k1(GetRand<uint64_t>()) diff --git a/src/util/bytevectorhash.h b/src/util/bytevectorhash.h index b88c17460b..c2322b8daf 100644 --- a/src/util/bytevectorhash.h +++ b/src/util/bytevectorhash.h @@ -5,7 +5,8 @@ #ifndef BITCOIN_UTIL_BYTEVECTORHASH_H #define BITCOIN_UTIL_BYTEVECTORHASH_H -#include <stdint.h> +#include <cstdint> +#include <cstddef> #include <vector> /** diff --git a/src/util/designator.h b/src/util/designator.h deleted file mode 100644 index 3670b11e00..0000000000 --- a/src/util/designator.h +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) 2022 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_UTIL_DESIGNATOR_H -#define BITCOIN_UTIL_DESIGNATOR_H - -/** - * Designated initializers can be used to avoid ordering mishaps in aggregate - * initialization. However, they do not prevent uninitialized members. The - * checks can be disabled by defining DISABLE_DESIGNATED_INITIALIZER_ERRORS. - * This should only be needed on MSVC 2019. MSVC 2022 supports them with the - * option "/std:c++20" - */ -#ifndef DISABLE_DESIGNATED_INITIALIZER_ERRORS -#define Desig(field_name) .field_name = -#else -#define Desig(field_name) -#endif - -#endif // BITCOIN_UTIL_DESIGNATOR_H diff --git a/src/util/error.cpp b/src/util/error.cpp index 22a5964279..33a35a6d59 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -5,9 +5,11 @@ #include <util/error.h> #include <tinyformat.h> -#include <util/system.h> #include <util/translation.h> +#include <cassert> +#include <string> + bilingual_str TransactionErrorString(const TransactionError err) { switch (err) { diff --git a/src/util/hasher.cpp b/src/util/hasher.cpp index c21941eb88..a80f20c894 100644 --- a/src/util/hasher.cpp +++ b/src/util/hasher.cpp @@ -2,11 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <crypto/siphash.h> #include <random.h> +#include <span.h> #include <util/hasher.h> -#include <limits> - SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {} SaltedOutpointHasher::SaltedOutpointHasher() : k0(GetRand<uint64_t>()), k1(GetRand<uint64_t>()) {} diff --git a/src/util/hasher.h b/src/util/hasher.h index 3d24a4d23c..426b8990e6 100644 --- a/src/util/hasher.h +++ b/src/util/hasher.h @@ -5,10 +5,16 @@ #ifndef BITCOIN_UTIL_HASHER_H #define BITCOIN_UTIL_HASHER_H +#include <crypto/common.h> #include <crypto/siphash.h> #include <primitives/transaction.h> #include <uint256.h> +#include <cstdint> +#include <cstring> + +template <typename C> class Span; + class SaltedTxidHasher { private: diff --git a/src/util/message.cpp b/src/util/message.cpp index f58876f915..d395c4b0bc 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -3,16 +3,20 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <hash.h> // For CHashWriter -#include <key.h> // For CKey -#include <key_io.h> // For DecodeDestination() -#include <pubkey.h> // For CPubKey -#include <script/standard.h> // For CTxDestination, IsValidDestination(), PKHash -#include <serialize.h> // For SER_GETHASH +#include <hash.h> +#include <key.h> +#include <key_io.h> +#include <pubkey.h> +#include <script/standard.h> +#include <serialize.h> +#include <uint256.h> #include <util/message.h> -#include <util/strencodings.h> // For DecodeBase64() +#include <util/strencodings.h> +#include <cassert> +#include <optional> #include <string> +#include <variant> #include <vector> /** diff --git a/src/util/message.h b/src/util/message.h index b31c5f5761..1b7febe60a 100644 --- a/src/util/message.h +++ b/src/util/message.h @@ -6,11 +6,12 @@ #ifndef BITCOIN_UTIL_MESSAGE_H #define BITCOIN_UTIL_MESSAGE_H -#include <key.h> // For CKey #include <uint256.h> #include <string> +class CKey; + extern const std::string MESSAGE_MAGIC; /** The result of a signed message verification. diff --git a/src/util/moneystr.cpp b/src/util/moneystr.cpp index 8c4bc6e6f4..d9e6cef600 100644 --- a/src/util/moneystr.cpp +++ b/src/util/moneystr.cpp @@ -10,6 +10,7 @@ #include <util/strencodings.h> #include <util/string.h> +#include <cstdint> #include <optional> std::string FormatMoney(const CAmount n) diff --git a/src/util/readwritefile.cpp b/src/util/readwritefile.cpp index 628e6a3980..3ec08119e7 100644 --- a/src/util/readwritefile.cpp +++ b/src/util/readwritefile.cpp @@ -5,8 +5,9 @@ #include <fs.h> +#include <algorithm> +#include <cstdio> #include <limits> -#include <stdio.h> #include <string> #include <utility> diff --git a/src/util/result.h b/src/util/result.h new file mode 100644 index 0000000000..2f586a4c9b --- /dev/null +++ b/src/util/result.h @@ -0,0 +1,49 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_RESULT_H +#define BITCOIN_UTIL_RESULT_H + +#include <util/translation.h> + +#include <variant> + +/* + * 'BResult' is a generic class useful for wrapping a return object + * (in case of success) or propagating the error cause. +*/ +template<class T> +class BResult { +private: + std::variant<bilingual_str, T> m_variant; + +public: + BResult() : m_variant{Untranslated("")} {} + BResult(T obj) : m_variant{std::move(obj)} {} + BResult(bilingual_str error) : m_variant{std::move(error)} {} + + /* Whether the function succeeded or not */ + bool HasRes() const { return std::holds_alternative<T>(m_variant); } + + /* In case of success, the result object */ + const T& GetObj() const { + assert(HasRes()); + return std::get<T>(m_variant); + } + T ReleaseObj() + { + assert(HasRes()); + return std::move(std::get<T>(m_variant)); + } + + /* In case of failure, the error cause */ + const bilingual_str& GetError() const { + assert(!HasRes()); + return std::get<bilingual_str>(m_variant); + } + + explicit operator bool() const { return HasRes(); } +}; + +#endif // BITCOIN_UTIL_RESULT_H diff --git a/src/util/serfloat.h b/src/util/serfloat.h index 4d912b0176..343ccb9d3a 100644 --- a/src/util/serfloat.h +++ b/src/util/serfloat.h @@ -5,7 +5,7 @@ #ifndef BITCOIN_UTIL_SERFLOAT_H #define BITCOIN_UTIL_SERFLOAT_H -#include <stdint.h> +#include <cstdint> /* Encode a double using the IEEE 754 binary64 format. All NaNs are encoded as x86/ARM's * positive quiet NaN with payload 0. */ diff --git a/src/util/spanparsing.cpp b/src/util/spanparsing.cpp index 8614bd1176..565c867e18 100644 --- a/src/util/spanparsing.cpp +++ b/src/util/spanparsing.cpp @@ -6,8 +6,9 @@ #include <span.h> +#include <algorithm> +#include <cstddef> #include <string> -#include <vector> namespace spanparsing { diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 675fe7d2d7..303e19beec 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -3,17 +3,18 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <span.h> #include <util/strencodings.h> -#include <util/string.h> - -#include <tinyformat.h> #include <algorithm> #include <array> -#include <cstdlib> +#include <cassert> #include <cstring> #include <limits> #include <optional> +#include <ostream> +#include <string> +#include <vector> static const std::string CHARS_ALPHA_NUM = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 9a96bbe67b..14867b21b2 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -13,11 +13,14 @@ #include <util/string.h> #include <charconv> +#include <cstddef> #include <cstdint> -#include <iterator> #include <limits> #include <optional> #include <string> +#include <string_view> +#include <system_error> +#include <type_traits> #include <vector> /** Used by SanitizeString() */ diff --git a/src/util/string.cpp b/src/util/string.cpp index d05222e8b8..dff782c330 100644 --- a/src/util/string.cpp +++ b/src/util/string.cpp @@ -6,6 +6,8 @@ #include <boost/algorithm/string/replace.hpp> +#include <string> + void ReplaceAll(std::string& in_out, std::string_view search, std::string_view substitute) { boost::replace_all(in_out, search, substitute); diff --git a/src/util/syserror.cpp b/src/util/syserror.cpp index 391ddd3560..5270f55366 100644 --- a/src/util/syserror.cpp +++ b/src/util/syserror.cpp @@ -10,6 +10,7 @@ #include <util/syserror.h> #include <cstring> +#include <string> std::string SysErrorString(int err) { diff --git a/src/util/thread.cpp b/src/util/thread.cpp index 14be668685..f9f427ba20 100644 --- a/src/util/thread.cpp +++ b/src/util/thread.cpp @@ -9,6 +9,7 @@ #include <util/threadnames.h> #include <exception> +#include <functional> void util::TraceThread(const char* thread_name, std::function<void()> thread_func) { diff --git a/src/util/time.cpp b/src/util/time.cpp index 7d9d6bcff1..2cafc55c69 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -8,16 +8,19 @@ #endif #include <compat.h> +#include <tinyformat.h> #include <util/time.h> - #include <util/check.h> -#include <atomic> #include <boost/date_time/posix_time/posix_time.hpp> + +#include <atomic> +#include <chrono> #include <ctime> +#include <locale> #include <thread> - -#include <tinyformat.h> +#include <sstream> +#include <string> void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } diff --git a/src/util/time.h b/src/util/time.h index 0f87d66c2e..fc49f23ce3 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -9,7 +9,7 @@ #include <compat.h> #include <chrono> -#include <stdint.h> +#include <cstdint> #include <string> using namespace std::chrono_literals; @@ -24,6 +24,7 @@ struct NodeClock : public std::chrono::system_clock { }; using NodeSeconds = std::chrono::time_point<NodeClock, std::chrono::seconds>; +using SteadyClock = std::chrono::steady_clock; using SteadySeconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::seconds>; using SteadyMilliseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::milliseconds>; using SteadyMicroseconds = std::chrono::time_point<std::chrono::steady_clock, std::chrono::microseconds>; diff --git a/src/util/translation.h b/src/util/translation.h index aee601d9c1..07b7f43c8a 100644 --- a/src/util/translation.h +++ b/src/util/translation.h @@ -6,7 +6,9 @@ #define BITCOIN_UTIL_TRANSLATION_H #include <tinyformat.h> + #include <functional> +#include <string> /** * Bilingual messages: diff --git a/src/util/url.cpp b/src/util/url.cpp index e42d93bce8..ea9323e666 100644 --- a/src/util/url.cpp +++ b/src/util/url.cpp @@ -5,7 +5,8 @@ #include <util/url.h> #include <event2/http.h> -#include <stdlib.h> + +#include <cstdlib> #include <string> std::string urlDecode(const std::string &urlEncoded) { diff --git a/src/util/vector.h b/src/util/vector.h index dab65ded2a..ed745affe5 100644 --- a/src/util/vector.h +++ b/src/util/vector.h @@ -7,6 +7,7 @@ #include <initializer_list> #include <type_traits> +#include <utility> #include <vector> /** Construct a vector with the specified elements. diff --git a/src/validation.cpp b/src/validation.cpp index 4c694a2c21..c46eb36e5c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5,6 +5,9 @@ #include <validation.h> +#include <kernel/coinstats.h> +#include <kernel/mempool_persist.h> + #include <arith_uint256.h> #include <chain.h> #include <chainparams.h> @@ -17,8 +20,8 @@ #include <consensus/validation.h> #include <cuckoocache.h> #include <flatfile.h> +#include <fs.h> #include <hash.h> -#include <kernel/coinstats.h> #include <logging.h> #include <logging/timer.h> #include <node/blockstorage.h> @@ -47,12 +50,14 @@ #include <util/rbf.h> #include <util/strencodings.h> #include <util/system.h> +#include <util/time.h> #include <util/trace.h> #include <util/translation.h> #include <validationinterface.h> #include <warnings.h> #include <algorithm> +#include <chrono> #include <deque> #include <numeric> #include <optional> @@ -61,8 +66,9 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; using kernel::ComputeUTXOStats; +using kernel::LoadMempool; -using node::BLOCKFILE_CHUNK_SIZE; +using fsbridge::FopenFn; using node::BlockManager; using node::BlockMap; using node::CBlockIndexHeightOnlyComparator; @@ -70,11 +76,8 @@ using node::CBlockIndexWorkComparator; using node::fImporting; using node::fPruneMode; using node::fReindex; -using node::nPruneTarget; -using node::OpenBlockFile; using node::ReadBlockFromDisk; using node::SnapshotMetadata; -using node::UNDOFILE_CHUNK_SIZE; using node::UndoReadFromDisk; using node::UnlinkPrunedFiles; @@ -3861,13 +3864,11 @@ void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeigh } } -void CChainState::LoadMempool(const ArgsManager& args) +void CChainState::LoadMempool(const fs::path& load_path, FopenFn mockable_fopen_function) { if (!m_mempool) return; - if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - ::LoadMempool(*m_mempool, *this); - } - m_mempool->SetIsLoaded(!ShutdownRequested()); + ::LoadMempool(*m_mempool, load_path, *this, mockable_fopen_function); + m_mempool->SetLoadTried(!ShutdownRequested()); } bool CChainState::LoadChainTip() @@ -4638,153 +4639,6 @@ bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) return ret; } -static const uint64_t MEMPOOL_DUMP_VERSION = 1; - -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) -{ - int64_t nExpiryTimeout = std::chrono::seconds{pool.m_expiry}.count(); - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")}; - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - if (file.IsNull()) { - LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); - return false; - } - - int64_t count = 0; - int64_t expired = 0; - int64_t failed = 0; - int64_t already_there = 0; - int64_t unbroadcast = 0; - int64_t nNow = GetTime(); - - try { - uint64_t version; - file >> version; - if (version != MEMPOOL_DUMP_VERSION) { - return false; - } - uint64_t num; - file >> num; - while (num) { - --num; - CTransactionRef tx; - int64_t nTime; - int64_t nFeeDelta; - file >> tx; - file >> nTime; - file >> nFeeDelta; - - CAmount amountdelta = nFeeDelta; - if (amountdelta) { - pool.PrioritiseTransaction(tx->GetHash(), amountdelta); - } - if (nTime > nNow - nExpiryTimeout) { - LOCK(cs_main); - const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); - if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { - ++count; - } else { - // mempool may contain the transaction already, e.g. from - // wallet(s) having loaded it while we were processing - // mempool transactions; consider these as valid, instead of - // failed, but mark them as 'already there' - if (pool.exists(GenTxid::Txid(tx->GetHash()))) { - ++already_there; - } else { - ++failed; - } - } - } else { - ++expired; - } - if (ShutdownRequested()) - return false; - } - std::map<uint256, CAmount> mapDeltas; - file >> mapDeltas; - - for (const auto& i : mapDeltas) { - pool.PrioritiseTransaction(i.first, i.second); - } - - std::set<uint256> unbroadcast_txids; - file >> unbroadcast_txids; - unbroadcast = unbroadcast_txids.size(); - for (const auto& txid : unbroadcast_txids) { - // Ensure transactions were accepted to mempool then add to - // unbroadcast set. - if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); - } - } catch (const std::exception& e) { - LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); - return false; - } - - LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); - return true; -} - -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit) -{ - int64_t start = GetTimeMicros(); - - std::map<uint256, CAmount> mapDeltas; - std::vector<TxMempoolInfo> vinfo; - std::set<uint256> unbroadcast_txids; - - static Mutex dump_mutex; - LOCK(dump_mutex); - - { - LOCK(pool.cs); - for (const auto &i : pool.mapDeltas) { - mapDeltas[i.first] = i.second; - } - vinfo = pool.infoAll(); - unbroadcast_txids = pool.GetUnbroadcastTxs(); - } - - int64_t mid = GetTimeMicros(); - - try { - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat.new", "wb")}; - if (!filestr) { - return false; - } - - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - - uint64_t version = MEMPOOL_DUMP_VERSION; - file << version; - - file << (uint64_t)vinfo.size(); - for (const auto& i : vinfo) { - file << *(i.tx); - file << int64_t{count_seconds(i.m_time)}; - file << int64_t{i.nFeeDelta}; - mapDeltas.erase(i.tx->GetHash()); - } - - file << mapDeltas; - - LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); - file << unbroadcast_txids; - - if (!skip_file_commit && !FileCommit(file.Get())) - throw std::runtime_error("FileCommit failed"); - file.fclose(); - if (!RenameOver(gArgs.GetDataDirNet() / "mempool.dat.new", gArgs.GetDataDirNet() / "mempool.dat")) { - throw std::runtime_error("Rename failed"); - } - int64_t last = GetTimeMicros(); - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*MICRO, (last-mid)*MICRO); - } catch (const std::exception& e) { - LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); - return false; - } - return true; -} - //! Guess how far we are in the verification process at the given block index //! require cs_main if pindex has not been validated yet (because nChainTx might be unset) double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pindex) { diff --git a/src/validation.h b/src/validation.h index 0e27e117fa..a21d9e3a28 100644 --- a/src/validation.h +++ b/src/validation.h @@ -68,8 +68,6 @@ static const bool DEFAULT_CHECKPOINTS_ENABLED = true; static const bool DEFAULT_TXINDEX = false; static constexpr bool DEFAULT_COINSTATSINDEX{false}; static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; -/** Default for -persistmempool */ -static const bool DEFAULT_PERSIST_MEMPOOL = true; /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ @@ -679,7 +677,7 @@ public: void CheckBlockIndex(); /** Load the persisted mempool from disk */ - void LoadMempool(const ArgsManager& args); + void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */ bool LoadChainTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -834,7 +832,7 @@ private: friend CChainState; public: - using Options = ChainstateManagerOpts; + using Options = kernel::ChainstateManagerOpts; explicit ChainstateManager(const Options& opts) : m_chainparams{opts.chainparams}, @@ -1014,14 +1012,6 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) return DeploymentEnabled(chainman.GetConsensus(), dep); } -using FopenFn = std::function<FILE*(const fs::path&, const char*)>; - -/** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); - -/** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function = fsbridge::fopen); - /** * Return the expected assumeutxo value for a given height, if one exists. * diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 5e70ed4a30..43fdcee48d 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -219,19 +219,18 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo new_coin_control.m_min_depth = 1; constexpr int RANDOM_CHANGE_POSITION = -1; - bilingual_str fail_reason; - FeeCalculation fee_calc_out; - std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, fail_reason, new_coin_control, fee_calc_out, false); - if (!txr) { - errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + fail_reason); + auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false); + if (!res) { + errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + res.GetError()); return Result::WALLET_ERROR; } + const auto& txr = res.GetObj(); // Write back new fee if successful - new_fee = txr->fee; + new_fee = txr.fee; // Write back transaction - mtx = CMutableTransaction(*txr->tx); + mtx = CMutableTransaction(*txr.tx); return Result::OK; } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 327276f2ed..23f91d9b3a 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -146,11 +146,10 @@ public: void abortRescan() override { m_wallet->AbortRescan(); } bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } std::string getWalletName() override { return m_wallet->GetName(); } - bool getNewDestination(const OutputType type, const std::string label, CTxDestination& dest) override + BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override { LOCK(m_wallet->cs_wallet); - bilingual_str error; - return m_wallet->GetNewDestination(type, label, dest, error); + return m_wallet->GetNewDestination(type, label); } bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) override { @@ -250,22 +249,21 @@ public: LOCK(m_wallet->cs_wallet); return m_wallet->ListLockedCoins(outputs); } - CTransactionRef createTransaction(const std::vector<CRecipient>& recipients, + BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, - CAmount& fee, - bilingual_str& fail_reason) override + CAmount& fee) override { LOCK(m_wallet->cs_wallet); - FeeCalculation fee_calc_out; - std::optional<CreatedTransactionResult> txr = CreateTransaction(*m_wallet, recipients, change_pos, - fail_reason, coin_control, fee_calc_out, sign); - if (!txr) return {}; - fee = txr->fee; - change_pos = txr->change_pos; + const auto& res = CreateTransaction(*m_wallet, recipients, change_pos, + coin_control, sign); + if (!res) return res.GetError(); + const auto& txr = res.GetObj(); + fee = txr.fee; + change_pos = txr.change_pos; - return txr->tx; + return txr.tx; } void commitTransaction(CTransactionRef tx, WalletValueMap value_map, @@ -571,11 +569,13 @@ public: options.require_existing = true; return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); } - std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) override + BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override { DatabaseStatus status; - - return MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings)); + bilingual_str error; + BResult<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; + if (!wallet) return error; + return wallet; } std::string getWalletDir() override { diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 8de4017371..d3303c0b1f 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -130,12 +130,10 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const ism return 0; CAmount credit = 0; - if (filter & ISMINE_SPENDABLE) { + const isminefilter get_amount_filter{filter & ISMINE_ALL}; + if (get_amount_filter) { // GetBalance can assume transactions in mapWallet won't change - credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, ISMINE_SPENDABLE); - } - if (filter & ISMINE_WATCH_ONLY) { - credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, ISMINE_WATCH_ONLY); + credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, get_amount_filter); } return credit; } @@ -146,11 +144,9 @@ CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const ismi return 0; CAmount debit = 0; - if (filter & ISMINE_SPENDABLE) { - debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, ISMINE_SPENDABLE); - } - if (filter & ISMINE_WATCH_ONLY) { - debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, ISMINE_WATCH_ONLY); + const isminefilter get_amount_filter{filter & ISMINE_ALL}; + if (get_amount_filter) { + debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, get_amount_filter); } return debit; } diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index da4cc44ee6..9428d049de 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -58,13 +58,12 @@ RPCHelpMan getnewaddress() output_type = parsed.value(); } - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewDestination(output_type, label, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewDestination(output_type, label); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } @@ -106,12 +105,11 @@ RPCHelpMan getrawchangeaddress() output_type = parsed.value(); } - CTxDestination dest; - bilingual_str error; - if (!pwallet->GetNewChangeDestination(output_type, dest, error)) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error.original); + auto op_dest = pwallet->GetNewChangeDestination(output_type); + if (!op_dest) { + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); } - return EncodeDestination(dest); + return EncodeDestination(op_dest.GetObj()); }, }; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 5f1673fb12..62aba4f3f2 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -100,11 +100,13 @@ RPCHelpMan importprivkey() "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" "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key (see dumpprivkey)"}, {"label", RPCArg::Type::STR, RPCArg::DefaultHint{"current label if address exists, otherwise \"\""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -201,6 +203,8 @@ 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" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "Hint: use importmulti to import more than one address.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" @@ -210,7 +214,7 @@ RPCHelpMan importaddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, {"p2sh", RPCArg::Type::BOOL, RPCArg::Default{false}, "Add the P2SH version of the script as well"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, @@ -401,11 +405,13 @@ RPCHelpMan importpubkey() "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" "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"pubkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The hex-encoded public key"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -484,7 +490,7 @@ 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", + "Note: Blockchain and Mempool will be rescanned after a successful import. Use \"getwalletinfo\" to query the scanning progress.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet file"}, }, @@ -1250,6 +1256,8 @@ RPCHelpMan importmulti() "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" "\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 keys, addresses or scripts exist but related transactions are still missing.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"requests", RPCArg::Type::ARR, RPCArg::Optional::NO, "Data to be imported", @@ -1291,7 +1299,7 @@ RPCHelpMan importmulti() "\"requests\""}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Stating if should rescan the blockchain after all imports"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, "\"options\""}, }, @@ -1593,7 +1601,7 @@ RPCHelpMan importdescriptors() " Use the string \"now\" to substitute the current synced blockchain time.\n" " \"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n" " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n" - " of all descriptors being imported will be scanned.", + "of all descriptors being imported will be scanned as well as the mempool.", /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"} }, {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 0e8cee5db8..83e23497cb 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -156,18 +156,16 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto // Send constexpr int RANDOM_CHANGE_POSITION = -1; - bilingual_str error; - FeeCalculation fee_calc_out; - std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc_out, true); - if (!txr) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); + auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true); + if (!res) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original); } - CTransactionRef tx = txr->tx; + const CTransactionRef& tx = res.GetObj().tx; wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); entry.pushKV("txid", tx->GetHash().GetHex()); - entry.pushKV("fee_reason", StringForFeeReason(fee_calc_out.reason)); + entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason)); return entry; } return tx->GetHash().GetHex(); @@ -1635,7 +1633,7 @@ RPCHelpMan walletcreatefundedpsbt() }, true ); - UniValue options = request.params[3]; + UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]}; CAmount fee; int change_position; diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 6e42c0736d..0b52dcc001 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -310,11 +310,12 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param wtx The wallet transaction. * @param nMinDepth The minimum confirmation depth. * @param fLong Whether to include the JSON version of the transaction. - * @param ret The UniValue into which the result is stored. + * @param ret The vector into which the result is stored. * @param filter_ismine The "is mine" filter flags. * @param filter_label Optional label string to filter incoming transactions. */ -static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +template <class Vec> +static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { CAmount nFee; std::list<COutputEntry> listReceived; @@ -500,8 +501,7 @@ RPCHelpMan listtransactions() if (nFrom < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative from"); - UniValue ret(UniValue::VARR); - + std::vector<UniValue> ret; { LOCK(pwallet->cs_wallet); @@ -523,9 +523,9 @@ RPCHelpMan listtransactions() if ((nFrom + nCount) > (int)ret.size()) nCount = ret.size() - nFrom; - const std::vector<UniValue>& txs = ret.getValues(); + auto txs_rev_it{std::make_move_iterator(ret.rend())}; UniValue result{UniValue::VARR}; - result.push_backV({ txs.rend() - nFrom - nCount, txs.rend() - nFrom }); // Return oldest to newest + result.push_backV(txs_rev_it - nFrom - nCount, txs_rev_it - nFrom); // Return oldest to newest return result; }, }; @@ -889,7 +889,7 @@ RPCHelpMan rescanblockchain() } CWallet::ScanResult result = - pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, true /* fUpdate */); + pwallet->ScanForWalletTransactions(start_block, start_height, stop_height, reserver, /*fUpdate=*/true, /*save_progress=*/false); switch (result.status) { case CWallet::ScanResult::SUCCESS: break; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 1fec82a485..bba31dfe0b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -21,26 +21,22 @@ namespace wallet { //! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details. const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) +BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); - return false; + return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); - error.clear(); // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } LearnRelatedScripts(new_key, type); - dest = GetDestinationForKey(new_key, type); - return true; + return GetDestinationForKey(new_key, type); } typedef std::vector<unsigned char> valtype; @@ -1658,12 +1654,11 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) +BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later if (!CanGetAddresses()) { - error = _("No addresses available"); - return false; + return _("No addresses available"); } { LOCK(cs_desc_man); @@ -1681,15 +1676,14 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest std::vector<CScript> scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return _("Error: Keypool ran out, please call keypoolrefill first"); } + CTxDestination dest; std::optional<OutputType> out_script_type = m_wallet_descriptor.descriptor->GetOutputType(); if (out_script_type && out_script_type == type) { ExtractDestination(scripts_temp[0], dest); @@ -1698,7 +1692,7 @@ bool DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDest } m_wallet_descriptor.next_index++; WalletBatch(m_storage.GetDatabase()).WriteDescriptor(GetID(), m_wallet_descriptor); - return true; + return dest; } } @@ -1769,9 +1763,14 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { LOCK(cs_desc_man); - bool result = GetNewDestination(type, address, error); + auto op_dest = GetNewDestination(type); index = m_wallet_descriptor.next_index - 1; - return result; + if (op_dest) { + address = op_dest.GetObj(); + } else { + error = op_dest.GetError(); + } + return op_dest.HasRes(); } void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index ad924791af..eebc05330f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -11,6 +11,7 @@ #include <script/standard.h> #include <util/error.h> #include <util/message.h> +#include <util/result.h> #include <util/time.h> #include <wallet/crypter.h> #include <wallet/ismine.h> @@ -171,7 +172,7 @@ protected: public: explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {} virtual ~ScriptPubKeyMan() {}; - virtual bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) { return false; } + virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); } virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. @@ -359,7 +360,7 @@ private: public: using ScriptPubKeyMan::ScriptPubKeyMan; - bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override; + BResult<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; @@ -567,7 +568,7 @@ public: mutable RecursiveMutex cs_desc_man; - bool GetNewDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) override; + BResult<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 21007f5a92..9be29c4709 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -652,19 +652,16 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } -static std::optional<CreatedTransactionResult> CreateTransactionInternal( +static BResult<CreatedTransactionResult> CreateTransactionInternal( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, - bilingual_str& error, const CCoinControl& coin_control, - FeeCalculation& fee_calc_out, bool sign) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { AssertLockHeld(wallet.cs_wallet); // out variables, to be packed into returned result structure - CTransactionRef tx; CAmount nFeeRet; int nChangePosInOut = change_pos; @@ -693,6 +690,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Create change script that will be used if we need change CScript scriptChange; + bilingual_str error; // possible error str // coin control: send change to custom address if (!std::get_if<CNoDestination>(&coin_control.destChange)) { @@ -740,13 +738,11 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { - error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); - return std::nullopt; + return strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); - return std::nullopt; + return _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); } // Calculate the cost of change @@ -771,10 +767,8 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, PROTOCOL_VERSION); } - if (IsDust(txout, wallet.chain().relayDustFee())) - { - error = _("Transaction amount too small"); - return std::nullopt; + if (IsDust(txout, wallet.chain().relayDustFee())) { + return _("Transaction amount too small"); } txNew.vout.push_back(txout); } @@ -795,8 +789,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Choose coins to use std::optional<SelectionResult> result = SelectCoins(wallet, res_available_coins.coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { - error = _("Insufficient funds"); - return std::nullopt; + return _("Insufficient funds"); } TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue()); @@ -810,10 +803,8 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Insert change txn at random position: nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1); } - else if ((unsigned int)nChangePosInOut > txNew.vout.size()) - { - error = _("Transaction change output index out of range"); - return std::nullopt; + else if ((unsigned int)nChangePosInOut > txNew.vout.size()) { + return _("Transaction change output index out of range"); } assert(nChangePosInOut != -1); @@ -840,8 +831,7 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); int nBytes = tx_sizes.vsize; if (nBytes == -1) { - error = _("Missing solving data for estimating transaction size"); - return std::nullopt; + return _("Missing solving data for estimating transaction size"); } nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes); @@ -901,11 +891,10 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Error if this output is reduced to be below dust if (IsDust(txout, wallet.chain().relayDustFee())) { if (txout.nValue < 0) { - error = _("The transaction amount is too small to pay the fee"); + return _("The transaction amount is too small to pay the fee"); } else { - error = _("The transaction amount is too small to send after the fee has been deducted"); + return _("The transaction amount is too small to send after the fee has been deducted"); } - return std::nullopt; } } ++i; @@ -915,42 +904,37 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( // Give up if change keypool ran out and change is required if (scriptChange.empty() && nChangePosInOut != -1) { - return std::nullopt; + return error; } if (sign && !wallet.SignTransaction(txNew)) { - error = _("Signing transaction failed"); - return std::nullopt; + return _("Signing transaction failed"); } // Return the constructed transaction data. - tx = MakeTransactionRef(std::move(txNew)); + CTransactionRef tx = MakeTransactionRef(std::move(txNew)); // Limit size if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) || (!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT)) { - error = _("Transaction too large"); - return std::nullopt; + return _("Transaction too large"); } if (nFeeRet > wallet.m_default_max_tx_fee) { - error = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); - return std::nullopt; + return TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); } if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits if (!wallet.chain().checkChainLimits(tx)) { - error = _("Transaction has too long of a mempool chain"); - return std::nullopt; + return _("Transaction has too long of a mempool chain"); } } // Before we return success, we assume any change key will be used to prevent // accidental re-use. reservedest.KeepDestination(); - fee_calc_out = feeCalc; wallet.WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, @@ -960,52 +944,50 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal( feeCalc.est.fail.start, feeCalc.est.fail.end, (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0, feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool); - return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut); + return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc); } -std::optional<CreatedTransactionResult> CreateTransaction( +BResult<CreatedTransactionResult> CreateTransaction( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, - bilingual_str& error, const CCoinControl& coin_control, - FeeCalculation& fee_calc_out, bool sign) { if (vecSend.empty()) { - error = _("Transaction must have at least one recipient"); - return std::nullopt; + return _("Transaction must have at least one recipient"); } if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) { - error = _("Transaction amounts must not be negative"); - return std::nullopt; + return _("Transaction amounts must not be negative"); } LOCK(wallet.cs_wallet); - std::optional<CreatedTransactionResult> txr_ungrouped = CreateTransactionInternal(wallet, vecSend, change_pos, error, coin_control, fee_calc_out, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), txr_ungrouped.has_value(), - txr_ungrouped.has_value() ? txr_ungrouped->fee : 0, txr_ungrouped.has_value() ? txr_ungrouped->change_pos : 0); - if (!txr_ungrouped) return std::nullopt; + auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign); + TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res.HasRes(), + res ? res.GetObj().fee : 0, res ? res.GetObj().change_pos : 0); + if (!res) return res; + const auto& txr_ungrouped = res.GetObj(); // try with avoidpartialspends unless it's enabled already - if (txr_ungrouped->fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { + if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; - bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results - std::optional<CreatedTransactionResult> txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, error2, tmp_cc, fee_calc_out, sign); + auto res_tx_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); + // Helper optional class for now + std::optional<CreatedTransactionResult> txr_grouped{res_tx_grouped.HasRes() ? std::make_optional(res_tx_grouped.GetObj()) : std::nullopt}; // if fee of this alternative one is within the range of the max fee, we use this one - const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped->fee + wallet.m_max_aps_fee) : false}; + const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() ? txr_grouped->change_pos : 0); if (txr_grouped) { wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", - txr_ungrouped->fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); - if (use_aps) return txr_grouped; + txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); + if (use_aps) return res_tx_grouped; } } - return txr_ungrouped; + return res; } bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl) @@ -1041,12 +1023,15 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, } } - FeeCalculation fee_calc_out; - std::optional<CreatedTransactionResult> txr = CreateTransaction(wallet, vecSend, nChangePosInOut, error, coinControl, fee_calc_out, false); - if (!txr) return false; - CTransactionRef tx_new = txr->tx; - nFeeRet = txr->fee; - nChangePosInOut = txr->change_pos; + auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false); + if (!res) { + error = res.GetError(); + return false; + } + const auto& txr = res.GetObj(); + CTransactionRef tx_new = txr.tx; + nFeeRet = txr.fee; + nChangePosInOut = txr.change_pos; if (nChangePosInOut != -1) { tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]); diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 21f0095e77..ab0ff1ee58 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -6,6 +6,8 @@ #define BITCOIN_WALLET_SPEND_H #include <consensus/amount.h> +#include <policy/fees.h> // for FeeCalculation +#include <util/result.h> #include <wallet/coinselection.h> #include <wallet/transaction.h> #include <wallet/wallet.h> @@ -100,10 +102,11 @@ struct CreatedTransactionResult { CTransactionRef tx; CAmount fee; + FeeCalculation fee_calc; int change_pos; - CreatedTransactionResult(CTransactionRef tx, CAmount fee, int change_pos) - : tx(tx), fee(fee), change_pos(change_pos) {} + CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, int _change_pos, const FeeCalculation& _fee_calc) + : tx(_tx), fee(_fee), fee_calc(_fee_calc), change_pos(_change_pos) {} }; /** @@ -111,7 +114,7 @@ struct CreatedTransactionResult * selected by SelectCoins(); Also create the change output, when needed * @note passing change_pos as -1 will result in setting a random position */ -std::optional<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true); +BResult<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true); /** * Insert additional inputs into the transaction by diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 85f3cd7f3c..a418105ee1 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -74,11 +74,9 @@ static void add_coin(std::vector<COutput>& coins, CWallet& wallet, const CAmount tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; if (spendable) { - CTxDestination dest; - bilingual_str error; - const bool destination_ok = wallet.GetNewDestination(OutputType::BECH32, "", dest, error); - assert(destination_ok); - tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest); + auto op_dest = wallet.GetNewDestination(OutputType::BECH32, ""); + assert(op_dest.HasRes()); + tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj()); } uint256 txid = tx.GetHash(); @@ -867,7 +865,23 @@ BOOST_AUTO_TEST_CASE(waste_test) const CAmount new_target{in_amt - fee * 2 - fee_diff * 2}; add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); - BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /* change cost */ 0, new_target)); + BOOST_CHECK_EQUAL(0, GetSelectionWaste(selection, /*change_cost=*/ 0, new_target)); + selection.clear(); + + // Negative waste when the long term fee is greater than the current fee and the selected value == target + const CAmount exact_target1{3 * COIN - 2 * fee}; + const CAmount target_waste1{-2 * fee_diff}; // = (2 * fee) - (2 * (fee + fee_diff)) + add_coin(1 * COIN, 1, selection, fee, fee + fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + fee_diff); + BOOST_CHECK_EQUAL(target_waste1, GetSelectionWaste(selection, /*change_cost=*/ 0, exact_target1)); + selection.clear(); + + // Negative waste when the long term fee is greater than the current fee and change_cost < - (inputs * (fee - long_term_fee)) + const CAmount large_fee_diff{90}; + const CAmount target_waste2{-2 * large_fee_diff + change_cost}; // = (2 * fee) - (2 * (fee + large_fee_diff)) + change_cost + add_coin(1 * COIN, 1, selection, fee, fee + large_fee_diff); + add_coin(2 * COIN, 2, selection, fee, fee + large_fee_diff); + BOOST_CHECK_EQUAL(target_waste2, GetSelectionWaste(selection, change_cost, target)); } BOOST_AUTO_TEST_CASE(effective_value_test) diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 9089c8ff46..816bef6148 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -69,15 +69,14 @@ struct FuzzedWallet { CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - CTxDestination dest; - bilingual_str error; + BResult<CTxDestination> op_dest; if (fuzzed_data_provider.ConsumeBool()) { - assert(wallet->GetNewDestination(type, "", dest, error)); + op_dest = wallet->GetNewDestination(type, ""); } else { - assert(wallet->GetNewChangeDestination(type, dest, error)); + op_dest = wallet->GetNewChangeDestination(type); } - assert(error.empty()); - return GetScriptForDestination(dest); + assert(op_dest.HasRes()); + return GetScriptForDestination(op_dest.GetObj()); } }; diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index bdc148afb4..53c3b5d2ae 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -28,19 +28,18 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) auto check_tx = [&wallet](CAmount leftover_input_amount) { CRecipient recipient{GetScriptForRawPubKey({}), 50 * COIN - leftover_input_amount, true /* subtract fee */}; constexpr int RANDOM_CHANGE_POSITION = -1; - bilingual_str error; CCoinControl coin_control; coin_control.m_feerate.emplace(10000); coin_control.fOverrideFeeRate = true; // We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output coin_control.m_change_type = OutputType::LEGACY; - FeeCalculation fee_calc; - std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, coin_control, fee_calc); - BOOST_CHECK(txr.has_value()); - BOOST_CHECK_EQUAL(txr->tx->vout.size(), 1); - BOOST_CHECK_EQUAL(txr->tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr->fee); - BOOST_CHECK_GT(txr->fee, 0); - return txr->fee; + auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control); + BOOST_CHECK(res); + const auto& txr = res.GetObj(); + BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1); + BOOST_CHECK_EQUAL(txr.tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr.fee); + BOOST_CHECK_GT(txr.fee, 0); + return txr.fee; }; // Send full input amount to recipient, check that only nonzero fee is diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index aa3121511d..ab72721f9d 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -38,7 +38,7 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc } WalletRescanReserver reserver(*wallet); reserver.reserve(); - CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), 0 /* start_height */, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet->ScanForWalletTransactions(cchain.Genesis()->GetBlockHash(), /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK_EQUAL(result.last_scanned_block, cchain.Tip()->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, cchain.Height()); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 0c03b2f4a2..7b0906c0a8 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -4,7 +4,6 @@ #include <wallet/wallet.h> -#include <any> #include <future> #include <memory> #include <stdint.h> @@ -13,7 +12,6 @@ #include <interfaces/chain.h> #include <key_io.h> #include <node/blockstorage.h> -#include <node/context.h> #include <policy/policy.h> #include <rpc/server.h> #include <test/util/logging.h> @@ -55,9 +53,6 @@ static const std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) auto database = MakeWalletDatabase("", options, status, error); auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); NotifyWalletLoaded(context, wallet); - if (context.chain) { - wallet->postInitProcess(); - } return wallet; } @@ -112,7 +107,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions({} /* start_block */, 0 /* start_height */, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/{}, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK(result.last_scanned_block.IsNull()); @@ -123,7 +118,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -131,13 +126,28 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) } AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); + std::chrono::steady_clock::time_point fake_time; + reserver.setNow([&] { fake_time += 60s; return fake_time; }); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + + { + CBlockLocator locator; + BOOST_CHECK(!WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator)); + BOOST_CHECK(locator.IsNull()); + } + + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/true); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 100 * COIN); + + { + CBlockLocator locator; + BOOST_CHECK(WalletBatch{wallet.GetDatabase()}.ReadBestBlock(locator)); + BOOST_CHECK(!locator.IsNull()); + } } // Prune the older block file. @@ -161,7 +171,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); @@ -188,7 +198,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(wallet); reserver.reserve(); - CWallet::ScanResult result = wallet.ScanForWalletTransactions(oldTip->GetBlockHash(), oldTip->nHeight, {} /* max_height */, reserver, false /* update */); + CWallet::ScanResult result = wallet.ScanForWalletTransactions(/*start_block=*/oldTip->GetBlockHash(), /*start_height=*/oldTip->nHeight, /*max_height=*/{}, reserver, /*fUpdate=*/false, /*save_progress=*/false); BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::FAILURE); BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash()); BOOST_CHECK(result.last_scanned_block.IsNull()); @@ -521,14 +531,12 @@ public: CWalletTx& AddTx(CRecipient recipient) { CTransactionRef tx; - bilingual_str error; CCoinControl dummy; - FeeCalculation fee_calc_out; { constexpr int RANDOM_CHANGE_POSITION = -1; - std::optional<CreatedTransactionResult> txr = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, error, dummy, fee_calc_out); - BOOST_CHECK(txr.has_value()); - tx = txr->tx; + auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy); + BOOST_CHECK(res); + tx = res.GetObj().tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; @@ -614,9 +622,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); BOOST_CHECK(!wallet->TopUpKeyPool(1000)); - CTxDestination dest; - bilingual_str error; - BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); + BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "")); } { const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); @@ -624,9 +630,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); - CTxDestination dest; - bilingual_str error; - BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "", dest, error)); + BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "")); } } @@ -761,6 +765,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // being blocked wallet = TestLoadWallet(context); BOOST_CHECK(rescan_completed); + // AddToWallet events for block_tx and mempool_tx BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); @@ -773,6 +778,8 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // transactionAddedToMempool events are processed promise.set_value(); SyncWithValidationInterfaceQueue(); + // AddToWallet events for block_tx and mempool_tx events are counted a + // second time as the notificaiton queue is processed BOOST_CHECK_EQUAL(addtx_count, 4); @@ -796,7 +803,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) SyncWithValidationInterfaceQueue(); }); wallet = TestLoadWallet(context); - BOOST_CHECK_EQUAL(addtx_count, 4); + BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2981797913..9af68000aa 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1665,7 +1665,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r if (start) { // TODO: this should take into account failure by ScanResult::USER_ABORT - ScanResult result = ScanForWalletTransactions(start_block, start_height, {} /* max_height */, reserver, update); + ScanResult result = ScanForWalletTransactions(start_block, start_height, /*max_height=*/{}, reserver, /*fUpdate=*/update, /*save_progress=*/false); if (result.status == ScanResult::FAILURE) { int64_t time_max; CHECK_NONFATAL(chain().findBlock(result.last_failed_block, FoundBlock().maxTime(time_max))); @@ -1678,7 +1678,8 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r /** * Scan the block chain (starting in start_block) for transactions * from or to us. If fUpdate is true, found transactions that already - * exist in the wallet will be updated. + * exist in the wallet will be updated. If max_height is not set, the + * mempool will be scanned as well. * * @param[in] start_block Scan starting block. If block is not on the active * chain, the scan will return SUCCESS immediately. @@ -1696,12 +1697,11 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r * the main chain after to the addition of any new keys you want to detect * transactions for. */ -CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate) +CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress) { - using Clock = std::chrono::steady_clock; - constexpr auto LOG_INTERVAL{60s}; - auto current_time{Clock::now()}; - auto start_time{Clock::now()}; + constexpr auto INTERVAL_TIME{60s}; + auto current_time{reserver.now()}; + auto start_time{reserver.now()}; assert(reserver.isReserved()); @@ -1728,8 +1728,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) { ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); } - if (Clock::now() >= current_time + LOG_INTERVAL) { - current_time = Clock::now(); + + bool next_interval = reserver.now() >= current_time + INTERVAL_TIME; + if (next_interval) { + current_time = reserver.now(); WalletLogPrintf("Still rescanning. At block %d. Progress=%f\n", block_height, progress_current); } @@ -1759,6 +1761,16 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; result.last_scanned_height = block_height; + + if (save_progress && next_interval) { + CBlockLocator loc = m_chain->getActiveChainLocator(block_hash); + + if (!loc.IsNull()) { + WalletLogPrintf("Saving scan progress %d.\n", block_height); + WalletBatch batch(GetDatabase()); + batch.WriteBestBlock(loc); + } + } } else { // could not scan block, keep scanning but record this block as the most recent failure result.last_failed_block = block_hash; @@ -1788,6 +1800,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc } } } + if (!max_height) { + WalletLogPrintf("Scanning current mempool transactions.\n"); + WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); + } ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 100); // hide progress dialog in GUI if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current); @@ -1796,8 +1812,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", block_height, progress_current); result.status = ScanResult::USER_ABORT; } else { - auto duration_milliseconds = std::chrono::duration_cast<std::chrono::milliseconds>(Clock::now() - start_time); - WalletLogPrintf("Rescan completed in %15dms\n", duration_milliseconds.count()); + WalletLogPrintf("Rescan completed in %15dms\n", Ticks<std::chrono::milliseconds>(reserver.now() - start_time)); } return result; } @@ -2308,37 +2323,36 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return res; } -bool CWallet::GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error) +BResult<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label) { LOCK(cs_wallet); - error.clear(); - bool result = false; auto spk_man = GetScriptPubKeyMan(type, false /* internal */); - if (spk_man) { - spk_man->TopUp(); - result = spk_man->GetNewDestination(type, dest, error); - } else { - error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); + if (!spk_man) { + return strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); } - if (result) { - SetAddressBook(dest, label, "receive"); + + spk_man->TopUp(); + auto op_dest = spk_man->GetNewDestination(type); + if (op_dest) { + SetAddressBook(op_dest.GetObj(), label, "receive"); } - return result; + return op_dest; } -bool CWallet::GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error) +BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) { LOCK(cs_wallet); - error.clear(); + CTxDestination dest; + bilingual_str error; ReserveDestination reservedest(this, type); if (!reservedest.GetReservedDestination(dest, true, error)) { - return false; + return error; } reservedest.KeepDestination(); - return true; + return dest; } std::optional<int64_t> CWallet::GetOldestKeyPoolTime() const @@ -3036,20 +3050,31 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf if (tip_height && *tip_height != rescan_height) { - if (chain.havePruned()) { + // Technically we could execute the code below in any case, but performing the + // `while` loop below can make startup very slow, so only check blocks on disk + // if necessary. + if (chain.havePruned() || chain.hasAssumedValidChain()) { int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height; } if (rescan_height != block_height) { - // We can't rescan beyond non-pruned blocks, stop and throw an error. + // We can't rescan beyond blocks we don't have data for, stop and throw an error. // This might happen if a user uses an old wallet within a pruned node // or if they ran -disablewallet for a longer time, then decided to re-enable // Exit early and print an error. + // It also may happen if an assumed-valid chain is in use and therefore not + // all block data is available. // If a block is pruned after this check, we will load the wallet, // but fail the rescan with a generic error. - error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); + + error = chain.hasAssumedValidChain() ? + _( + "Assumed-valid: last wallet synchronisation goes beyond " + "available block data. You need to wait for the background " + "validation chain to download more blocks.") : + _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); return false; } } @@ -3070,7 +3095,7 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf { WalletRescanReserver reserver(*walletInstance); - if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { + if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/true).status)) { error = _("Failed to rescan the wallet during initialization"); return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 669aa8d195..e2c3d76438 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -15,6 +15,7 @@ #include <psbt.h> #include <tinyformat.h> #include <util/message.h> +#include <util/result.h> #include <util/strencodings.h> #include <util/string.h> #include <util/system.h> @@ -45,7 +46,6 @@ using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wall class CScript; enum class FeeEstimateMode; -struct FeeCalculation; struct bilingual_str; namespace wallet { @@ -528,7 +528,7 @@ public: //! USER_ABORT. uint256 last_failed_block; }; - ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate); + ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress); void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); @@ -666,8 +666,8 @@ public: */ void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, bilingual_str& error); - bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, bilingual_str& error); + BResult<CTxDestination> GetNewDestination(const OutputType type, const std::string label); + BResult<CTxDestination> GetNewChangeDestination(const OutputType type); isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -919,8 +919,11 @@ void MaybeResendWalletTxs(WalletContext& context); class WalletRescanReserver { private: + using Clock = std::chrono::steady_clock; + using NowFn = std::function<Clock::time_point()>; CWallet& m_wallet; bool m_could_reserve; + NowFn m_now; public: explicit WalletRescanReserver(CWallet& w) : m_wallet(w), m_could_reserve(false) {} @@ -941,6 +944,10 @@ public: return (m_could_reserve && m_wallet.fScanningWallet); } + Clock::time_point now() const { return m_now ? m_now() : Clock::now(); }; + + void setNow(NowFn now) { m_now = std::move(now); } + ~WalletRescanReserver() { if (m_could_reserve) { |