diff options
Diffstat (limited to 'src')
40 files changed, 321 insertions, 146 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 58c1da697d..e9d3bff1de 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -146,6 +146,7 @@ BITCOIN_TESTS =\ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ test/sock_tests.cpp \ + test/span_tests.cpp \ test/streams_tests.cpp \ test/sync_tests.cpp \ test/system_tests.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index fc83a4ad3a..995b4781fc 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -26,13 +26,13 @@ #include <scheduler.h> #include <script/sigcache.h> #include <util/chaintype.h> +#include <util/fs.h> #include <util/thread.h> #include <validation.h> #include <validationinterface.h> #include <cassert> #include <cstdint> -#include <filesystem> #include <functional> #include <iosfwd> #include <memory> @@ -50,8 +50,8 @@ int main(int argc, char* argv[]) << " BREAK IN FUTURE VERSIONS. DO NOT USE ON YOUR ACTUAL DATADIR." << std::endl; return 1; } - std::filesystem::path abs_datadir = std::filesystem::absolute(argv[1]); - std::filesystem::create_directories(abs_datadir); + fs::path abs_datadir{fs::absolute(argv[1])}; + fs::create_directories(abs_datadir); // SETUP: Context diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 211b4740be..29306b2229 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -107,12 +107,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c std::vector<bool> have_txn(txn_available.size()); { LOCK(pool->cs); - for (size_t i = 0; i < pool->vTxHashes.size(); i++) { - uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first); + for (const auto& tx : pool->txns_randomized) { + uint64_t shortid = cmpctblock.GetShortID(tx->GetWitnessHash()); std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid); if (idit != shorttxids.end()) { if (!have_txn[idit->second]) { - txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx(); + txn_available[idit->second] = tx; have_txn[idit->second] = true; mempool_count++; } else { diff --git a/src/common/args.cpp b/src/common/args.cpp index ca04175696..1f25d13bee 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -28,7 +28,6 @@ #include <cstdint> #include <cstdlib> #include <cstring> -#include <filesystem> #include <map> #include <optional> #include <stdexcept> @@ -720,6 +719,13 @@ fs::path ArgsManager::GetConfigFilePath() const return *Assert(m_config_path); } +void ArgsManager::SetConfigFilePath(fs::path path) +{ + LOCK(cs_args); + assert(!m_config_path); + m_config_path = path; +} + ChainType ArgsManager::GetChainType() const { std::variant<ChainType, std::string> arg = GetChainArg(); diff --git a/src/common/args.h b/src/common/args.h index ae3ed02bc7..1c5db718f4 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -180,6 +180,7 @@ protected: * Return config file path (read-only) */ fs::path GetConfigFilePath() const; + void SetConfigFilePath(fs::path); [[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** diff --git a/src/init.cpp b/src/init.cpp index 62e7e32272..52ebd4a626 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -458,6 +458,11 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-par=<n>", strprintf("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)", -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-persistmempoolv1", + strprintf("Whether a mempool.dat file created by -persistmempool or the savemempool RPC will be written in the legacy format " + "(version 1) or the current format (version 2). This temporary option will be removed in the future. (default: %u)", + DEFAULT_PERSIST_V1_DAT), + ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index dea868f844..b6ea91aaa7 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -244,7 +244,7 @@ public: // outputs in the same transaction) or have shared ancestry, the bump fees are calculated // independently, i.e. as if only one of them is spent. This may result in double-fee-bumping. This // caveat can be rectified per use of the sister-function CalculateCombinedBumpFee(…). - virtual std::map<COutPoint, CAmount> CalculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; + virtual std::map<COutPoint, CAmount> calculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; //! Calculate the combined bump fee for an input set per the same strategy // as in CalculateIndividualBumpFees(…). @@ -252,7 +252,7 @@ public: // bump fees per outpoint, but a single bump fee for the shared ancestry. // The combined bump fee may be used to correct overestimation due to // shared ancestry by multiple UTXOs after coin selection. - virtual std::optional<CAmount> CalculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; + virtual std::optional<CAmount> calculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) = 0; //! Get the node's package limits. //! Currently only returns the ancestor and descendant count limits, but could be enhanced to @@ -395,6 +395,9 @@ public: //! Set mock time. virtual void setMockTime(int64_t time) = 0; + + //! Mock the scheduler to fast forward in time. + virtual void schedulerMockForward(std::chrono::seconds delta_seconds) = 0; }; //! Return implementation of Chain interface. diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 3f8df57124..aeb2612c07 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -204,8 +204,8 @@ public: //! Unset RPC timer interface. virtual void rpcUnsetTimerInterface(RPCTimerInterface* iface) = 0; - //! Get unspent outputs associated with a transaction. - virtual bool getUnspentOutput(const COutPoint& output, Coin& coin) = 0; + //! Get unspent output associated with a transaction. + virtual std::optional<Coin> getUnspentOutput(const COutPoint& output) = 0; //! Broadcast transaction. virtual TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) = 0; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 4b896c11a3..6114236623 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -118,7 +118,7 @@ public: wallet::AddressPurpose* purpose) = 0; //! Get wallet address list. - virtual std::vector<WalletAddress> getAddresses() const = 0; + virtual std::vector<WalletAddress> getAddresses() = 0; //! Get receive requests. virtual std::vector<std::string> getAddressReceiveRequests() = 0; diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 1f175a5ccf..7c905ca4f4 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -83,7 +83,7 @@ private: const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block - LockPoints lockPoints; //!< Track the height and time at which tx was final + mutable LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these @@ -151,7 +151,7 @@ public: } // Update the LockPoints after a reorg - void UpdateLockPoints(const LockPoints& lp) + void UpdateLockPoints(const LockPoints& lp) const { lockPoints = lp; } @@ -172,8 +172,10 @@ public: Parents& GetMemPoolParents() const { return m_parents; } Children& GetMemPoolChildren() const { return m_children; } - mutable size_t vTxHashesIdx; //!< Index in mempool's vTxHashes + mutable size_t idx_randomized; //!< Index in mempool's txns_randomized mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms }; +using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef; + #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index 757be41b3c..d09fd2ba35 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -23,6 +23,8 @@ static constexpr unsigned int DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB{5}; static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336}; /** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */ static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false}; +/** Whether to fall back to legacy V1 serialization when writing mempool.dat */ +static constexpr bool DEFAULT_PERSIST_V1_DAT{false}; /** Default for -acceptnonstdtxn */ static constexpr bool DEFAULT_ACCEPT_NON_STD_TXN{false}; @@ -56,6 +58,7 @@ struct MemPoolOptions { bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG}; bool require_standard{true}; bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF}; + bool persist_v1_dat{DEFAULT_PERSIST_V1_DAT}; MemPoolLimits limits{}; }; } // namespace kernel diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index ff655c5ffa..4087308d1a 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -8,6 +8,7 @@ #include <consensus/amount.h> #include <logging.h> #include <primitives/transaction.h> +#include <random.h> #include <serialize.h> #include <streams.h> #include <sync.h> @@ -34,14 +35,14 @@ using fsbridge::FopenFn; namespace kernel { -static const uint64_t MEMPOOL_DUMP_VERSION = 1; +static const uint64_t MEMPOOL_DUMP_VERSION_NO_XOR_KEY{1}; +static const uint64_t MEMPOOL_DUMP_VERSION{2}; bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active_chainstate, ImportMempoolOptions&& opts) { if (load_path.empty()) return false; - FILE* filestr{opts.mockable_fopen_function(load_path, "rb")}; - CAutoFile file{filestr, CLIENT_VERSION}; + CAutoFile file{opts.mockable_fopen_function(load_path, "rb"), CLIENT_VERSION}; if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); return false; @@ -57,9 +58,15 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active try { uint64_t version; file >> version; - if (version != MEMPOOL_DUMP_VERSION) { + std::vector<std::byte> xor_key; + if (version == MEMPOOL_DUMP_VERSION_NO_XOR_KEY) { + // Leave XOR-key empty + } else if (version == MEMPOOL_DUMP_VERSION) { + file >> xor_key; + } else { return false; } + file.SetXor(xor_key); uint64_t num; file >> num; while (num) { @@ -151,17 +158,22 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock auto mid = SteadyClock::now(); - try { - FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; - if (!filestr) { - return false; - } - - CAutoFile file{filestr, CLIENT_VERSION}; + CAutoFile file{mockable_fopen_function(dump_path + ".new", "wb"), CLIENT_VERSION}; + if (file.IsNull()) { + return false; + } - uint64_t version = MEMPOOL_DUMP_VERSION; + try { + const uint64_t version{pool.m_persist_v1_dat ? MEMPOOL_DUMP_VERSION_NO_XOR_KEY : MEMPOOL_DUMP_VERSION}; file << version; + std::vector<std::byte> xor_key(8); + if (!pool.m_persist_v1_dat) { + FastRandomContext{}.fillrand(xor_key); + file << xor_key; + } + file.SetXor(xor_key); + file << (uint64_t)vinfo.size(); for (const auto& i : vinfo) { file << *(i.tx); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f6dbe4f008..5ec7e17b26 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -328,10 +328,12 @@ public: std::vector<std::string> listRpcCommands() override { return ::tableRPC.listCommands(); } void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } - bool getUnspentOutput(const COutPoint& output, Coin& coin) override + std::optional<Coin> getUnspentOutput(const COutPoint& output) override { LOCK(::cs_main); - return chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin); + Coin coin; + if (chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin)) return coin; + return {}; } TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) override { @@ -648,8 +650,9 @@ public: { if (!m_node.mempool) return false; LOCK(m_node.mempool->cs); - auto it = m_node.mempool->GetIter(txid); - return it && (*it)->GetCountWithDescendants() > 1; + const auto entry{m_node.mempool->GetEntry(Txid::FromUint256(txid))}; + if (entry == nullptr) return false; + return entry->GetCountWithDescendants() > 1; } bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, @@ -669,7 +672,7 @@ public: m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants, ancestorsize, ancestorfees); } - std::map<COutPoint, CAmount> CalculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override + std::map<COutPoint, CAmount> calculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override { if (!m_node.mempool) { std::map<COutPoint, CAmount> bump_fees; @@ -681,7 +684,7 @@ public: return MiniMiner(*m_node.mempool, outpoints).CalculateBumpFees(target_feerate); } - std::optional<CAmount> CalculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override + std::optional<CAmount> calculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override { if (!m_node.mempool) { return 0; @@ -702,9 +705,9 @@ public: if (!m_node.mempool) return true; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); - const CTxMemPool::Limits& limits{m_node.mempool->m_limits}; LOCK(m_node.mempool->cs); - return m_node.mempool->CalculateMemPoolAncestors(entry, limits).has_value(); + std::string err_string; + return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize(), err_string); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { @@ -806,7 +809,7 @@ public: { if (!m_node.mempool) return; LOCK2(::cs_main, m_node.mempool->cs); - for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) { + for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) { notifications.transactionAddedToMempool(entry.GetSharedTx()); } } diff --git a/src/node/mempool_args.cpp b/src/node/mempool_args.cpp index f63d9875fc..ac26600919 100644 --- a/src/node/mempool_args.cpp +++ b/src/node/mempool_args.cpp @@ -93,6 +93,8 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf); + mempool_opts.persist_v1_dat = argsman.GetBoolArg("-persistmempoolv1", mempool_opts.persist_v1_dat); + ApplyArgsManOptions(argsman, mempool_opts.limits); return {}; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index caa2991819..ce5452d1f9 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -188,7 +188,7 @@ void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet) { for (CTxMemPool::setEntries::iterator iit = testSet.begin(); iit != testSet.end(); ) { // Only test txs not already in the block - if (inBlock.count(*iit)) { + if (inBlock.count((*iit)->GetSharedTx()->GetHash())) { testSet.erase(iit++); } else { iit++; @@ -229,7 +229,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) ++nBlockTx; nBlockSigOpsCost += iter->GetSigOpCost(); nFees += iter->GetFee(); - inBlock.insert(iter); + inBlock.insert(iter->GetSharedTx()->GetHash()); bool fPrintPriority = gArgs.GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY); if (fPrintPriority) { @@ -298,7 +298,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // because some of their txs are already in the block indexed_modified_transaction_set mapModifiedTx; // Keep track of entries that failed inclusion, to avoid duplicate work - CTxMemPool::setEntries failedTx; + std::set<Txid> failedTx; CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = mempool.mapTx.get<ancestor_score>().begin(); CTxMemPool::txiter iter; @@ -326,7 +326,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele if (mi != mempool.mapTx.get<ancestor_score>().end()) { auto it = mempool.mapTx.project<0>(mi); assert(it != mempool.mapTx.end()); - if (mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it)) { + if (mapModifiedTx.count(it) || inBlock.count(it->GetSharedTx()->GetHash()) || failedTx.count(it->GetSharedTx()->GetHash())) { ++mi; continue; } @@ -360,7 +360,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't // contain anything that is inBlock. - assert(!inBlock.count(iter)); + assert(!inBlock.count(iter->GetSharedTx()->GetHash())); uint64_t packageSize = iter->GetSizeWithAncestors(); CAmount packageFees = iter->GetModFeesWithAncestors(); @@ -382,7 +382,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele // we must erase failed entries so that we can consider the // next best entry on the next loop iteration mapModifiedTx.get<ancestor_score>().erase(modit); - failedTx.insert(iter); + failedTx.insert(iter->GetSharedTx()->GetHash()); } ++nConsecutiveFailed; @@ -404,7 +404,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele if (!TestPackageTransactions(ancestors)) { if (fUsingModified) { mapModifiedTx.get<ancestor_score>().erase(modit); - failedTx.insert(iter); + failedTx.insert(iter->GetSharedTx()->GetHash()); } continue; } diff --git a/src/node/miner.h b/src/node/miner.h index 4173521585..06a917228d 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -142,7 +142,7 @@ private: uint64_t nBlockTx; uint64_t nBlockSigOpsCost; CAmount nFees; - CTxMemPool::setEntries inBlock; + std::unordered_set<Txid, SaltedTxidHasher> inBlock; // Chain context for the block int nHeight; diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 9557594622..654c4cf0ce 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -1044,8 +1044,8 @@ void CBlockPolicyEstimator::FlushUnconfirmed() std::chrono::hours CBlockPolicyEstimator::GetFeeEstimatorFileAge() { - auto file_time = std::filesystem::last_write_time(m_estimation_filepath); - auto now = std::filesystem::file_time_type::clock::now(); + auto file_time{fs::last_write_time(m_estimation_filepath)}; + auto now{fs::file_time_type::clock::now()}; return std::chrono::duration_cast<std::chrono::hours>(now - file_time); } diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index d032b74008..76ab2b1a96 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -12,6 +12,7 @@ #include <tinyformat.h> #include <txmempool.h> #include <uint256.h> +#include <util/check.h> #include <util/moneystr.h> #include <util/rbf.h> @@ -35,7 +36,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) // If all the inputs have nSequence >= maxint-1, it still might be // signaled for RBF if any unconfirmed parents have signaled. - const CTxMemPoolEntry entry{*pool.mapTx.find(tx.GetHash())}; + const auto& entry{*Assert(pool.GetEntry(tx.GetHash()))}; auto ancestors{pool.AssumeCalculateMemPoolAncestors(__func__, entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 51f6f44923..a916e4ead6 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -360,12 +360,10 @@ QString TransactionDesc::toHTML(interfaces::Node& node, interfaces::Wallet& wall { COutPoint prevout = txin.prevout; - Coin prev; - if(node.getUnspentOutput(prevout, prev)) - { + if (auto prev{node.getUnspentOutput(prevout)}) { { strHTML += "<li>"; - const CTxOut &vout = prev.out; + const CTxOut& vout = prev->out; CTxDestination address; if (ExtractDestination(vout.scriptPubKey, address)) { diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index b387febc1d..bf60eae433 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -290,7 +290,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool info.pushKV("descendantsize", e.GetSizeWithDescendants()); info.pushKV("ancestorcount", e.GetCountWithAncestors()); info.pushKV("ancestorsize", e.GetSizeWithAncestors()); - info.pushKV("wtxid", pool.vTxHashes[e.vTxHashesIdx].first.ToString()); + info.pushKV("wtxid", e.GetTx().GetWitnessHash().ToString()); UniValue fees(UniValue::VOBJ); fees.pushKV("base", ValueFromAmount(e.GetFee())); @@ -316,9 +316,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool info.pushKV("depends", depends); UniValue spent(UniValue::VARR); - const CTxMemPool::txiter& it = pool.mapTx.find(tx.GetHash()); - const CTxMemPoolEntry::Children& children = it->GetMemPoolChildrenConst(); - for (const CTxMemPoolEntry& child : children) { + for (const CTxMemPoolEntry& child : e.GetMemPoolChildrenConst()) { spent.push_back(child.GetTx().GetHash().ToString()); } @@ -345,14 +343,13 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo } LOCK(pool.cs); UniValue o(UniValue::VOBJ); - for (const CTxMemPoolEntry& e : pool.mapTx) { - const uint256& hash = e.GetTx().GetHash(); + for (const CTxMemPoolEntry& e : pool.entryAll()) { UniValue info(UniValue::VOBJ); entryToJSON(pool, info, e); // Mempool has unique entries so there is no advantage in using // UniValue::pushKV, which checks if the key already exists in O(N). // UniValue::pushKVEnd is used instead which currently is O(1). - o.pushKVEnd(hash.ToString(), info); + o.pushKVEnd(e.GetTx().GetHash().ToString(), info); } return o; } else { @@ -461,12 +458,12 @@ static RPCHelpMan getmempoolancestors() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto entry{mempool.GetEntry(Txid::FromUint256(hash))}; + if (entry == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; + auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)}; if (!fVerbose) { UniValue o(UniValue::VARR); @@ -522,15 +519,15 @@ static RPCHelpMan getmempooldescendants() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto it{mempool.GetIter(hash)}; + if (!it) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } CTxMemPool::setEntries setDescendants; - mempool.CalculateDescendants(it, setDescendants); + mempool.CalculateDescendants(*it, setDescendants); // CTxMemPool::CalculateDescendants will include the given tx - setDescendants.erase(it); + setDescendants.erase(*it); if (!fVerbose) { UniValue o(UniValue::VARR); @@ -574,14 +571,13 @@ static RPCHelpMan getmempoolentry() const CTxMemPool& mempool = EnsureAnyMemPool(request.context); LOCK(mempool.cs); - CTxMemPool::txiter it = mempool.mapTx.find(hash); - if (it == mempool.mapTx.end()) { + const auto entry{mempool.GetEntry(Txid::FromUint256(hash))}; + if (entry == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool"); } - const CTxMemPoolEntry &e = *it; UniValue info(UniValue::VOBJ); - entryToJSON(mempool, info, e); + entryToJSON(mempool, info, *entry); return info; }, }; diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 6b3662996c..b085828215 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -91,6 +91,9 @@ static RPCHelpMan mockscheduler() const NodeContext& node_context{EnsureAnyNodeContext(request.context)}; CHECK_NONFATAL(node_context.scheduler)->MockForward(std::chrono::seconds{delta_seconds}); SyncWithValidationInterfaceQueue(); + for (const auto& chain_client : node_context.chain_clients) { + chain_client->schedulerMockForward(std::chrono::seconds(delta_seconds)); + } return UniValue::VNULL; }, diff --git a/src/span.h b/src/span.h index 7209d21a58..2e8da27cde 100644 --- a/src/span.h +++ b/src/span.h @@ -222,15 +222,32 @@ public: template <typename O> friend class Span; }; +// Return result of calling .data() method on type T. This is used to be able to +// write template deduction guides for the single-parameter Span constructor +// below that will work if the value that is passed has a .data() method, and if +// the data method does not return a void pointer. +// +// It is important to check for the void type specifically below, so the +// deduction guides can be used in SFINAE contexts to check whether objects can +// be converted to spans. If the deduction guides did not explicitly check for +// void, and an object was passed that returned void* from data (like +// std::vector<bool>), the template deduction would succeed, but the Span<void> +// object instantiation would fail, resulting in a hard error, rather than a +// SFINAE error. +// https://stackoverflow.com/questions/68759148/sfinae-to-detect-the-explicitness-of-a-ctad-deduction-guide +// https://stackoverflow.com/questions/16568986/what-happens-when-you-call-data-on-a-stdvectorbool +template<typename T> +using DataResult = std::remove_pointer_t<decltype(std::declval<T&>().data())>; + // Deduction guides for Span // For the pointer/size based and iterator based constructor: template <typename T, typename EndOrSize> Span(T*, EndOrSize) -> Span<T>; // For the array constructor: template <typename T, std::size_t N> Span(T (&)[N]) -> Span<T>; // For the temporaries/rvalue references constructor, only supporting const output. -template <typename T> Span(T&&) -> Span<std::enable_if_t<!std::is_lvalue_reference_v<T>, const std::remove_pointer_t<decltype(std::declval<T&&>().data())>>>; +template <typename T> Span(T&&) -> Span<std::enable_if_t<!std::is_lvalue_reference_v<T> && !std::is_void_v<DataResult<T&&>>, const DataResult<T&&>>>; // For (lvalue) references, supporting mutable output. -template <typename T> Span(T&) -> Span<std::remove_pointer_t<decltype(std::declval<T&>().data())>>; +template <typename T> Span(T&) -> Span<std::enable_if_t<!std::is_void_v<DataResult<T&>>, DataResult<T&>>>; /** Pop the last element off a span, and return a reference to that element. */ template <typename T> diff --git a/src/streams.h b/src/streams.h index d58de5233b..f5b441344f 100644 --- a/src/streams.h +++ b/src/streams.h @@ -182,6 +182,11 @@ public: memcpy(dst.data(), m_data.data(), dst.size()); m_data = m_data.subspan(dst.size()); } + + void ignore(size_t n) + { + m_data = m_data.subspan(n); + } }; /** Double ended buffer combining vector and stream-like interfaces. @@ -471,7 +476,7 @@ class AutoFile { protected: std::FILE* m_file; - const std::vector<std::byte> m_xor; + std::vector<std::byte> m_xor; public: explicit AutoFile(std::FILE* file, std::vector<std::byte> data_xor={}) : m_file{file}, m_xor{std::move(data_xor)} {} @@ -511,6 +516,9 @@ public: */ bool IsNull() const { return m_file == nullptr; } + /** Continue with a different XOR key */ + void SetXor(std::vector<std::byte> data_xor) { m_xor = data_xor; } + /** Implementation detail, only used internally. */ std::size_t detail_fread(Span<std::byte> dst); diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 4348a20886..e4ef019daf 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -51,8 +51,8 @@ static CBlock BuildBlockTestCase() { } // Number of shared use_counts we expect for a tx we haven't touched -// (block + mempool + our copy from the GetSharedTx call) -constexpr long SHARED_TX_OFFSET{3}; +// (block + mempool entry + mempool txns_randomized + our copy from the GetSharedTx call) +constexpr long SHARED_TX_OFFSET{4}; BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) { @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT { @@ -80,7 +80,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK(!partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); size_t poolSize = pool.size(); pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock CBlock block2; { @@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that partialBlock = tmp; } - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2 bool mutated; BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated)); @@ -196,15 +196,15 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 + BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3 txhash = block.vtx[2]->GetHash(); block.vtx.clear(); block2.vtx.clear(); block3.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) @@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) LOCK2(cs_main, pool.cs); pool.addUnchecked(entry.FromTx(block.vtx[1])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0); uint256 txhash; @@ -240,7 +240,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); CBlock block2; PartiallyDownloadedBlock partialBlockCopy = partialBlock; @@ -253,9 +253,9 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) txhash = block.vtx[1]->GetHash(); block.vtx.clear(); block2.vtx.clear(); - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block. } - BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block + BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block } BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 3882e0e547..4a040c56de 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -63,17 +63,28 @@ FUZZ_TARGET(banman, .init = initialize_banman) // The complexity is O(N^2), where N is the input size, because each call // might call DumpBanlist (or other methods that are at least linear // complexity of the input size). + bool contains_invalid{false}; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) { CallOneOf( fuzzed_data_provider, [&] { - ban_man.Ban(ConsumeNetAddr(fuzzed_data_provider), - ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); + CNetAddr net_addr{ConsumeNetAddr(fuzzed_data_provider)}; + const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)}; + if (addr.has_value() && addr->IsValid()) { + net_addr = *addr; + } else { + contains_invalid = true; + } + ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); }, [&] { - ban_man.Ban(ConsumeSubNet(fuzzed_data_provider), - ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); + CSubNet subnet{ConsumeSubNet(fuzzed_data_provider)}; + subnet = LookupSubNet(subnet.ToString()); + if (!subnet.IsValid()) { + contains_invalid = true; + } + ban_man.Ban(subnet, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool()); }, [&] { ban_man.ClearBanned(); @@ -109,7 +120,9 @@ FUZZ_TARGET(banman, .init = initialize_banman) BanMan ban_man_read{banlist_file, /*client_interface=*/nullptr, /*default_ban_time=*/0}; banmap_t banmap_read; ban_man_read.GetBanned(banmap_read); - assert(banmap == banmap_read); + if (!contains_invalid) { + assert(banmap == banmap_read); + } } } fs::remove(fs::PathToString(banlist_file + ".json")); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index db58a0baec..217e4a6d22 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CheckSort<descendant_score>(pool, sortedOrder); CTxMemPool::setEntries setAncestors; - setAncestors.insert(pool.mapTx.find(tx6.GetHash())); + setAncestors.insert(pool.GetIter(tx6.GetHash()).value()); CMutableTransaction tx7 = CMutableTransaction(); tx7.vin.resize(1); tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); @@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) tx8.vout.resize(1); tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; tx8.vout[0].nValue = 10 * COIN; - setAncestors.insert(pool.mapTx.find(tx7.GetHash())); + setAncestors.insert(pool.GetIter(tx7.GetHash()).value()); pool.addUnchecked(entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8), setAncestors); // Now tx8 should be sorted low, but tx6/tx both high @@ -247,8 +247,8 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) std::vector<std::string> snapshotOrder = sortedOrder; - setAncestors.insert(pool.mapTx.find(tx8.GetHash())); - setAncestors.insert(pool.mapTx.find(tx9.GetHash())); + setAncestors.insert(pool.GetIter(tx8.GetHash()).value()); + setAncestors.insert(pool.GetIter(tx9.GetHash()).value()); /* tx10 depends on tx8 and tx9 and has a high fee*/ CMutableTransaction tx10 = CMutableTransaction(); tx10.vin.resize(2); @@ -291,11 +291,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10U); // Now try removing tx10 and verify the sort order returns to normal - pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY); CheckSort<descendant_score>(pool, snapshotOrder); - pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); - pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY); + pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY); } BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp index 2531ea7c47..311e402e3e 100644 --- a/src/test/miniminer_tests.cpp +++ b/src/test/miniminer_tests.cpp @@ -94,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup) const CFeeRate feerate_zero(0); mini_miner_target0.BuildMockTemplate(feerate_zero); // Check the quit condition: - BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize())); + BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(Assert(pool.GetEntry(tx_mod_negative->GetHash()))->GetTxSize())); BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty()); // With no target feerate, the template includes all transactions, even negative feerate ones. @@ -179,9 +179,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup) }; std::map<uint256, TxDimensions> tx_dims; for (const auto& tx : all_transactions) { - const auto it = pool.GetIter(tx->GetHash()).value(); - tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(), - CFeeRate(it->GetModifiedFee(), it->GetTxSize())}); + const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))}; + tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(), + CFeeRate(entry.GetModifiedFee(), entry.GetTxSize())}); } const std::vector<CFeeRate> various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999), @@ -447,15 +447,15 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup) // tx3's feerate is lower than tx2's. same fee, different weight. BOOST_CHECK(tx2_feerate > tx3_feerate); const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]); - const auto tx3_iter = pool.GetIter(tx3->GetHash()); - BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors())); + const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))}; + BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors())); const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]); const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]); - const auto tx6_iter = pool.GetIter(tx6->GetHash()); - BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors())); + const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))}; + BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors())); const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]); - const auto tx7_iter = pool.GetIter(tx7->GetHash()); - BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors())); + const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))}; + BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors())); BOOST_CHECK(tx4_feerate > tx6_anc_feerate); BOOST_CHECK(tx4_feerate > tx7_anc_feerate); diff --git a/src/test/span_tests.cpp b/src/test/span_tests.cpp new file mode 100644 index 0000000000..f6cac10b09 --- /dev/null +++ b/src/test/span_tests.cpp @@ -0,0 +1,73 @@ +// Copyright (c) 2023 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 <span.h> + +#include <boost/test/unit_test.hpp> +#include <array> +#include <set> +#include <vector> + +namespace { +struct Ignore +{ + template<typename T> Ignore(T&&) {} +}; +template<typename T> +bool Spannable(T&& value, decltype(Span{value})* enable = nullptr) +{ + return true; +} +bool Spannable(Ignore) +{ + return false; +} + +#if defined(__clang__) +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wunneeded-member-function" +# pragma clang diagnostic ignored "-Wunused-member-function" +#endif +struct SpannableYes +{ + int* data(); + size_t size(); +}; +struct SpannableNo +{ + void* data(); + size_t size(); +}; +#if defined(__clang__) +# pragma clang diagnostic pop +#endif +} // namespace + +BOOST_AUTO_TEST_SUITE(span_tests) + +// Make sure template Span template deduction guides accurately enable calls to +// Span constructor overloads that work, and disable calls to constructor overloads that +// don't work. This makes it is possible to use the Span constructor in a SFINAE +// contexts like in the Spannable function above to detect whether types are or +// aren't compatible with Spans at compile time. +// +// Previously there was a bug where writing a SFINAE check for vector<bool> was +// not possible, because in libstdc++ vector<bool> has a data() memeber +// returning void*, and the Span template guide ignored the data() return value, +// so the template substitution would succeed, but the constructor would fail, +// resulting in a fatal compile error, rather than a SFINAE error that could be +// handled. +BOOST_AUTO_TEST_CASE(span_constructor_sfinae) +{ + BOOST_CHECK(Spannable(std::vector<int>{})); + BOOST_CHECK(!Spannable(std::set<int>{})); + BOOST_CHECK(!Spannable(std::vector<bool>{})); + BOOST_CHECK(Spannable(std::array<int, 3>{})); + BOOST_CHECK(Spannable(Span<int>{})); + BOOST_CHECK(Spannable("char array")); + BOOST_CHECK(Spannable(SpannableYes{})); + BOOST_CHECK(!Spannable(SpannableNo{})); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 64cb5522eb..35e5c6a037 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -283,8 +283,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // Check that all txs are in the pool { - LOCK(m_node.mempool->cs); - BOOST_CHECK_EQUAL(m_node.mempool->mapTx.size(), txs.size()); + BOOST_CHECK_EQUAL(m_node.mempool->size(), txs.size()); } // Run a thread that simulates an RPC caller that is polling while @@ -295,7 +294,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // not some intermediate amount. while (true) { LOCK(m_node.mempool->cs); - if (m_node.mempool->mapTx.size() == 0) { + if (m_node.mempool->size() == 0) { // We are done with the reorg break; } @@ -304,7 +303,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg) // be atomic. So the caller assumes that the returned mempool // is consistent. That is, it has all txs that were there // before the reorg. - assert(m_node.mempool->mapTx.size() == txs.size()); + assert(m_node.mempool->size() == txs.size()); continue; } LOCK(cs_main); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 461662ad93..e057d7ece1 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -412,6 +412,7 @@ CTxMemPool::CTxMemPool(const Options& opts) m_max_datacarrier_bytes{opts.max_datacarrier_bytes}, m_require_standard{opts.require_standard}, m_full_rbf{opts.full_rbf}, + m_persist_v1_dat{opts.persist_v1_dat}, m_limits{opts.limits} { } @@ -480,8 +481,8 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces minerPolicyEstimator->processTransaction(entry, validFeeEstimate); } - vTxHashes.emplace_back(tx.GetWitnessHash(), newit); - newit->vTxHashesIdx = vTxHashes.size() - 1; + txns_randomized.emplace_back(newit->GetSharedTx()); + newit->idx_randomized = txns_randomized.size() - 1; TRACE3(mempool, added, entry.GetTx().GetHash().data(), @@ -517,14 +518,16 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) RemoveUnbroadcastTx(hash, true /* add logging because unchecked */ ); - if (vTxHashes.size() > 1) { - vTxHashes[it->vTxHashesIdx] = std::move(vTxHashes.back()); - vTxHashes[it->vTxHashesIdx].second->vTxHashesIdx = it->vTxHashesIdx; - vTxHashes.pop_back(); - if (vTxHashes.size() * 2 < vTxHashes.capacity()) - vTxHashes.shrink_to_fit(); + if (txns_randomized.size() > 1) { + // Update idx_randomized of the to-be-moved entry. + Assert(GetEntry(txns_randomized.back()->GetHash()))->idx_randomized = it->idx_randomized; + // Remove entry from txns_randomized by replacing it with the back and deleting the back. + txns_randomized[it->idx_randomized] = std::move(txns_randomized.back()); + txns_randomized.pop_back(); + if (txns_randomized.size() * 2 < txns_randomized.capacity()) + txns_randomized.shrink_to_fit(); } else - vTxHashes.clear(); + txns_randomized.clear(); totalTxSize -= it->GetTxSize(); m_total_fee -= it->GetFee(); @@ -836,6 +839,18 @@ static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()}; } +std::vector<CTxMemPoolEntryRef> CTxMemPool::entryAll() const +{ + AssertLockHeld(cs); + + std::vector<CTxMemPoolEntryRef> ret; + ret.reserve(mapTx.size()); + for (const auto& it : GetSortedDepthAndScore()) { + ret.emplace_back(*it); + } + return ret; +} + std::vector<TxMempoolInfo> CTxMemPool::infoAll() const { LOCK(cs); @@ -850,6 +865,13 @@ std::vector<TxMempoolInfo> CTxMemPool::infoAll() const return ret; } +const CTxMemPoolEntry* CTxMemPool::GetEntry(const Txid& txid) const +{ + AssertLockHeld(cs); + const auto i = mapTx.find(txid); + return i == mapTx.end() ? nullptr : &(*i); +} + CTransactionRef CTxMemPool::get(const uint256& hash) const { LOCK(cs); @@ -1033,7 +1055,7 @@ void CCoinsViewMemPool::Reset() size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + cachedInnerUsage; } void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) { diff --git a/src/txmempool.h b/src/txmempool.h index cbeabb31fa..122bac09b1 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -392,7 +392,7 @@ public: indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; - std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order + std::vector<CTransactionRef> txns_randomized GUARDED_BY(cs); //!< All transactions in mapTx, in random order typedef std::set<txiter, CompareIteratorByHash> setEntries; @@ -446,6 +446,7 @@ public: const std::optional<unsigned> m_max_datacarrier_bytes; const bool m_require_standard; const bool m_full_rbf; + const bool m_persist_v1_dat; const Limits m_limits; @@ -684,6 +685,8 @@ public: return (mapTx.count(gtxid.GetHash()) != 0); } + const CTxMemPoolEntry* GetEntry(const Txid& txid) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs); + CTransactionRef get(const uint256& hash) const; txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs) { @@ -695,6 +698,7 @@ public: /** Returns info for a transaction if its entry_sequence < last_sequence */ TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const; + std::vector<CTxMemPoolEntryRef> entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector<TxMempoolInfo> infoAll() const; size_t DynamicMemoryUsage() const; diff --git a/src/util/fs.h b/src/util/fs.h index 8f79f6cba6..7e2803b6aa 100644 --- a/src/util/fs.h +++ b/src/util/fs.h @@ -184,6 +184,7 @@ static inline path PathFromString(const std::string& string) * already exists or is a symlink to an existing directory. * This is a temporary workaround for an issue in libstdc++ that has been fixed * upstream [PR101510]. + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 */ static inline bool create_directories(const std::filesystem::path& p) { diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index 2a9eb3502e..8aa7493aa8 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -11,13 +11,11 @@ #include <logging.h> #include <sync.h> -#include <tinyformat.h> #include <util/fs.h> #include <util/getuniquepath.h> #include <util/syserror.h> #include <cerrno> -#include <filesystem> #include <fstream> #include <map> #include <memory> @@ -263,7 +261,7 @@ bool RenameOver(fs::path src, fs::path dest) { #ifdef __MINGW64__ // This is a workaround for a bug in libstdc++ which - // implements std::filesystem::rename with _wrename function. + // implements fs::rename with _wrename function. // This bug has been fixed in upstream: // - GCC 10.3: 8dd1c1085587c9f8a21bb5e588dfe1e8cdbba79e // - GCC 11.1: 1dfd95f0a0ca1d9e6cbc00e6cbfd1fa20a98f312 diff --git a/src/validation.cpp b/src/validation.cpp index 018566fa9c..34103d18bc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -353,7 +353,7 @@ void Chainstate::MaybeUpdateMempoolForReorg( const std::optional<LockPoints> new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)}; if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) { // Now update the mempool entry lockpoints as well. - m_mempool->mapTx.modify(it, [&new_lock_points](CTxMemPoolEntry& e) { e.UpdateLockPoints(*new_lock_points); }); + it->UpdateLockPoints(*new_lock_points); } else { return true; } @@ -362,9 +362,7 @@ void Chainstate::MaybeUpdateMempoolForReorg( // If the transaction spends any coinbase outputs, it must be mature. if (it->GetSpendsCoinbase()) { for (const CTxIn& txin : tx.vin) { - auto it2 = m_mempool->mapTx.find(txin.prevout.hash); - if (it2 != m_mempool->mapTx.end()) - continue; + if (m_mempool->exists(GenTxid::Txid(txin.prevout.hash))) continue; const Coin& coin{CoinsTip().AccessCoin(txin.prevout)}; assert(!coin.IsSpent()); const auto mempool_spend_height{m_chain.Tip()->nHeight + 1}; @@ -1506,9 +1504,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with // the new transactions. This ensures we don't double-count transaction counts and sizes when // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy. - auto iter = m_pool.GetIter(txid); - assert(iter != std::nullopt); - results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); + const auto& entry{*Assert(m_pool.GetEntry(txid))}; + results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(entry.GetTxSize(), entry.GetFee())); } else if (m_pool.exists(GenTxid::Txid(txid))) { // Transaction with the same non-witness data but different witness (same txid, // different wtxid) already exists in the mempool. @@ -1517,10 +1514,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, // transaction for the mempool one. Note that we are ignoring the validity of the // package transaction passed in. // TODO: allow witness replacement in packages. - auto iter = m_pool.GetIter(txid); - assert(iter != std::nullopt); + const auto& entry{*Assert(m_pool.GetEntry(txid))}; // Provide the wtxid of the mempool tx so that the caller can look it up in the mempool. - results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash())); + results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(entry.GetTx().GetWitnessHash())); } else { // Transaction does not already exist in the mempool. // Try submitting the transaction on its own. diff --git a/src/wallet/context.h b/src/wallet/context.h index 57a6ed77f7..58b9924fb4 100644 --- a/src/wallet/context.h +++ b/src/wallet/context.h @@ -13,6 +13,7 @@ #include <vector> class ArgsManager; +class CScheduler; namespace interfaces { class Chain; class Wallet; @@ -34,6 +35,7 @@ using LoadWalletFn = std::function<void(std::unique_ptr<interfaces::Wallet> wall //! behavior. struct WalletContext { interfaces::Chain* chain{nullptr}; + CScheduler* scheduler{nullptr}; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct // It is unsafe to lock this after locking a CWallet::cs_wallet mutex because // this could introduce inconsistent lock ordering and cause deadlocks. diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index f4cb4bbd66..f0fd789c96 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -86,7 +86,7 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CMutableTrans reused_inputs.push_back(txin.prevout); } - std::optional<CAmount> combined_bump_fee = wallet.chain().CalculateCombinedBumpFee(reused_inputs, newFeerate); + std::optional<CAmount> combined_bump_fee = wallet.chain().calculateCombinedBumpFee(reused_inputs, newFeerate); if (!combined_bump_fee.has_value()) { errors.push_back(strprintf(Untranslated("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions."))); } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index c9419be0ab..4ca5e29229 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -11,6 +11,7 @@ #include <policy/fees.h> #include <primitives/transaction.h> #include <rpc/server.h> +#include <scheduler.h> #include <support/allocators/secure.h> #include <sync.h> #include <uint256.h> @@ -212,7 +213,7 @@ public: } return true; } - std::vector<WalletAddress> getAddresses() const override + std::vector<WalletAddress> getAddresses() override { LOCK(m_wallet->cs_wallet); std::vector<WalletAddress> result; @@ -585,10 +586,15 @@ public: } bool verify() override { return VerifyWallets(m_context); } bool load() override { return LoadWallets(m_context); } - void start(CScheduler& scheduler) override { return StartWallets(m_context, scheduler); } + void start(CScheduler& scheduler) override + { + m_context.scheduler = &scheduler; + return StartWallets(m_context); + } void flush() override { return FlushWallets(m_context); } void stop() override { return StopWallets(m_context); } void setMockTime(int64_t time) override { return SetMockTime(time); } + void schedulerMockForward(std::chrono::seconds delta) override { Assert(m_context.scheduler)->MockForward(delta); } //! WalletLoader methods util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) override diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 4cdfadbee2..8b78a670e4 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -141,7 +141,7 @@ bool LoadWallets(WalletContext& context) } } -void StartWallets(WalletContext& context, CScheduler& scheduler) +void StartWallets(WalletContext& context) { for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { pwallet->postInitProcess(); @@ -149,9 +149,9 @@ void StartWallets(WalletContext& context, CScheduler& scheduler) // Schedule periodic wallet flushes and tx rebroadcasts if (context.args->GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { - scheduler.scheduleEvery([&context] { MaybeCompactWalletDB(context); }, std::chrono::milliseconds{500}); + context.scheduler->scheduleEvery([&context] { MaybeCompactWalletDB(context); }, 500ms); } - scheduler.scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); + context.scheduler->scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); } void FlushWallets(WalletContext& context) diff --git a/src/wallet/load.h b/src/wallet/load.h index 0882f7f8ad..c079cad955 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -26,7 +26,7 @@ bool VerifyWallets(WalletContext& context); bool LoadWallets(WalletContext& context); //! Complete startup of wallets. -void StartWallets(WalletContext& context, CScheduler& scheduler); +void StartWallets(WalletContext& context); //! Flush all wallets in preparation for shutdown. void FlushWallets(WalletContext& context); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 8314a2ddfa..ed0134375c 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -259,7 +259,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const { PreSelectedInputs result; const bool can_grind_r = wallet.CanGrindR(); - std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); + std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate); for (const COutPoint& outpoint : coin_control.ListSelected()) { int input_bytes = -1; CTxOut txout; @@ -453,7 +453,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, } if (feerate.has_value()) { - std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().CalculateIndividualBumpFees(outpoints, feerate.value()); + std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(outpoints, feerate.value()); for (auto& [_, outputs] : result.coins) { for (auto& output : outputs) { @@ -725,7 +725,7 @@ util::Result<SelectionResult> ChooseSelectionResult(interfaces::Chain& chain, co outpoints.push_back(coin->outpoint); summed_bump_fees += coin->ancestor_bump_fees; } - std::optional<CAmount> combined_bump_fee = chain.CalculateCombinedBumpFee(outpoints, coin_selection_params.m_effective_feerate); + std::optional<CAmount> combined_bump_fee = chain.calculateCombinedBumpFee(outpoints, coin_selection_params.m_effective_feerate); if (!combined_bump_fee.has_value()) { return util::Error{_("Failed to calculate bump fees, because unconfirmed UTXOs depend on enormous cluster of unconfirmed transactions.")}; } |