diff --git a/src/avalanche/test/processor_tests.cpp b/src/avalanche/test/processor_tests.cpp --- a/src/avalanche/test/processor_tests.cpp +++ b/src/avalanche/test/processor_tests.cpp @@ -81,8 +81,8 @@ m_connman = connman.get(); m_node.connman = std::move(connman); m_node.peer_logic = std::make_unique( - m_connman, m_node.banman.get(), *m_node.scheduler, - *m_node.chainman); + m_connman, m_node.banman.get(), *m_node.scheduler, *m_node.chainman, + *m_node.mempool); m_node.chain = interfaces::MakeChain(m_node, config.GetChainParams()); // Get the processor ready. diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -2260,12 +2260,20 @@ node.connman = std::make_unique( config, GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max())); + + // Make mempool generally available in the node context. For example the + // connection manager, wallet, or RPC threads, which are all started after + // this, may use it from the node context. + assert(!node.mempool); + node.mempool = &::g_mempool; + assert(!node.chainman); node.chainman = &g_chainman; ChainstateManager &chainman = *Assert(node.chainman); - node.peer_logic.reset(new PeerLogicValidation( - node.connman.get(), node.banman.get(), *node.scheduler, chainman)); + node.peer_logic.reset( + new PeerLogicValidation(node.connman.get(), node.banman.get(), + *node.scheduler, chainman, *node.mempool)); RegisterValidationInterface(node.peer_logic.get()); // sanitize comments per BIP-0014, format user agent and check total size @@ -2733,13 +2741,6 @@ // We do this by default to avoid confusion with BTC addresses. config.SetCashAddrEncoding(args.GetBoolArg("-usecashaddr", true)); - // Now that the chain state is loaded, make mempool generally available in - // the node context. For example the connection manager, wallet, or RPC - // threads, which are all started after this, may use it from the node - // context. - assert(!node.mempool); - node.mempool = &::g_mempool; - // Step 8: load indexers if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { g_txindex = std::make_unique(nTxIndexCache, false, fReindex); diff --git a/src/net_processing.h b/src/net_processing.h --- a/src/net_processing.h +++ b/src/net_processing.h @@ -14,6 +14,7 @@ extern RecursiveMutex cs_main; extern RecursiveMutex g_cs_orphans; +class CTxMemPool; class ChainstateManager; class Config; @@ -35,12 +36,14 @@ CConnman *const connman; BanMan *const m_banman; ChainstateManager &m_chainman; + CTxMemPool &m_mempool; bool CheckIfBanned(CNode &pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main); public: PeerLogicValidation(CConnman *connman, BanMan *banman, - CScheduler &scheduler, ChainstateManager &chainman); + CScheduler &scheduler, ChainstateManager &chainman, + CTxMemPool &pool); /** * Overridden from CValidationInterface. @@ -137,8 +140,8 @@ bool ProcessMessage(const Config &config, CNode &pfrom, const std::string &msg_type, CDataStream &vRecv, - int64_t nTimeReceived, ChainstateManager &chainman, - CConnman &connman, BanMan *banman, - const std::atomic &interruptMsgProc); + int64_t nTimeReceived, CTxMemPool &mempool, + ChainstateManager &chainman, CConnman &connman, + BanMan *banman, const std::atomic &interruptMsgProc); #endif // BITCOIN_NET_PROCESSING_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -578,7 +578,8 @@ // same peer // pit will only be valid as long as the same cs_main lock is being held. static bool -MarkBlockAsInFlight(const Config &config, NodeId nodeid, const BlockHash &hash, +MarkBlockAsInFlight(const Config &config, CTxMemPool &mempool, NodeId nodeid, + const BlockHash &hash, const Consensus::Params &consensusParams, const CBlockIndex *pindex = nullptr, std::list::iterator **pit = nullptr) @@ -605,8 +606,7 @@ state->vBlocksInFlight.end(), {hash, pindex, pindex != nullptr, std::unique_ptr( - pit ? new PartiallyDownloadedBlock(config, &g_mempool) - : nullptr)}); + pit ? new PartiallyDownloadedBlock(config, &mempool) : nullptr)}); state->nBlocksInFlight++; state->nBlocksInFlightValidHeaders += it->fValidatedHeaders; if (state->nBlocksInFlight == 1) { @@ -1347,9 +1347,10 @@ PeerLogicValidation::PeerLogicValidation(CConnman *connmanIn, BanMan *banman, CScheduler &scheduler, - ChainstateManager &chainman) + ChainstateManager &chainman, + CTxMemPool &pool) : connman(connmanIn), m_banman(banman), m_chainman(chainman), - m_stale_tip_check_time(0) { + m_mempool(pool), m_stale_tip_check_time(0) { // Initialize global variables that cannot be constructed at startup. recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); @@ -1589,7 +1590,8 @@ // Messages // -static bool AlreadyHave(const CInv &inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { +static bool AlreadyHave(const CInv &inv, const CTxMemPool &mempool) + EXCLUSIVE_LOCKS_REQUIRED(cs_main) { switch (inv.type) { case MSG_TX: { assert(recentRejects); @@ -1619,7 +1621,7 @@ } } - return recentRejects->contains(txid) || g_mempool.exists(txid); + return recentRejects->contains(txid) || mempool.exists(txid); } case MSG_BLOCK: return LookupBlockIndex(BlockHash(inv.hash)) != nullptr; @@ -1853,7 +1855,7 @@ } static void ProcessGetData(const Config &config, CNode &pfrom, - CConnman &connman, + CConnman &connman, const CTxMemPool &mempool, const std::atomic &interruptMsgProc) LOCKS_EXCLUDED(cs_main) { AssertLockNotHeld(cs_main); @@ -1897,7 +1899,7 @@ msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); push = true; } else { - auto txinfo = g_mempool.info(TxId(inv.hash)); + auto txinfo = mempool.info(TxId(inv.hash)); // To protect privacy, do not answer getdata using the mempool // when that TX couldn't have been INVed in reply to a MEMPOOL // request, or when it's too recent to have expired from @@ -1974,7 +1976,7 @@ } static bool ProcessHeadersMessage(const Config &config, CNode &pfrom, - CConnman &connman, + CConnman &connman, CTxMemPool &mempool, ChainstateManager &chainman, const std::vector &headers, bool via_compact_block) { @@ -2129,7 +2131,7 @@ break; } vGetData.push_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); - MarkBlockAsInFlight(config, pfrom.GetId(), + MarkBlockAsInFlight(config, mempool, pfrom.GetId(), pindex->GetBlockHash(), chainparams.GetConsensus(), pindex); LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", @@ -2206,6 +2208,7 @@ } void static ProcessOrphanTx(const Config &config, CConnman &connman, + CTxMemPool &mempool, std::set &orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans) { AssertLockHeld(cs_main); @@ -2236,7 +2239,7 @@ continue; } - if (AcceptToMemoryPool(config, g_mempool, orphan_state, porphanTx, + if (AcceptToMemoryPool(config, mempool, orphan_state, porphanTx, false /* bypass_limits */, Amount::zero() /* nAbsurdFee */)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", @@ -2272,7 +2275,7 @@ EraseOrphanTx(orphanTxId); done = true; } - g_mempool.check(&::ChainstateActive().CoinsTip()); + mempool.check(&::ChainstateActive().CoinsTip()); } } @@ -2519,9 +2522,9 @@ bool ProcessMessage(const Config &config, CNode &pfrom, const std::string &msg_type, CDataStream &vRecv, - int64_t nTimeReceived, ChainstateManager &chainman, - CConnman &connman, BanMan *banman, - const std::atomic &interruptMsgProc) { + int64_t nTimeReceived, CTxMemPool &mempool, + ChainstateManager &chainman, CConnman &connman, + BanMan *banman, const std::atomic &interruptMsgProc) { const CChainParams &chainparams = config.GetChainParams(); LogPrint(BCLog::NET, "received: %s (%u bytes) peer=%d\n", SanitizeString(msg_type), vRecv.size(), pfrom.GetId()); @@ -2892,7 +2895,7 @@ return true; } - bool fAlreadyHave = AlreadyHave(inv); + bool fAlreadyHave = AlreadyHave(inv, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); @@ -2955,7 +2958,7 @@ pfrom.vRecvGetData.insert(pfrom.vRecvGetData.end(), vInv.begin(), vInv.end()); - ProcessGetData(config, pfrom, connman, interruptMsgProc); + ProcessGetData(config, pfrom, connman, mempool, interruptMsgProc); return true; } @@ -3207,11 +3210,11 @@ nodestate->m_tx_download.m_tx_in_flight.erase(txid); EraseTxRequest(txid); - if (!AlreadyHave(CInv(MSG_TX, txid)) && - AcceptToMemoryPool(config, g_mempool, state, ptx, + if (!AlreadyHave(CInv(MSG_TX, txid), mempool) && + AcceptToMemoryPool(config, mempool, state, ptx, false /* bypass_limits */, Amount::zero() /* nAbsurdFee */)) { - g_mempool.check(&::ChainstateActive().CoinsTip()); + mempool.check(&::ChainstateActive().CoinsTip()); RelayTransaction(tx.GetId(), connman); for (size_t i = 0; i < tx.vout.size(); i++) { auto it_by_prev = @@ -3228,12 +3231,12 @@ LogPrint(BCLog::MEMPOOL, "AcceptToMemoryPool: peer=%d: accepted %s " "(poolsz %u txn, %u kB)\n", - pfrom.GetId(), tx.GetId().ToString(), g_mempool.size(), - g_mempool.DynamicMemoryUsage() / 1000); + pfrom.GetId(), tx.GetId().ToString(), mempool.size(), + mempool.DynamicMemoryUsage() / 1000); // Recursively process any orphan transactions that depended on this // one - ProcessOrphanTx(config, connman, pfrom.orphan_work_set); + ProcessOrphanTx(config, connman, mempool, pfrom.orphan_work_set); } else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { // It may be the case that the orphans parents have all been // rejected. @@ -3251,7 +3254,7 @@ // FIXME: MSG_TX should use a TxHash, not a TxId. const TxId _txid = txin.prevout.GetTxId(); pfrom.AddKnownTx(_txid); - if (!AlreadyHave(CInv(MSG_TX, _txid))) { + if (!AlreadyHave(CInv(MSG_TX, _txid), mempool)) { RequestTx(State(pfrom.GetId()), _txid, current_time); } } @@ -3453,7 +3456,7 @@ (fAlreadyInFlight && blockInFlightIt->second.first == pfrom.GetId())) { std::list::iterator *queuedBlockIt = nullptr; - if (!MarkBlockAsInFlight(config, pfrom.GetId(), + if (!MarkBlockAsInFlight(config, mempool, pfrom.GetId(), pindex->GetBlockHash(), chainparams.GetConsensus(), pindex, &queuedBlockIt)) { @@ -3461,7 +3464,7 @@ (*queuedBlockIt) ->partialBlock.reset( new PartiallyDownloadedBlock(config, - &g_mempool)); + &mempool)); } else { // The block was already in flight using compact // blocks from the same peer. @@ -3516,7 +3519,7 @@ // peer, or this peer has too many blocks outstanding to // download from. Optimistically try to reconstruct anyway // since we might be able to without any round trips. - PartiallyDownloadedBlock tempBlock(config, &g_mempool); + PartiallyDownloadedBlock tempBlock(config, &mempool); ReadStatus status = tempBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status != READ_STATUS_OK) { @@ -3549,8 +3552,8 @@ if (fProcessBLOCKTXN) { return ProcessMessage(config, pfrom, NetMsgType::BLOCKTXN, - blockTxnMsg, nTimeReceived, chainman, connman, - banman, interruptMsgProc); + blockTxnMsg, nTimeReceived, mempool, chainman, + connman, banman, interruptMsgProc); } if (fRevertToHeaderProcessing) { @@ -3559,8 +3562,8 @@ // disconnect the peer if the header turns out to be for an invalid // block. Note that if a peer tries to build on an invalid chain, // that will be detected and the peer will be banned. - return ProcessHeadersMessage(config, pfrom, connman, chainman, - {cmpctblock.header}, + return ProcessHeadersMessage(config, pfrom, connman, mempool, + chainman, {cmpctblock.header}, /*via_compact_block=*/true); } @@ -3730,7 +3733,8 @@ ReadCompactSize(vRecv); } - return ProcessHeadersMessage(config, pfrom, connman, chainman, headers, + return ProcessHeadersMessage(config, pfrom, connman, mempool, chainman, + headers, /*via_compact_block=*/false); } @@ -4267,12 +4271,12 @@ bool fMoreWork = false; if (!pfrom->vRecvGetData.empty()) { - ProcessGetData(config, *pfrom, *connman, interruptMsgProc); + ProcessGetData(config, *pfrom, *connman, m_mempool, interruptMsgProc); } if (!pfrom->orphan_work_set.empty()) { LOCK2(cs_main, g_cs_orphans); - ProcessOrphanTx(config, *connman, pfrom->orphan_work_set); + ProcessOrphanTx(config, *connman, m_mempool, pfrom->orphan_work_set); } if (pfrom->fDisconnect) { @@ -4355,7 +4359,8 @@ bool fRet = false; try { fRet = ProcessMessage(config, *pfrom, msg_type, vRecv, msg.m_time, - m_chainman, *connman, m_banman, interruptMsgProc); + m_mempool, m_chainman, *connman, m_banman, + interruptMsgProc); if (interruptMsgProc) { return false; } @@ -4947,7 +4952,7 @@ // Respond to BIP35 mempool requests if (fSendTrickle && pto->m_tx_relay->fSendMempool) { - auto vtxinfo = g_mempool.infoAll(); + auto vtxinfo = m_mempool.infoAll(); pto->m_tx_relay->fSendMempool = false; CFeeRate filterrate; { @@ -5001,7 +5006,7 @@ // Topologically and fee-rate sort the inventory we send for // privacy and priority reasons. A heap is used so that not all // items need sorting if only a few are being sent. - CompareInvMempoolOrder compareInvMempoolOrder(&g_mempool); + CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool); std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); // No reason to drain out at many times the network's capacity, @@ -5026,7 +5031,7 @@ continue; } // Not in the mempool anymore? don't bother sending it. - auto txinfo = g_mempool.info(txid); + auto txinfo = m_mempool.info(txid); if (!txinfo.tx) { continue; } @@ -5176,8 +5181,9 @@ vToDownload, staller, consensusParams); for (const CBlockIndex *pindex : vToDownload) { vGetData.push_back(CInv(MSG_BLOCK, pindex->GetBlockHash())); - MarkBlockAsInFlight(config, pto->GetId(), pindex->GetBlockHash(), - consensusParams, pindex); + MarkBlockAsInFlight(config, m_mempool, pto->GetId(), + pindex->GetBlockHash(), consensusParams, + pindex); LogPrint(BCLog::NET, "Requesting block %s (%d) peer=%d\n", pindex->GetBlockHash().ToString(), pindex->nHeight, pto->GetId()); @@ -5226,7 +5232,7 @@ // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); CInv inv(MSG_TX, txid); - if (!AlreadyHave(inv)) { + if (!AlreadyHave(inv, m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. const auto last_request_time = GetTxRequestTime(txid); @@ -5270,7 +5276,7 @@ gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && !pto->HasPermission(PF_FORCERELAY)) { Amount currentFilter = - g_mempool + m_mempool .GetMinFee( gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000) diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -79,7 +79,8 @@ auto connman = std::make_unique(config, 0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), nullptr, *m_node.scheduler, *m_node.chainman); + connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, + *m_node.mempool); // Mock an outbound peer CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -162,7 +163,8 @@ auto connman = std::make_unique(config, 0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), nullptr, *m_node.scheduler, *m_node.chainman); + connman.get(), nullptr, *m_node.scheduler, *m_node.chainman, + *m_node.mempool); const Consensus::Params &consensusParams = config.GetChainParams().GetConsensus(); @@ -241,7 +243,8 @@ DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(config, 0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman); + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, + *m_node.mempool); banman->ClearBanned(); CAddress addr1(ip(0xa0b0c001), NODE_NONE); @@ -310,7 +313,8 @@ DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(config, 0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman); + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, + *m_node.mempool); banman->ClearBanned(); // because 11 is my favorite number. @@ -367,7 +371,8 @@ DEFAULT_MISBEHAVING_BANTIME); auto connman = std::make_unique(config, 0x1337, 0x1337); auto peerLogic = std::make_unique( - connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman); + connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, + *m_node.mempool); banman->ClearBanned(); int64_t nStartTime = GetTime(); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -115,9 +115,9 @@ try { (void)ProcessMessage( config, p2p_node, random_message_type, random_bytes_data_stream, - GetTimeMillis(), *g_setup->m_node.chainman, - *g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), - std::atomic{false}); + GetTimeMillis(), *g_setup->m_node.mempool, + *g_setup->m_node.chainman, *g_setup->m_node.connman.get(), + g_setup->m_node.banman.get(), std::atomic{false}); } catch (const std::ios_base::failure &e) { const std::string exception_message{e.what()}; const auto p = diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -206,7 +206,7 @@ m_node.connman = std::make_unique(config, 0x1337, 0x1337); m_node.peer_logic = std::make_unique( m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, - *m_node.chainman); + *m_node.chainman, *m_node.mempool); { CConnman::Options options; options.m_msgproc = m_node.peer_logic.get();