diff --git a/CMakeLists.txt b/CMakeLists.txt index 931aebf4b275..6a88776965af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ set(SERVER_SOURCES ./src/checkpoints.cpp ./src/httprpc.cpp ./src/httpserver.cpp + ./src/indirectmap.h ./src/init.cpp ./src/interfaces/handler.cpp ./src/interfaces/wallet.cpp diff --git a/doc/files.md b/doc/files.md index 18b672c63a24..7815643abb0b 100644 --- a/doc/files.md +++ b/doc/files.md @@ -11,6 +11,7 @@ database/* | BDB database environment; only used for wallet since 0.8.0 db.log | wallet database log file; moved to wallets/ directory on new installs since 0.16.0 debug.log | contains debug information and general logging generated by pivxd or pivx-qt fee_estimates.dat | stores statistics used to estimate minimum transaction fees and priorities required for confirmation; since 0.10.0 +mempool.dat | dump of the mempool's transactions; since 5.0.2 budget.dat | stores data for budget objects masternode.conf | contains configuration settings for remote masternodes mncache.dat | stores data for masternode list diff --git a/src/Makefile.am b/src/Makefile.am index 986fd6cd8a68..f2c2a834e9ca 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -189,6 +189,7 @@ BITCOIN_CORE_H = \ hash.h \ httprpc.h \ httpserver.h \ + indirectmap.h \ init.h \ interfaces/handler.h \ interfaces/wallet.h \ diff --git a/src/indirectmap.h b/src/indirectmap.h new file mode 100644 index 000000000000..af8f23b46fc4 --- /dev/null +++ b/src/indirectmap.h @@ -0,0 +1,58 @@ +// Copyright (c) 2016-2020 The Bitcoin developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_INDIRECTMAP_H +#define BITCOIN_INDIRECTMAP_H + +#include + +template +struct DereferencingComparator { bool operator()(const T a, const T b) const { return *a < *b; } }; + +/* Map whose keys are pointers, but are compared by their dereferenced values. + * + * Differs from a plain std::map > in + * that methods that take a key for comparison take a K rather than taking a K* + * (taking a K* would be confusing, since it's the value rather than the address + * of the object for comparison that matters due to the dereferencing comparator). + * + * Objects pointed to by keys must not be modified in any way that changes the + * result of DereferencingComparator. + */ +template +class indirectmap { +private: + typedef std::map > base; + base m; +public: + typedef typename base::iterator iterator; + typedef typename base::const_iterator const_iterator; + typedef typename base::size_type size_type; + typedef typename base::value_type value_type; + + // passthrough (pointer interface) + std::pair insert(const value_type& value) { return m.insert(value); } + + // pass address (value interface) + iterator find(const K& key) { return m.find(&key); } + const_iterator find(const K& key) const { return m.find(&key); } + iterator lower_bound(const K& key) { return m.lower_bound(&key); } + const_iterator lower_bound(const K& key) const { return m.lower_bound(&key); } + size_type erase(const K& key) { return m.erase(&key); } + size_type count(const K& key) const { return m.count(&key); } + + // passthrough + bool empty() const { return m.empty(); } + size_type size() const { return m.size(); } + size_type max_size() const { return m.max_size(); } + void clear() { m.clear(); } + iterator begin() { return m.begin(); } + iterator end() { return m.end(); } + const_iterator begin() const { return m.begin(); } + const_iterator end() const { return m.end(); } + const_iterator cbegin() const { return m.cbegin(); } + const_iterator cend() const { return m.cend(); } +}; + +#endif // BITCOIN_INDIRECTMAP_H diff --git a/src/init.cpp b/src/init.cpp index a0f200660cde..cd98e6f568d8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -155,6 +155,7 @@ CClientUIInterface uiInterface; // Declared but not defined in guiinterface.h // volatile bool fRequestShutdown = false; +std::atomic fDumpMempoolLater(false); void StartShutdown() { @@ -245,6 +246,9 @@ void PrepareShutdown() DumpBudgets(g_budgetman); DumpMasternodePayments(); UnregisterNodeSignals(GetNodeSignals()); + if (fDumpMempoolLater && gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + DumpMempool(); + } // After everything has been shut down, but before things get flushed, stop the // CScheduler/checkqueue threadGroup @@ -418,6 +422,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-maxorphantx=", strprintf(_("Keep at most unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS)); strUsage += HelpMessageOpt("-maxmempool=", strprintf(_("Keep the transaction memory pool below megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE)); strUsage += HelpMessageOpt("-mempoolexpiry=", strprintf(_("Do not keep transactions in the mempool longer than hours (default: %u)"), DEFAULT_MEMPOOL_EXPIRY)); + strUsage += HelpMessageOpt("-persistmempool", strprintf(_("Whether to save the mempool on shutdown and load on restart (default: %u)"), DEFAULT_PERSIST_MEMPOOL)); strUsage += HelpMessageOpt("-par=", 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)); #ifndef WIN32 strUsage += HelpMessageOpt("-pid=", strprintf(_("Specify pid file (default: %s)"), PIVX_PID_FILENAME)); @@ -700,6 +705,11 @@ void ThreadImport(std::vector vImportFiles) LogPrintf("Stopping after block import\n"); StartShutdown(); } + + if (gArgs.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + LoadMempool(); + fDumpMempoolLater = !fRequestShutdown; + } } /** Sanity checks @@ -899,10 +909,16 @@ void InitParameterInteraction() LogPrintf("%s : parameter interaction: -salvagewallet=1 -> setting -rescan=1\n", __func__); } - // -zapwallettx implies a rescan - if (gArgs.GetBoolArg("-zapwallettxes", false)) { + int zapwallettxes = gArgs.GetArg("-zapwallettxes", 0); + // -zapwallettxes implies dropping the mempool on startup + if (zapwallettxes != 0 && gArgs.SoftSetBoolArg("-persistmempool", false)) { + LogPrintf("%s: parameter interaction: -zapwallettxes=%s -> setting -persistmempool=0\n", __func__, zapwallettxes); + } + + // -zapwallettxes implies a rescan + if (zapwallettxes != 0) { if (gArgs.SoftSetBoolArg("-rescan", true)) - LogPrintf("%s : parameter interaction: -zapwallettxes= -> setting -rescan=1\n", __func__); + LogPrintf("%s : parameter interaction: -zapwallettxes=%s -> setting -rescan=1\n", __func__, zapwallettxes); } } diff --git a/src/memusage.h b/src/memusage.h index b06421a75017..f2a56b6b9b90 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_MEMUSAGE_H #define BITCOIN_MEMUSAGE_H +#include "indirectmap.h" #include "prevector.h" #include @@ -185,6 +186,20 @@ static inline size_t DynamicUsage(const std::shared_ptr& p) return p ? MallocUsage(sizeof(X)) + MallocUsage(sizeof(stl_shared_counter)) : 0; } +// indirectmap has underlying map with pointer as key + +template +static inline size_t DynamicUsage(const indirectmap& m) +{ + return MallocUsage(sizeof(stl_tree_node >)) * m.size(); +} + +template +static inline size_t IncrementalDynamicUsage(const indirectmap& m) +{ + return MallocUsage(sizeof(stl_tree_node >)); +} + // Boost data structures template diff --git a/src/net.cpp b/src/net.cpp index ff41506d9c30..792ff0a05ed4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2438,12 +2438,14 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hashContinue = UINT256_ZERO; nStartingHeight = -1; filterInventoryKnown.reset(); + fSendMempool = false; fGetAddr = false; nNextLocalAddrSend = 0; nNextAddrSend = 0; nNextInvSend = 0; fRelayTxes = false; pfilter = new CBloomFilter(); + timeLastMempoolReq = 0; nPingNonceSent = 0; nPingUsecStart = 0; nPingUsecTime = 0; diff --git a/src/net.h b/src/net.h index 93cc53b72e8c..365ff809886e 100644 --- a/src/net.h +++ b/src/net.h @@ -580,7 +580,7 @@ class CNode // a) it allows us to not relay tx invs before receiving the peer's version message // b) the peer may tell us in their version message that we should not relay tx invs // until they have initialized their bloom filter. - bool fRelayTxes; + bool fRelayTxes; //protected by cs_filter CSemaphoreGrant grantOutbound; RecursiveMutex cs_filter; CBloomFilter* pfilter; @@ -610,11 +610,24 @@ class CNode // inventory based relay CRollingBloomFilter filterInventoryKnown; - std::vector vInventoryToSend; + // Set of transaction ids we still have to announce. + // They are sorted by the mempool before relay, so the order is not important. + std::set setInventoryTxToSend; + // List of block ids we still have announce. + // There is no final sorting before sending, as they are always sent immediately + // and in the order requested. + std::vector vInventoryBlockToSend; + // Set of tier two messages ids we still have to announce. + std::vector vInventoryTierTwoToSend; RecursiveMutex cs_inventory; std::multimap mapAskFor; std::vector vBlockRequested; int64_t nNextInvSend; + // Used for BIP35 mempool sending, also protected by cs_inventory + bool fSendMempool; + + // Last time a "MEMPOOL" request was serviced. + std::atomic timeLastMempoolReq{0}; // Ping time measurement: // The pong reply we're expecting, or 0 if no pong expected. @@ -735,11 +748,15 @@ class CNode void PushInventory(const CInv& inv) { - { - LOCK(cs_inventory); - if (inv.type == MSG_TX && filterInventoryKnown.contains(inv.hash)) - return; - vInventoryToSend.push_back(inv); + LOCK(cs_inventory); + if (inv.type == MSG_TX) { + if (!filterInventoryKnown.contains(inv.hash)) { + setInventoryTxToSend.insert(inv.hash); + } + } else if (inv.type == MSG_BLOCK) { + vInventoryBlockToSend.push_back(inv.hash); + } else { + vInventoryTierTwoToSend.emplace_back(inv); } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 48de89d900f8..5f308867b767 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -864,11 +864,11 @@ void static ProcessGetData(CNode* pfrom, CConnman& connman, std::atomic& i bool pushed = false; if (inv.type == MSG_TX) { - CTransaction tx; - if (mempool.lookup(inv.hash, tx)) { + auto txinfo = mempool.info(inv.hash); + if (txinfo.tx) { // future: add timeLastMempoolReq check CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss.reserve(1000); - ss << tx; + ss << *txinfo.tx; connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::TX, ss)); pushed = true; } @@ -1027,7 +1027,6 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR std::string strSubVer; std::string cleanSubVer; int nStartingHeight = -1; - bool fRelay = true; vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; nSendVersion = std::min(nVersion, PROTOCOL_VERSION); nServices = ServiceFlags(nServiceInt); @@ -1053,10 +1052,9 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH); cleanSubVer = SanitizeString(strSubVer); } - if (!vRecv.empty()) + if (!vRecv.empty()) { vRecv >> nStartingHeight; - if (!vRecv.empty()) - vRecv >> fRelay; + } // Disconnect if we connected to ourself if (pfrom->fInbound && !connman.CheckIncomingNonce(nNonce)) { @@ -1084,9 +1082,14 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR } pfrom->nStartingHeight = nStartingHeight; pfrom->fClient = !(nServices & NODE_NETWORK); + { LOCK(pfrom->cs_filter); - pfrom->fRelayTxes = fRelay; // set to true after we get the first filter* message + if (!vRecv.empty()) { + vRecv >> pfrom->fRelayTxes; // set to true after we get the first filter* message + } else { + pfrom->fRelayTxes = true; + } } // Change version @@ -1633,26 +1636,10 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR else if (strCommand == NetMsgType::MEMPOOL) { - LOCK2(cs_main, pfrom->cs_filter); - std::vector vtxid; - mempool.queryHashes(vtxid); - std::vector vInv; - for (uint256& hash : vtxid) { - CInv inv(MSG_TX, hash); - CTransaction tx; - bool fInMemPool = mempool.lookup(hash, tx); - if (!fInMemPool) continue; // another thread removed since queryHashes, maybe... - if ((pfrom->pfilter && pfrom->pfilter->IsRelevantAndUpdate(tx)) || - (!pfrom->pfilter)) - vInv.push_back(inv); - if (vInv.size() == MAX_INV_SZ) { - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); - vInv.clear(); - } - } - if (vInv.size() > 0) - connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + // todo: limit mempool request with a bandwidth limit + LOCK(pfrom->cs_inventory); + pfrom->fSendMempool = true; } @@ -1745,12 +1732,13 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR CBloomFilter filter; vRecv >> filter; + LOCK(pfrom->cs_filter); + if (!filter.IsWithinSizeConstraints()) { // There is no excuse for sending a too-large filter LOCK(cs_main); Misbehaving(pfrom->GetId(), 100); } else { - LOCK(pfrom->cs_filter); delete pfrom->pfilter; pfrom->pfilter = new CBloomFilter(filter); pfrom->pfilter->UpdateEmptyFull(); @@ -1940,6 +1928,22 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic& interru return fMoreWork; } +class CompareInvMempoolOrder +{ + CTxMemPool *mp; +public: + CompareInvMempoolOrder(CTxMemPool *_mempool) + { + mp = _mempool; + } + + bool operator()(std::set::iterator a, std::set::iterator b) + { + /* As std::make_heap produces a max-heap, we want the entries with the + * fewest ancestors/highest fee to sort later. */ + return mp->CompareDepthAndScore(*b, *a); + } +}; bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsgProc) { @@ -2064,43 +2068,113 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic& interruptMsg std::vector vInv; std::vector vInvWait; { + LOCK(pto->cs_inventory); + vInv.reserve(std::max(pto->vInventoryBlockToSend.size(), INVENTORY_BROADCAST_MAX)); + + // Add blocks + for (const uint256& hash : pto->vInventoryBlockToSend) { + vInv.emplace_back(CInv(MSG_BLOCK, hash)); + if (vInv.size() == MAX_INV_SZ) { + connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); + } + } + pto->vInventoryBlockToSend.clear(); + + // Add tier two INVs + for (const CInv& tInv : pto->vInventoryTierTwoToSend) { + vInv.emplace_back(tInv); + if (vInv.size() == MAX_INV_SZ) { + connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); + } + } + pto->vInventoryTierTwoToSend.clear(); + + // Check whether periodic send should happen bool fSendTrickle = pto->fWhitelisted; if (pto->nNextInvSend < nNow) { fSendTrickle = true; - pto->nNextInvSend = PoissonNextSend(nNow, AVG_INVENTORY_BROADCAST_INTERVAL); + // Use half the delay for outbound peers, as there is less privacy concern for them. + pto->nNextInvSend = PoissonNextSend(nNow, INVENTORY_BROADCAST_INTERVAL >> !pto->fInbound); } - LOCK(pto->cs_inventory); - vInv.reserve(pto->vInventoryToSend.size()); - vInvWait.reserve(pto->vInventoryToSend.size()); - for (const CInv& inv : pto->vInventoryToSend) { - if (inv.type == MSG_TX && pto->filterInventoryKnown.contains(inv.hash)) - continue; - // trickle out tx inv to protect privacy - if (inv.type == MSG_TX && !fSendTrickle) { - // 1/4 of tx invs blast to all immediately - static uint256 hashSalt; - if (hashSalt.IsNull()) - hashSalt = GetRandHash(); - uint256 hashRand = inv.hash ^ hashSalt; - hashRand = Hash(BEGIN(hashRand), END(hashRand)); - bool fTrickleWait = ((hashRand & 3) != 0); - - if (fTrickleWait) { - vInvWait.push_back(inv); - continue; + // Time to send but the peer has requested we not relay transactions. + if (fSendTrickle) { + LOCK(pto->cs_filter); + if (!pto->fRelayTxes) pto->setInventoryTxToSend.clear(); + } + + // Respond to BIP35 mempool requests + if (fSendTrickle && pto->fSendMempool) { + auto vtxinfo = mempool.infoAll(); + pto->fSendMempool = false; + // future: back port fee filter rate + LOCK(pto->cs_filter); + + for (const auto& txinfo : vtxinfo) { + const uint256& hash = txinfo.tx->GetHash(); + CInv inv(MSG_TX, hash); + pto->setInventoryTxToSend.erase(hash); + // future: add fee filter check here.. + if (pto->pfilter) { + if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + } + pto->filterInventoryKnown.insert(hash); + vInv.emplace_back(inv); + if (vInv.size() == MAX_INV_SZ) { + connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); } } + pto->timeLastMempoolReq = GetTime(); + } - pto->filterInventoryKnown.insert(inv.hash); - - vInv.push_back(inv); - if (vInv.size() >= 1000) { - connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); - vInv.clear(); + // Determine transactions to relay + if (fSendTrickle) { + // Produce a vector with all candidates for sending + std::vector::iterator> vInvTx; + vInvTx.reserve(pto->setInventoryTxToSend.size()); + for (std::set::iterator it = pto->setInventoryTxToSend.begin(); it != pto->setInventoryTxToSend.end(); it++) { + vInvTx.push_back(it); + } + // 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(&mempool); + std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); + // No reason to drain out at many times the network's capacity, + // especially since we have many peers and some will draw much shorter delays. + unsigned int nRelayedTransactions = 0; + LOCK(pto->cs_filter); + while (!vInvTx.empty() && nRelayedTransactions < INVENTORY_BROADCAST_MAX) { + // Fetch the top element from the heap + std::pop_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); + std::set::iterator it = vInvTx.back(); + vInvTx.pop_back(); + uint256 hash = *it; + // Remove it from the to-be-sent set + pto->setInventoryTxToSend.erase(it); + // Check if not in the filter already + if (pto->filterInventoryKnown.contains(hash)) { + continue; + } + // Not in the mempool anymore? don't bother sending it. + auto txinfo = mempool.info(hash); + if (!txinfo.tx) { + continue; + } + // todo: back port feerate filter. + if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; + // Send + vInv.emplace_back(CInv(MSG_TX, hash)); + nRelayedTransactions++; + if (vInv.size() == MAX_INV_SZ) { + connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + vInv.clear(); + } + pto->filterInventoryKnown.insert(hash); } } - pto->vInventoryToSend = vInvWait; } if (!vInv.empty()) connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); diff --git a/src/net_processing.h b/src/net_processing.h index c524e59ed751..9924cf704d00 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -21,6 +21,13 @@ static const unsigned int DEFAULT_BLOCK_SPAM_FILTER_MAX_SIZE = 100; /** Default for -blockspamfiltermaxavg, maximum average size of an index occurrence in the block spam filter */ static const unsigned int DEFAULT_BLOCK_SPAM_FILTER_MAX_AVG = 10; +/** Average delay between trickled inventory transmissions in seconds. + * Blocks and whitelisted receivers bypass this, outbound peers get half this delay. */ +static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5; +/** Maximum number of inventory items to send per transmission. + * Limits the impact of low-fee transaction floods. */ +static const unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL; + /** Register with a network node to receive its signals */ void RegisterNodeSignals(CNodeSignals& nodeSignals); /** Unregister a network node */ diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index ef99ca8bcc8f..b80003d535e6 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -464,6 +464,8 @@ void BitcoinApplication::requestShutdown() // Show a simple window indicating shutdown status ShutdownWindow::showShutdownWindow(window); + StartShutdown(); + // Request shutdown from core thread Q_EMIT requestedShutdown(); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 03eea4e0a419..ab699df23af5 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -399,7 +399,7 @@ UniValue mempoolToJSON(bool fVerbose = false) info.pushKV("currentpriority", e.GetPriority(chainActive.Height())); info.pushKV("descendantcount", e.GetCountWithDescendants()); info.pushKV("descendantsize", e.GetSizeWithDescendants()); - info.pushKV("descendantfees", e.GetFeesWithDescendants()); + info.pushKV("descendantfees", e.GetModFeesWithDescendants()); const CTransaction& tx = e.GetTx(); std::set setDepends; for (const CTxIn& txin : tx.vin) { @@ -456,7 +456,7 @@ UniValue getrawmempool(const JSONRPCRequest& request) " \"currentpriority\" : n, (numeric) transaction priority now\n" " \"descendantcount\" : n, (numeric) number of in-mempool descendant transactions (including this one)\n" " \"descendantsize\" : n, (numeric) size of in-mempool descendants (including this one)\n" - " \"descendantfees\" : n, (numeric) fees of in-mempool descendants (including this one)\n" + " \"descendantfees\" : n, (numeric) modified fees (see above) of in-mempool descendants (including this one)\n" " \"depends\" : [ (array) unconfirmed transactions used as inputs for this transaction\n" " \"transactionid\", (string) parent transaction id\n" " ... ]\n" diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0a863a6a97c3..9ed326af740a 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -53,15 +53,15 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) CTxMemPool testPool(CFeeRate(0)); - std::list removed; + std::vector removed; // Nothing in pool, remove should do nothing: - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Just the parent: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 1); removed.clear(); @@ -73,16 +73,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: - testPool.remove(txChild[0], removed, true); + testPool.removeRecursive(txChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 2); removed.clear(); // ... make sure grandchild and child are gone: - testPool.remove(txGrandChild[0], removed, true); + testPool.removeRecursive(txGrandChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 0); - testPool.remove(txChild[0], removed, true); + testPool.removeRecursive(txChild[0], &removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Remove parent, all children/grandchildren should go: - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 5); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, &removed); BOOST_CHECK_EQUAL(removed.size(), 6); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -279,12 +279,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) BOOST_CHECK_EQUAL(pool.size(), 10); // Now try removing tx10 and verify the sort order returns to normal - std::list removed; - pool.remove(pool.mapTx.find(tx10.GetHash())->GetTx(), removed, true); + pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx()); CheckSort(pool, snapshotOrder); - pool.remove(pool.mapTx.find(tx9.GetHash())->GetTx(), removed, true); - pool.remove(pool.mapTx.find(tx8.GetHash())->GetTx(), removed, true); + pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx()); + pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx()); /* Now check the sort on the mining score index. * Final order should be: * @@ -316,6 +315,118 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CheckSort(pool, sortedOrder); } +BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) +{ + CTxMemPool pool(CFeeRate(0)); + TestMemPoolEntryHelper entry; + entry.hadNoDependencies = true; + + /* 3rd highest fee */ + CMutableTransaction tx1 = CMutableTransaction(); + tx1.vout.resize(1); + tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx1.vout[0].nValue = 10 * COIN; + pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).Priority(10.0).FromTx(tx1)); + + /* highest fee */ + CMutableTransaction tx2 = CMutableTransaction(); + tx2.vout.resize(1); + tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx2.vout[0].nValue = 2 * COIN; + pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).Priority(9.0).FromTx(tx2)); + uint64_t tx2Size = ::GetSerializeSize(tx2, SER_NETWORK, PROTOCOL_VERSION); + + /* lowest fee */ + CMutableTransaction tx3 = CMutableTransaction(); + tx3.vout.resize(1); + tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx3.vout[0].nValue = 5 * COIN; + pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).Priority(100.0).FromTx(tx3)); + + /* 2nd highest fee */ + CMutableTransaction tx4 = CMutableTransaction(); + tx4.vout.resize(1); + tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx4.vout[0].nValue = 6 * COIN; + pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).Priority(1.0).FromTx(tx4)); + + /* equal fee rate to tx1, but newer */ + CMutableTransaction tx5 = CMutableTransaction(); + tx5.vout.resize(1); + tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx5.vout[0].nValue = 11 * COIN; + pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); + BOOST_CHECK_EQUAL(pool.size(), 5); + + std::vector sortedOrder; + sortedOrder.resize(5); + sortedOrder[0] = tx2.GetHash().ToString(); // 20000 + sortedOrder[1] = tx4.GetHash().ToString(); // 15000 + // tx1 and tx5 are both 10000 + // Ties are broken by hash, not timestamp, so determine which + // hash comes first. + if (tx1.GetHash() < tx5.GetHash()) { + sortedOrder[2] = tx1.GetHash().ToString(); + sortedOrder[3] = tx5.GetHash().ToString(); + } else { + sortedOrder[2] = tx5.GetHash().ToString(); + sortedOrder[3] = tx1.GetHash().ToString(); + } + sortedOrder[4] = tx3.GetHash().ToString(); // 0 + + CheckSort(pool, sortedOrder); + + /* low fee parent with high fee child */ + /* tx6 (0) -> tx7 (high) */ + CMutableTransaction tx6 = CMutableTransaction(); + tx6.vout.resize(1); + tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx6.vout[0].nValue = 20 * COIN; + uint64_t tx6Size = ::GetSerializeSize(tx6, SER_NETWORK, PROTOCOL_VERSION); + + pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6)); + BOOST_CHECK_EQUAL(pool.size(), 6); + // Ties are broken by hash + if (tx3.GetHash() < tx6.GetHash()) + sortedOrder.push_back(tx6.GetHash().ToString()); + else + sortedOrder.insert(sortedOrder.end()-1,tx6.GetHash().ToString()); + + CheckSort(pool, sortedOrder); + + CMutableTransaction tx7 = CMutableTransaction(); + tx7.vin.resize(1); + tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); + tx7.vin[0].scriptSig = CScript() << OP_11; + tx7.vout.resize(1); + tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx7.vout[0].nValue = 10 * COIN; + uint64_t tx7Size = ::GetSerializeSize(tx7, SER_NETWORK, PROTOCOL_VERSION); + + /* set the fee to just below tx2's feerate when including ancestor */ + CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; + + //CTxMemPoolEntry entry7(tx7, fee, 2, 10.0, 1, true); + pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7)); + BOOST_CHECK_EQUAL(pool.size(), 7); + sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); + CheckSort(pool, sortedOrder); + + /* after tx6 is mined, tx7 should move up in the sort */ + std::vector vtx; + vtx.emplace_back(MakeTransactionRef(tx6)); + pool.removeForBlock(vtx, 1, nullptr, false); + + sortedOrder.erase(sortedOrder.begin()+1); + // Ties are broken by hash + if (tx3.GetHash() < tx6.GetHash()) + sortedOrder.pop_back(); + else + sortedOrder.erase(sortedOrder.end()-2); + sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString()); + CheckSort(pool, sortedOrder); +} + BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { @@ -443,12 +554,11 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); std::vector vtx; - std::list conflicts; SetMockTime(42); SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); // ... we should keep the same min fee until we get a block - pool.removeForBlock(vtx, 1, conflicts); + pool.removeForBlock(vtx, 1); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); // ... then feerate should drop 1/2 each halflife diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 7c1bbea41635..16e7bb27b7be 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -38,7 +38,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (unsigned int i = 0; i < 128; i++) garbage.push_back('X'); CMutableTransaction tx; - std::list dummyConflicted; tx.vin.resize(1); tx.vin[0].scriptSig = garbage; tx.vout.resize(1); @@ -67,13 +66,13 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // 9/10 blocks add 2nd highest and so on until ... // 1/10 blocks add lowest fee transactions while (txHashes[9-h].size()) { - CTransaction btx; - if (mpool.lookup(txHashes[9-h].back(), btx)) - block.push_back(std::make_shared(btx)); + CTransactionRef ptx = mpool.get(txHashes[9-h].back()); + if (ptx) + block.emplace_back(ptx); txHashes[9-h].pop_back(); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); block.clear(); if (blocknum == 30) { // At this point we should need to combine 5 buckets to get enough data points @@ -112,7 +111,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Mine 50 more blocks with no transactions happening, estimates shouldn't change // We haven't decayed the moving average enough so we still have enough data points in every bucket while (blocknum < 250) - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); for (int i = 1; i < 10;i++) { BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee); @@ -131,7 +130,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].push_back(hash); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); } int answerFound; @@ -144,13 +143,13 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Estimates should still not be below original for (int j = 0; j < 10; j++) { while(txHashes[j].size()) { - CTransaction btx; - if (mpool.lookup(txHashes[j].back(), btx)) - block.push_back(std::make_shared(btx)); + CTransactionRef ptx = mpool.get(txHashes[j].back()); + if (ptx) + block.emplace_back(ptx); txHashes[j].pop_back(); } } - mpool.removeForBlock(block, 265, dummyConflicted); + mpool.removeForBlock(block, 265); block.clear(); for (int i = 1; i < 10;i++) { BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); @@ -164,12 +163,12 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); - CTransaction btx; - if (mpool.lookup(hash, btx)) - block.push_back(std::make_shared(btx)); + CTransactionRef ptx = mpool.get(hash); + if (ptx) + block.emplace_back(ptx); } } - mpool.removeForBlock(block, ++blocknum, dummyConflicted); + mpool.removeForBlock(block, ++blocknum); block.clear(); } for (int i = 1; i < 10; i++) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f181d464bcb3..0ee9e728a59f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -33,11 +33,16 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe nCountWithDescendants = 1; nSizeWithDescendants = nTxSize; - nFeesWithDescendants = nFee; + nModFeesWithDescendants = nFee; CAmount nValueIn = _tx->GetValueOut()+nFee; assert(inChainInputValue <= nValueIn); feeDelta = 0; + + nCountWithAncestors = 1; + nSizeWithAncestors = nTxSize; + nModFeesWithAncestors = nFee; + nSigOpCountWithAncestors = sigOpCount; } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -57,27 +62,21 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) { + nModFeesWithDescendants += newFeeDelta - feeDelta; + nModFeesWithAncestors += newFeeDelta - feeDelta; feeDelta = newFeeDelta; } // Update the given tx for any in-mempool descendants. // Assumes that setMemPoolChildren is correct for the given tx and all // descendants. -bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit, cacheMap &cachedDescendants, const std::set &setExclude) +void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set &setExclude) { - // Track the number of entries (outside setExclude) that we'd need to visit - // (will bail out if it exceeds maxDescendantsToVisit) - int nChildrenToVisit = 0; - setEntries stageEntries, setAllDescendants; stageEntries = GetMemPoolChildren(updateIt); while (!stageEntries.empty()) { const txiter cit = *stageEntries.begin(); - if (cit->IsDirty()) { - // Don't consider any more children if any descendant is dirty - return false; - } setAllDescendants.insert(cit); stageEntries.erase(cit); const setEntries &setChildren = GetMemPoolChildren(cit); @@ -87,22 +86,11 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit // We've already calculated this one, just add the entries for this set // but don't traverse again. for (const txiter& cacheEntry : cacheIt->second) { - // update visit count only for new child transactions - // (outside of setExclude and stageEntries) - if (setAllDescendants.insert(cacheEntry).second && - !setExclude.count(cacheEntry->GetTx().GetHash()) && - !stageEntries.count(cacheEntry)) { - nChildrenToVisit++; - } + setAllDescendants.insert(cacheEntry); } } else if (!setAllDescendants.count(childEntry)) { - // Schedule for later processing and update our visit count - if (stageEntries.insert(childEntry).second && !setExclude.count(childEntry->GetTx().GetHash())) { - nChildrenToVisit++; - } - } - if (nChildrenToVisit > maxDescendantsToVisit) { - return false; + // Schedule for later processing + stageEntries.insert(childEntry); } } } @@ -114,19 +102,21 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit for (const txiter& cit : setAllDescendants) { if (!setExclude.count(cit->GetTx().GetHash())) { modifySize += cit->GetTxSize(); - modifyFee += cit->GetFee(); + modifyFee += cit->GetModifiedFee(); modifyCount++; cachedDescendants[updateIt].insert(cit); + // Update ancestor state for each descendant + mapTx.modify(cit, update_ancestor_state(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCount())); } } mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount)); - return true; } // vHashesToUpdate is the set of transaction hashes from a disconnected block // which has been re-added to the mempool. // for each entry, look for descendants that are outside hashesToUpdate, and // add fee/size information for such descendants to the parent. +// for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashesToUpdate) { LOCK(cs); @@ -152,11 +142,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashes if (it == mapTx.end()) { continue; } - std::map::iterator iter = mapNextTx.lower_bound(COutPoint(hash, 0)); + auto iter = mapNextTx.lower_bound(COutPoint(hash, 0)); // First calculate the children, and update setMemPoolChildren to // include them, and update their setMemPoolParents to include this tx. - for (; iter != mapNextTx.end() && iter->first.hash == hash; ++iter) { - const uint256 &childHash = iter->second.ptx->GetHash(); + for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) { + const uint256 &childHash = iter->second->GetHash(); txiter childIter = mapTx.find(childHash); assert(childIter != mapTx.end()); // We can skip updating entries we've encountered before or that @@ -166,14 +156,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashes UpdateParent(childIter, it, true); } } - if (!UpdateForDescendants(it, 100, mapMemPoolDescendantsToUpdate, setAlreadyIncluded)) { - // Mark as dirty if we can't do the calculation. - mapTx.modify(it, set_dirty()); - } + UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded); } } -bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) +bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const { setEntries parentHashes; const auto &tx = entry.GetSharedTx(); @@ -244,12 +231,26 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors } const int64_t updateCount = (add ? 1 : -1); const int64_t updateSize = updateCount * it->GetTxSize(); - const CAmount updateFee = updateCount * it->GetFee(); + const CAmount updateFee = updateCount * it->GetModifiedFee(); for (const txiter& ancestorIt : setAncestors) { mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount)); } } +void CTxMemPool::UpdateEntryForAncestors(txiter it, const setEntries &setAncestors) +{ + int64_t updateCount = setAncestors.size(); + int64_t updateSize = 0; + CAmount updateFee = 0; + int updateSigOps = 0; + for (const txiter& ancestorIt : setAncestors) { + updateSize += ancestorIt->GetTxSize(); + updateFee += ancestorIt->GetModifiedFee(); + updateSigOps += ancestorIt->GetSigOpCount(); + } + mapTx.modify(it, update_ancestor_state(updateSize, updateFee, updateCount, updateSigOps)); +} + void CTxMemPool::UpdateChildrenForRemoval(txiter it) { const setEntries &setMemPoolChildren = GetMemPoolChildren(it); @@ -258,11 +259,30 @@ void CTxMemPool::UpdateChildrenForRemoval(txiter it) } } -void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) +void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) { // For each entry, walk back all ancestors and decrement size associated with this // transaction const uint64_t nNoLimit = std::numeric_limits::max(); + if (updateDescendants) { + // updateDescendants should be true whenever we're not recursively + // removing a tx and all its descendants, eg when a transaction is + // confirmed in a block. + // Here we only update statistics and not data in mapLinks (which + // we need to preserve until we're finished with all operations that + // need to traverse the mempool). + for (const txiter& removeIt : entriesToRemove) { + setEntries setDescendants; + CalculateDescendants(removeIt, setDescendants); + setDescendants.erase(removeIt); // don't update state for self + int64_t modifySize = -((int64_t)removeIt->GetTxSize()); + CAmount modifyFee = -removeIt->GetModifiedFee(); + int modifySigOps = -removeIt->GetSigOpCount(); + for (const txiter& dit : setDescendants) { + mapTx.modify(dit, update_ancestor_state(modifySize, modifyFee, -1, modifySigOps)); + } + } + } for (const txiter& removeIt : entriesToRemove) { setEntries setAncestors; const CTxMemPoolEntry &entry = *removeIt; @@ -286,10 +306,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) // transactions as the set of things to update for removal. CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); // Note that UpdateAncestorsOf severs the child links that point to - // removeIt in the entries for the parents of removeIt. This is - // fine since we don't need to use the mempool children of any entries - // to walk back over our ancestors (but we do need the mempool - // parents!) + // removeIt in the entries for the parents of removeIt. UpdateAncestorsOf(false, removeIt, setAncestors); } // After updating all the ancestor sizes, we can now sever the link between each @@ -300,23 +317,24 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) } } -void CTxMemPoolEntry::SetDirty() +void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) { - nCountWithDescendants = 0; - nSizeWithDescendants = nTxSize; - nFeesWithDescendants = nFee; + nSizeWithDescendants += modifySize; + assert(int64_t(nSizeWithDescendants) > 0); + nModFeesWithDescendants += modifyFee; + nCountWithDescendants += modifyCount; + assert(int64_t(nCountWithDescendants) > 0); } -void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) +void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps) { - if (!IsDirty()) { - nSizeWithDescendants += modifySize; - assert(int64_t(nSizeWithDescendants) > 0); - nFeesWithDescendants += modifyFee; - assert(nFeesWithDescendants >= 0); - nCountWithDescendants += modifyCount; - assert(int64_t(nCountWithDescendants) > 0); - } + nSizeWithAncestors += modifySize; + assert(int64_t(nSizeWithAncestors) > 0); + nModFeesWithAncestors += modifyFee; + nCountWithAncestors += modifyCount; + assert(int64_t(nCountWithAncestors) > 0); + nSigOpCountWithAncestors += modifySigOps; + assert(int(nSigOpCountWithAncestors) >= 0); } CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) : @@ -366,6 +384,17 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, indexed_transaction_set::iterator newit = mapTx.insert(entry).first; mapLinks.insert(make_pair(newit, TxLinks())); + // Update transaction for any feeDelta created by PrioritiseTransaction + // TODO: refactor so that the fee delta is calculated before inserting + // into mapTx. + std::map >::const_iterator pos = mapDeltas.find(hash); + if (pos != mapDeltas.end()) { + const std::pair &deltas = pos->second; + if (deltas.second) { + mapTx.modify(newit, update_fee_delta(deltas.second)); + } + } + // Update cachedInnerUsage to include contained transaction's usage. // (When we update the entry for in-mempool parents, memory usage will be // further updated.) @@ -375,7 +404,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, std::set setParentTransactions; if(!tx.HasZerocoinSpendInputs()) { for (unsigned int i = 0; i < tx.vin.size(); i++) { - mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); + mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit->GetSharedTx())); setParentTransactions.insert(tx.vin[i].prevout.hash); } } @@ -394,15 +423,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, } } UpdateAncestorsOf(true, newit, setAncestors); - - // Update transaction's score for any feeDelta created by PrioritiseTransaction - std::map >::const_iterator pos = mapDeltas.find(hash); - if (pos != mapDeltas.end()) { - const std::pair &deltas = pos->second; - if (deltas.second) { - mapTx.modify(newit, update_fee_delta(deltas.second)); - } - } + UpdateEntryForAncestors(newit, setAncestors); // Save spent nullifiers if (tx.IsShieldedTx()) { @@ -468,7 +489,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants } } -void CTxMemPool::remove(const CTransaction& origTx, std::list& removed, bool fRecursive) +void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vector* removed) { // Remove transaction from memory pool { @@ -477,32 +498,30 @@ void CTxMemPool::remove(const CTransaction& origTx, std::list& txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { txToRemove.insert(origit); - } else if (fRecursive) { - // If recursively removing but origTx isn't in the mempool + } else { + // When recursively removing but origTx isn't in the mempool // be sure to remove any children that are in the pool. This can // happen during chain re-orgs if origTx isn't re-accepted into // the mempool for any reason. for (unsigned int i = 0; i < origTx.vout.size(); i++) { - std::map::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); + auto it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); if (it == mapNextTx.end()) continue; - txiter nextit = mapTx.find(it->second.ptx->GetHash()); + txiter nextit = mapTx.find(it->second->GetHash()); assert(nextit != mapTx.end()); txToRemove.insert(nextit); } } setEntries setAllRemoves; - if (fRecursive) { - for (const txiter& it : txToRemove) { - CalculateDescendants(it, setAllRemoves); - } - } else { - setAllRemoves.swap(txToRemove); + for (const txiter& it : txToRemove) { + CalculateDescendants(it, setAllRemoves); } - for (const txiter& it : setAllRemoves) { - removed.emplace_back(it->GetSharedTx()); + if (removed) { + for (const txiter& it : setAllRemoves) { + removed->emplace_back(it->GetSharedTx()); + } } - RemoveStaged(setAllRemoves); + RemoveStaged(setAllRemoves, false); } } @@ -511,11 +530,11 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem AssertLockHeld(cs_main); // Remove transactions spending a coinbase which are now immature and no-longer-final transactions LOCK(cs); - std::list transactionsToRemove; + setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); if (!CheckFinalTx(tx, flags)) { - transactionsToRemove.push_back(tx); + txToRemove.insert(it); } else if (it->GetSpendsCoinbaseOrCoinstake()) { for (const CTxIn& txin : tx.vin) { indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); @@ -524,16 +543,17 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem const Coin &coin = pcoins->AccessCoin(txin.prevout); if (nCheckFrequency != 0) assert(!coin.IsSpent()); if (coin.IsSpent() || ((coin.IsCoinBase() || coin.IsCoinStake()) && ((signed long)nMemPoolHeight) - coin.nHeight < Params().GetConsensus().nCoinbaseMaturity)) { - transactionsToRemove.push_back(tx); + txToRemove.insert(it); break; } } } } - for (const CTransaction& tx : transactionsToRemove) { - std::list removed; - remove(tx, removed, true); + setEntries setAllRemoves; + for (txiter it : txToRemove) { + CalculateDescendants(it, setAllRemoves); } + RemoveStaged(setAllRemoves, false); } void CTxMemPool::removeWithAnchor(const uint256& invalidRoot) @@ -556,22 +576,21 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot) } } for (const CTransaction& tx : transactionsToRemove) { - std::list removed; - remove(tx, removed, true); + removeRecursive(tx); } } -void CTxMemPool::removeConflicts(const CTransaction& tx, std::list& removed) +void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector* removed) { // Remove transactions which depend on inputs of tx, recursively std::list result; LOCK(cs); for (const CTxIn& txin : tx.vin) { - std::map::iterator it = mapNextTx.find(txin.prevout); + auto it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { - const CTransaction& txConflict = *it->second.ptx; + const CTransaction& txConflict = *it->second; if (txConflict != tx) { - remove(txConflict, removed, true); + removeRecursive(txConflict, removed); } } } @@ -582,7 +601,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::listsecond; if (txConflict != tx) { - remove(txConflict, removed, true); + removeRecursive(txConflict, removed); } } } @@ -592,7 +611,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::list& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate) +void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, + std::vector* conflicts, bool fCurrentEstimate) { LOCK(cs); std::vector entries; @@ -603,8 +623,12 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne entries.push_back(*i); } for (const auto& tx : vtx) { - std::list dummy; - remove(*tx, dummy, false); + txiter it = mapTx.find(tx->GetHash()); + if (it != mapTx.end()) { + setEntries stage; + stage.insert(it); + RemoveStaged(stage, true); + } removeConflicts(*tx, conflicts); ClearPrioritisation(tx->GetHash()); } @@ -662,6 +686,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const innerUsage += memusage::DynamicUsage(links.parents) + memusage::DynamicUsage(links.children); bool fDependsWait = false; setEntries setParentCheck; + int64_t parentSizes = 0; + unsigned int parentSigOpCount = 0; bool fHasZerocoinSpends = false; for (const CTxIn& txin : tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. @@ -670,7 +696,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; - setParentCheck.insert(it2); + if (setParentCheck.insert(it2).second) { + parentSizes += it2->GetTxSize(); + parentSigOpCount += it2->GetSigOpCount(); + } } else if(!txin.IsZerocoinSpend() && !txin.IsZerocoinPublicSpend()) { assert(pcoins->HaveCoin(txin.prevout)); } else { @@ -678,10 +707,10 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const } // Check whether its inputs are marked in mapNextTx. if(!fHasZerocoinSpends) { - std::map::const_iterator it3 = mapNextTx.find(txin.prevout); + auto it3 = mapNextTx.find(txin.prevout); assert(it3 != mapNextTx.end()); - assert(it3->second.ptx == &tx); - assert(it3->second.n == i); + assert(it3->first == &txin.prevout); + assert(*it3->second == tx); } else { fDependsWait=false; } @@ -696,32 +725,43 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const } } assert(setParentCheck == GetMemPoolParents(it)); + // Verify ancestor state is correct. + setEntries setAncestors; + uint64_t nNoLimit = std::numeric_limits::max(); + std::string dummy; + CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + uint64_t nCountCheck = setAncestors.size() + 1; + uint64_t nSizeCheck = it->GetTxSize(); + CAmount nFeesCheck = it->GetModifiedFee(); + unsigned int nSigOpCheck = it->GetSigOpCount(); + + for (const txiter& ancestorIt : setAncestors) { + nSizeCheck += ancestorIt->GetTxSize(); + nFeesCheck += ancestorIt->GetModifiedFee(); + nSigOpCheck += ancestorIt->GetSigOpCount(); + } + + assert(it->GetCountWithAncestors() == nCountCheck); + assert(it->GetSizeWithAncestors() == nSizeCheck); + assert(it->GetSigOpCountWithAncestors() == nSigOpCheck); + assert(it->GetModFeesWithAncestors() == nFeesCheck); + // Check children against mapNextTx if (!fHasZerocoinSpends) { CTxMemPool::setEntries setChildrenCheck; - std::map::const_iterator iter = mapNextTx.lower_bound(COutPoint(tx.GetHash(), 0)); + auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); int64_t childSizes = 0; - CAmount childFees = 0; - for (; iter != mapNextTx.end() && iter->first.hash == tx.GetHash(); ++iter) { - txiter childit = mapTx.find(iter->second.ptx->GetHash()); + for (; iter != mapNextTx.end() && iter->first->hash == tx.GetHash(); ++iter) { + txiter childit = mapTx.find(iter->second->GetHash()); assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions if (setChildrenCheck.insert(childit).second) { childSizes += childit->GetTxSize(); - childFees += childit->GetFee(); } } assert(setChildrenCheck == GetMemPoolChildren(it)); - // Also check to make sure size/fees is greater than sum with immediate children. + // Also check to make sure size is greater than sum with immediate children. // just a sanity check, not definitive that this calc is correct... - // also check that the size is less than the size of the entire mempool. - if (!it->IsDirty()) { - assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); - assert(it->GetFeesWithDescendants() >= childFees + it->GetFee()); - } else { - assert(it->GetSizeWithDescendants() == it->GetTxSize()); - assert(it->GetFeesWithDescendants() == it->GetFee()); - } - assert(it->GetFeesWithDescendants() >= 0); + assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); } if (fDependsWait) @@ -750,14 +790,12 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const stepsSinceLastRemove = 0; } } - for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { - const uint256& hash = it->second.ptx->GetHash(); + for (auto it = mapNextTx.cbegin(); it != mapNextTx.cend(); it++) { + const uint256& hash = it->second->GetHash(); indexed_transaction_set::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->GetTx(); + const CTransactionRef& tx = it2->GetSharedTx(); assert(it2 != mapTx.end()); - assert(&tx == it->second.ptx); - assert(tx.vin.size() > it->second.n); - assert(it->first == it->second.ptx->vin[it->second.n].prevout); + assert(tx == it->second); } // Consistency check for sapling nullifiers @@ -778,14 +816,80 @@ void CTxMemPool::checkNullifiers() const } } +bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb) +{ + LOCK(cs); + indexed_transaction_set::const_iterator i = mapTx.find(hasha); + if (i == mapTx.end()) return false; + indexed_transaction_set::const_iterator j = mapTx.find(hashb); + if (j == mapTx.end()) return true; + uint64_t counta = i->GetCountWithAncestors(); + uint64_t countb = j->GetCountWithAncestors(); + if (counta == countb) { + return CompareTxMemPoolEntryByScore()(*i, *j); + } + return counta < countb; +} + +namespace { + class DepthAndScoreComparator + { + public: + bool operator()(const CTxMemPool::indexed_transaction_set::const_iterator& a, const CTxMemPool::indexed_transaction_set::const_iterator& b) const + { + uint64_t counta = a->GetCountWithAncestors(); + uint64_t countb = b->GetCountWithAncestors(); + if (counta == countb) { + return CompareTxMemPoolEntryByScore()(*a, *b); + } + return counta < countb; + } + }; +} + +std::vector CTxMemPool::GetSortedDepthAndScore() const +{ + std::vector iters; + AssertLockHeld(cs); + + iters.reserve(mapTx.size()); + + for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) { + iters.push_back(mi); + } + std::sort(iters.begin(), iters.end(), DepthAndScoreComparator()); + return iters; +} + void CTxMemPool::queryHashes(std::vector& vtxid) { + LOCK(cs); + auto iters = GetSortedDepthAndScore(); + vtxid.clear(); + vtxid.reserve(mapTx.size()); + + for (auto it : iters) { + vtxid.emplace_back(it->GetTx().GetHash()); + } +} + +static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) { + return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize()), it->GetModifiedFee() - it->GetFee()}; +} +std::vector CTxMemPool::infoAll() const +{ LOCK(cs); - vtxid.reserve(mapTx.size()); - for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) - vtxid.push_back(mi->GetTx().GetHash()); + auto iters = GetSortedDepthAndScore(); + + std::vector ret; + ret.reserve(mapTx.size()); + for (auto it : iters) { + ret.emplace_back(GetInfo(it)); + } + + return ret; } void CTxMemPool::getTransactions(std::set& setTxid) @@ -797,13 +901,22 @@ void CTxMemPool::getTransactions(std::set& setTxid) setTxid.insert(mi->GetTx().GetHash()); } -bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const +std::shared_ptr CTxMemPool::get(const uint256& hash) const { LOCK(cs); indexed_transaction_set::const_iterator i = mapTx.find(hash); - if (i == mapTx.end()) return false; - result = i->GetTx(); - return true; + if (i == mapTx.end()) + return nullptr; + return i->GetSharedTx(); +} + +TxMempoolInfo CTxMemPool::info(const uint256& hash) const +{ + LOCK(cs); + indexed_transaction_set::const_iterator i = mapTx.find(hash); + if (i == mapTx.end()) + return TxMempoolInfo(); + return GetInfo(i); } CFeeRate CTxMemPool::estimateFee(int nBlocks) const @@ -868,6 +981,14 @@ void CTxMemPool::PrioritiseTransaction(const uint256 hash, const std::string str txiter it = mapTx.find(hash); if (it != mapTx.end()) { mapTx.modify(it, update_fee_delta(deltas.second)); + // Now update all ancestors' modified fees with descendants + setEntries setAncestors; + uint64_t nNoLimit = std::numeric_limits::max(); + std::string dummy; + CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); + for (const txiter& ancestorIt : setAncestors) { + mapTx.modify(ancestorIt, update_descendant_state(0, nFeeDelta, 0)); + } } } LogPrintf("PrioritiseTransaction: %s priority += %f, fee += %d\n", strHash, dPriorityDelta, FormatMoney(nFeeDelta)); @@ -914,10 +1035,10 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint& outpoint, Coin& coin) const // If an entry in the mempool exists, always return that one, as it's guaranteed to never // conflict with the underlying cache, and it cannot have pruned entries (as it contains full) // transactions. First checking the underlying cache risks returning a pruned entry instead. - CTransaction tx; - if (mempool.lookup(outpoint.hash, tx)) { - if (outpoint.n < tx.vout.size()) { - coin = Coin(tx.vout[outpoint.n], MEMPOOL_HEIGHT, false, false); + CTransactionRef ptx = mempool.get(outpoint.hash); + if (ptx) { + if (outpoint.n < ptx->vout.size()) { + coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false, false); return true; } else { return false; @@ -939,9 +1060,9 @@ bool CCoinsViewMemPool::GetNullifier(const uint256& nullifier) const size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for + // 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) + 12 * sizeof(void*)) * mapTx.size() + + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + @@ -949,10 +1070,10 @@ size_t CTxMemPool::DynamicMemoryUsage() const memusage::DynamicUsage(mapSaplingNullifiers); } -void CTxMemPool::RemoveStaged(setEntries &stage) +void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants) { AssertLockHeld(cs); - UpdateForRemoveFromMempool(stage); + UpdateForRemoveFromMempool(stage, updateDescendants); for (const txiter& it : stage) { removeUnchecked(it); } @@ -971,7 +1092,7 @@ int CTxMemPool::Expire(int64_t time) for (const txiter& removeit : toremove) { CalculateDescendants(removeit, stage); } - RemoveStaged(stage); + RemoveStaged(stage, false); return stage.size(); } @@ -1067,7 +1188,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends // "minimum reasonable fee rate" (ie some value under which we consider txn // to have 0 fee). This way, we don't allow txn to enter mempool with feerate // equal to txn which were removed with no block in between. - CFeeRate removed(it->GetFeesWithDescendants(), it->GetSizeWithDescendants()); + CFeeRate removed(it->GetModFeesWithDescendants(), it->GetSizeWithDescendants()); removed += minReasonableRelayFee; trackPackageRemoved(removed); maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed); @@ -1082,7 +1203,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends for (txiter it: stage) txn.push_back(it->GetTx()); } - RemoveStaged(stage); + RemoveStaged(stage, false); if (pvNoSpendsRemaining) { for (const CTransaction& tx: txn) { for (const CTxIn& txin: tx.vin) { diff --git a/src/txmempool.h b/src/txmempool.h index 898533c3f149..dca6a3939405 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -13,6 +13,7 @@ #include "amount.h" #include "coins.h" +#include "indirectmap.h" #include "policy/feerate.h" #include "primitives/transaction.h" #include "sync.h" @@ -49,12 +50,12 @@ class CTxMemPool; * ("descendant" transactions). * * When a new entry is added to the mempool, we update the descendant state - * (nCountWithDescendants, nSizeWithDescendants, and nFeesWithDescendants) for + * (nCountWithDescendants, nSizeWithDescendants, and nModFeesWithDescendants) for * all ancestors of the newly added transaction. * * If updating the descendant state is skipped, we can mark the entry as - * "dirty", and set nSizeWithDescendants/nFeesWithDescendants to equal nTxSize/ - * nTxFee. (This can potentially happen during a reorg, where we limit the + * "dirty", and set nSizeWithDescendants/nModFeesWithDescendants to equal nTxSize/ + * nFee+feeDelta. (This can potentially happen during a reorg, where we limit the * amount of work we're willing to do to avoid consuming too much CPU.) * */ @@ -81,11 +82,17 @@ class CTxMemPoolEntry // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these // descendants as well. if nCountWithDescendants is 0, treat this entry as - // dirty, and nSizeWithDescendants and nFeesWithDescendants will not be + // dirty, and nSizeWithDescendants and nModFeesWithDescendants will not be // correct. uint64_t nCountWithDescendants; //! number of descendant transactions uint64_t nSizeWithDescendants; //! ... and size - CAmount nFeesWithDescendants; //! ... and total fees (all including us) + CAmount nModFeesWithDescendants; //! ... and total fees (all including us) + + // Analogous statistics for ancestor transactions + uint64_t nCountWithAncestors; + uint64_t nSizeWithAncestors; + CAmount nModFeesWithAncestors; + unsigned int nSigOpCountWithAncestors; public: CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, @@ -113,22 +120,23 @@ class CTxMemPoolEntry size_t DynamicMemoryUsage() const { return nUsageSize; } // Adjusts the descendant state, if this entry is not dirty. - void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); - // Updates the fee delta used for mining priority score + void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); + // Adjusts the ancestor state + void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps); + // Updates the fee delta used for mining priority score, and the + // modified fees with descendants. void UpdateFeeDelta(int64_t feeDelta); - /** We can set the entry to be dirty if doing the full calculation of in- - * mempool descendants will be too expensive, which can potentially happen - * when re-adding transactions from a block back to the mempool. - */ - void SetDirty(); - bool IsDirty() const { return nCountWithDescendants == 0; } - uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } - CAmount GetFeesWithDescendants() const { return nFeesWithDescendants; } + CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } bool GetSpendsCoinbaseOrCoinstake() const { return spendsCoinbaseOrCoinstake; } + + uint64_t GetCountWithAncestors() const { return nCountWithAncestors; } + uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } + unsigned int GetSigOpCountWithAncestors() const { return nSigOpCountWithAncestors; } }; // Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. @@ -139,7 +147,7 @@ struct update_descendant_state {} void operator() (CTxMemPoolEntry &e) - { e.UpdateState(modifySize, modifyFee, modifyCount); } + { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); } private: int64_t modifySize; @@ -147,10 +155,20 @@ struct update_descendant_state int64_t modifyCount; }; -struct set_dirty +struct update_ancestor_state { + update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int _modifySigOps) : + modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOps(_modifySigOps) + {} + void operator() (CTxMemPoolEntry &e) - { e.SetDirty(); } + { e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOps); } + +private: + int64_t modifySize; + CAmount modifyFee; + int64_t modifyCount; + int modifySigOps; }; struct update_fee_delta @@ -173,27 +191,27 @@ struct mempoolentry_txid } }; -/** \class CompareTxMemPoolEntryByFee +/** \class CompareTxMemPoolEntryByDescendantScore * - * Sort an entry by max(feerate of entry's tx, feerate with all descendants). + * Sort an entry by max(score/size of entry's tx, score/size with all descendants). */ -class CompareTxMemPoolEntryByFee +class CompareTxMemPoolEntryByDescendantScore { public: bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { - bool fUseADescendants = UseDescendantFeeRate(a); - bool fUseBDescendants = UseDescendantFeeRate(b); + bool fUseADescendants = UseDescendantScore(a); + bool fUseBDescendants = UseDescendantScore(b); - double aFees = fUseADescendants ? a.GetFeesWithDescendants() : a.GetFee(); + double aModFee = fUseADescendants ? a.GetModFeesWithDescendants() : a.GetModifiedFee(); double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize(); - double bFees = fUseBDescendants ? b.GetFeesWithDescendants() : b.GetFee(); + double bModFee = fUseBDescendants ? b.GetModFeesWithDescendants() : b.GetModifiedFee(); double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize(); // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). - double f1 = aFees * bSize; - double f2 = aSize * bFees; + double f1 = aModFee * bSize; + double f2 = aSize * bModFee; if (f1 == f2) { return a.GetTime() >= b.GetTime(); @@ -201,11 +219,11 @@ class CompareTxMemPoolEntryByFee return f1 < f2; } - // Calculate which feerate to use for an entry (avoiding division). - bool UseDescendantFeeRate(const CTxMemPoolEntry &a) const + // Calculate which score to use for an entry (avoiding division). + bool UseDescendantScore(const CTxMemPoolEntry &a) const { - double f1 = (double)a.GetFee() * a.GetSizeWithDescendants(); - double f2 = (double)a.GetFeesWithDescendants() * a.GetTxSize(); + double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants(); + double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize(); return f2 > f1; } }; @@ -237,33 +255,53 @@ class CompareTxMemPoolEntryByEntryTime } }; +class CompareTxMemPoolEntryByAncestorFee +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const + { + double aFees = a.GetModFeesWithAncestors(); + double aSize = a.GetSizeWithAncestors(); + + double bFees = b.GetModFeesWithAncestors(); + double bSize = b.GetSizeWithAncestors(); + + // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). + double f1 = aFees * bSize; + double f2 = aSize * bFees; + + if (f1 == f2) { + return a.GetTx().GetHash() < b.GetTx().GetHash(); + } + + return f1 > f2; + } +}; + // Multi_index tag names struct descendant_score {}; struct entry_time {}; struct mining_score {}; +struct ancestor_score {}; class CBlockPolicyEstimator; -/** An inpoint - a combination of a transaction and an index n into its vin */ -class CInPoint +/** + * Information about a mempool transaction. + */ +struct TxMempoolInfo { -public: - const CTransaction* ptx; - uint32_t n; + /** The transaction itself */ + CTransactionRef tx; - CInPoint() { SetNull(); } - CInPoint(const CTransaction* ptxIn, uint32_t nIn) - { - ptx = ptxIn; - n = nIn; - } - void SetNull() - { - ptx = NULL; - n = (uint32_t)-1; - } - bool IsNull() const { return (ptx == NULL && n == (uint32_t)-1); } - size_t DynamicMemoryUsage() const { return 0; } + /** Time the transaction entered the mempool. */ + int64_t nTime; + + /** Feerate of the transaction. */ + CFeeRate feeRate; + + /** The fee delta. */ + int64_t nFeeDelta; }; /** @@ -380,7 +418,7 @@ class CTxMemPool boost::multi_index::ordered_non_unique< boost::multi_index::tag, boost::multi_index::identity, - CompareTxMemPoolEntryByFee + CompareTxMemPoolEntryByDescendantScore >, // sorted by entry time boost::multi_index::ordered_non_unique< @@ -393,6 +431,12 @@ class CTxMemPool boost::multi_index::tag, boost::multi_index::identity, CompareTxMemPoolEntryByScore + >, + // sorted by fee rate with ancestors + boost::multi_index::ordered_non_unique< + boost::multi_index::tag, + boost::multi_index::identity, + CompareTxMemPoolEntryByAncestorFee > > > indexed_transaction_set; @@ -450,8 +494,10 @@ class CTxMemPool void UpdateParent(txiter entry, txiter parent, bool add); void UpdateChild(txiter entry, txiter child, bool add); + std::vector GetSortedDepthAndScore() const; + public: - std::map mapNextTx; + indirectmap mapNextTx; std::map > mapDeltas; /** Create a new CTxMemPool. @@ -477,13 +523,15 @@ class CTxMemPool // then invoke the second version. bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); - void remove(const CTransaction& tx, std::list& removed, bool fRecursive = false); + void removeRecursive(const CTransaction& tx, std::vector* removed = nullptr); void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags); void removeWithAnchor(const uint256& invalidRoot); - void removeConflicts(const CTransaction& tx, std::list& removed); - void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate = true); + void removeConflicts(const CTransaction& tx, std::vector* removed = nullptr); + void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, + std::vector* conflicts = nullptr, bool fCurrentEstimate = true); void clear(); void _clear(); // lock-free + bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); void queryHashes(std::vector& vtxid); void getTransactions(std::set& setTxid); bool isSpent(const COutPoint& outpoint); @@ -504,8 +552,12 @@ class CTxMemPool /** Remove a set of transactions from the mempool. * If a transaction is in this set, then all in-mempool descendants must - * also be in the set.*/ - void RemoveStaged(setEntries &stage); + * also be in the set, unless this transaction is being removed for being + * in a block. + * Set updateDescendants to true when removing a tx that was in a block, so + * that any in-mempool descendants have their ancestor state updated. + */ + void RemoveStaged(setEntries &stage, bool updateDescendants); /** When adding transactions from a disconnected block back to the mempool, * new mempool entries may have children in the mempool (which is generally @@ -528,7 +580,7 @@ class CTxMemPool * fSearchForParents = whether to search a tx's vin for in-mempool parents, or * look up parents from mapLinks. Must be true for entries not in the mempool */ - bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true); + bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true) const; /** The minimum fee to get into the mempool, which may itself not be enough * for larger-sized transactions. @@ -571,7 +623,9 @@ class CTxMemPool return (it != mapTx.end() && outpoint.n < it->GetTx().vout.size()); } - bool lookup(uint256 hash, CTransaction& result) const; + CTransactionRef get(const uint256& hash) const; + TxMempoolInfo info(const uint256& hash) const; + std::vector infoAll() const; /** Estimate fee rate needed to get into the next nBlocks * If no answer can be given at nBlocks, return an estimate @@ -607,21 +661,21 @@ class CTxMemPool * updated and hence their state is already reflected in the parent * state). * - * If updating an entry requires looking at more than maxDescendantsToVisit - * transactions, outside of the ones in setExclude, then give up. - * * cachedDescendants will be updated with the descendants of the transaction * being updated, so that future invocations don't need to walk the * same transaction again, if encountered in another transaction chain. */ - bool UpdateForDescendants(txiter updateIt, - int maxDescendantsToVisit, + void UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set &setExclude); /** Update ancestors of hash to add/remove it as a descendant transaction. */ void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors); - /** For each transaction being removed, update ancestors and any direct children. */ - void UpdateForRemoveFromMempool(const setEntries &entriesToRemove); + /** Set ancestor state for an entry */ + void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors); + /** For each transaction being removed, update ancestors and any direct children. + * If updateDescendants is true, then also update in-mempool descendants' + * ancestor state. */ + void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants); /** Sever link between specified transaction and direct children. */ void UpdateChildrenForRemoval(txiter entry); /** Populate setDescendants with all in-mempool descendants of hash. diff --git a/src/validation.cpp b/src/validation.cpp index 43f069db3155..b2cb3df078ec 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -309,7 +309,7 @@ std::string FormatStateMessage(const CValidationState &state) } bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransactionRef& _tx, bool fLimitFree, - bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees, + bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees, std::vector& coins_to_uncache) { AssertLockHeld(cs_main); @@ -495,7 +495,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C } } - CTxMemPoolEntry entry(_tx, nFees, GetTime(), dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps); + CTxMemPoolEntry entry(_tx, nFees, nAcceptTime, dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps); unsigned int nSize = entry.GetTxSize(); // Don't accept it if it can't get into a block @@ -589,6 +589,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); } + // todo: pool.removeStaged for all conflicting entries // Store transaction in memory pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload()); @@ -610,11 +611,11 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return true; } -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef& tx, bool fLimitFree, - bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool fIgnoreFees) +bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef& tx, bool fLimitFree, + bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool fIgnoreFees) { std::vector coins_to_uncache; - bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, fOverrideMempoolLimit, fRejectAbsurdFee, fIgnoreFees, coins_to_uncache); + bool res = AcceptToMemoryPoolWorker(pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, fOverrideMempoolLimit, fRejectAbsurdFee, fIgnoreFees, coins_to_uncache); if (!res) { for (const COutPoint& outpoint: coins_to_uncache) pcoinsTip->Uncache(outpoint); @@ -625,6 +626,13 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return res; } +bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransactionRef& tx, + bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit, + bool fRejectInsaneFee, bool ignoreFees) +{ + return AcceptToMemoryPoolWithTime(pool, state, tx, fLimitFree, pfMissingInputs, GetTime(), fOverrideMempoolLimit, fRejectInsaneFee, ignoreFees); +} + bool GetOutput(const uint256& hash, unsigned int index, CValidationState& state, CTxOut& out) { CTransaction txPrev; @@ -647,7 +655,10 @@ bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock LOCK(cs_main); if (!blockIndex) { - if (mempool.lookup(hash, txOut)) { + + CTransactionRef ptx = mempool.get(hash); + if (ptx) { + txOut = *ptx; return true; } @@ -1962,10 +1973,9 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara std::vector vHashUpdate; for (const auto& tx : block.vtx) { // ignore validation errors in resurrected transactions - std::list removed; CValidationState stateDummy; if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) { - mempool.remove(*tx, removed, true); + mempool.removeRecursive(*tx); } else if (mempool.exists(tx->GetHash())) { vHashUpdate.push_back(tx->GetHash()); } @@ -2017,7 +2027,7 @@ static int64_t nTimePostConnect = 0; * applied to the UTXO state as a part of a single ActivateBestChainStep call. */ struct ConnectTrace { - std::list txConflicted; + std::vector txConflicted; std::vector > > blocksConnected; }; @@ -2082,7 +2092,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool. - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, connectTrace.txConflicted, !IsInitialBlockDownload()); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew); // Update MN manager cache @@ -4086,6 +4096,120 @@ std::string CBlockFileInfo::ToString() const return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, DateTimeStrFormat("%Y-%m-%d", nTimeFirst), DateTimeStrFormat("%Y-%m-%d", nTimeLast)); } +static const uint64_t MEMPOOL_DUMP_VERSION = 1; + +bool LoadMempool(void) +{ + int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; + FILE* filestr = fopen((GetDataDir() / "mempool.dat").string().c_str(), "r"); + 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 skipped = 0; + int64_t failed = 0; + int64_t nNow = GetTime(); + + try { + uint64_t version; + file >> version; + if (version != MEMPOOL_DUMP_VERSION) { + return false; + } + uint64_t num; + file >> num; + double prioritydummy = 0; + while (num--) { + CTransactionRef tx; + int64_t nTime; + int64_t nFeeDelta; + file >> tx; + file >> nTime; + file >> nFeeDelta; + + CAmount amountdelta = nFeeDelta; + if (amountdelta) { + mempool.PrioritiseTransaction(tx->GetHash(), tx->GetHash().ToString(), prioritydummy, amountdelta); + } + CValidationState state; + if (nTime + nExpiryTimeout > nNow) { + LOCK(cs_main); + AcceptToMemoryPoolWithTime(mempool, state, tx, true, NULL, nTime); + if (state.IsValid()) { + ++count; + } else { + ++failed; + } + } else { + ++skipped; + } + if (ShutdownRequested()) + return false; + } + std::map mapDeltas; + file >> mapDeltas; + + for (const auto& i : mapDeltas) { + mempool.PrioritiseTransaction(i.first, i.first.ToString(), prioritydummy, i.second); + } + } 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 successes, %i failed, %i expired\n", count, failed, skipped); + return true; +} + +void DumpMempool(void) +{ + int64_t start = GetTimeMicros(); + + std::map mapDeltas; + std::vector vinfo; + + { + LOCK(mempool.cs); + for (const auto &i : mempool.mapDeltas) { + mapDeltas[i.first] = i.second.first; + } + vinfo = mempool.infoAll(); + } + + int64_t mid = GetTimeMicros(); + + try { + FILE* filestr = fopen((GetDataDir() / "mempool.dat.new").string().c_str(), "w"); + if (!filestr) { + return; + } + + 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)i.nTime; + file << (int64_t)i.nFeeDelta; + mapDeltas.erase(i.tx->GetHash()); + } + + file << mapDeltas; + FileCommit(file.Get()); + file.fclose(); + RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat"); + int64_t last = GetTimeMicros(); + LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*0.000001, (last-mid)*0.000001); + } catch (const std::exception& e) { + LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + } +} class CMainCleanup { diff --git a/src/validation.h b/src/validation.h index 98965ee455f0..b33810d6714f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -57,6 +57,8 @@ static const unsigned int DEFAULT_ANCESTOR_LIMIT = 25; static const unsigned int DEFAULT_ANCESTOR_SIZE_LIMIT = 101; /** Default for -banscore */ static const int DEFAULT_BANSCORE_THRESHOLD = 100; +/** Default for -persistmempool */ +static const bool DEFAULT_PERSIST_MEMPOOL = true; /** Default for -limitdescendantcount, max number of in-mempool descendants */ static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25; /** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */ @@ -107,9 +109,6 @@ static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60; /** Average delay between peer address broadcasts in seconds. */ static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30; -/** Average delay between trickled inventory broadcasts in seconds. - * Blocks, whitelisted receivers, and a random 25% of transactions bypass this. */ -static const unsigned int AVG_INVENTORY_BROADCAST_INTERVAL = 5; /** Default multiplier used in the computation for shielded txes min fee */ static const unsigned int DEFAULT_SHIELDEDTXFEE_K = 100; @@ -219,6 +218,11 @@ void FlushStateToDisk(); /** (try to) add transaction to memory pool **/ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransactionRef& tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit = false, bool fRejectInsaneFee = false, bool ignoreFees = false); +/** (try to) add transaction to memory pool with a specified acceptance time **/ +bool AcceptToMemoryPoolWithTime(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree, + bool* pfMissingInputs, int64_t nAcceptTime, bool fOverrideMempoolLimit = false, + bool fRejectInsaneFee = false, bool ignoreFees = false); + /** Convert CValidationState to a human-readable message for logging */ std::string FormatStateMessage(const CValidationState &state); @@ -395,4 +399,10 @@ static const unsigned int REJECT_ALREADY_KNOWN = 0x101; /** Transaction conflicts with a transaction already known */ static const unsigned int REJECT_CONFLICT = 0x102; +/** Dump the mempool to disk. */ +void DumpMempool(); + +/** Load the mempool from disk. */ +bool LoadMempool(); + #endif // BITCOIN_MAIN_H diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index b3fbec58bc1d..14f9df526527 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -69,17 +69,41 @@ def run_test(self): for x in reversed(chain): assert_equal(mempool[x]['descendantcount'], descendant_count) descendant_fees += mempool[x]['fee'] + assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']) assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees) descendant_size += mempool[x]['size'] assert_equal(mempool[x]['descendantsize'], descendant_size) descendant_count += 1 + # Check that descendant modified fees includes fee deltas from + # prioritisetransaction + self.nodes[0].prioritisetransaction(chain[-1], 0, 1000) + mempool = self.nodes[0].getrawmempool(True) + + descendant_fees = 0 + for x in reversed(chain): + descendant_fees += mempool[x]['fee'] + assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+1000) + # Adding one more transaction on to the chain should fail. try: self.chain_transaction(self.nodes[0],txid, vout, value, fee, 1) except JSONRPCException as e: self.log.info("too-long-ancestor-chain successfully rejected") + # Check that prioritising a tx before it's added to the mempool works + self.nodes[0].generate(1) + self.nodes[0].prioritisetransaction(chain[-1], 0, 2000) + self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) + mempool = self.nodes[0].getrawmempool(True) + + descendant_fees = 0 + for x in reversed(chain): + descendant_fees += mempool[x]['fee'] + if (x == chain[-1]): + assert_equal(mempool[x]['modifiedfee'], mempool[x]['fee']+satoshi_round(0.00002)) + assert_equal(mempool[x]['descendantfees'], SATOSHIS*descendant_fees+2000) + # TODO: check that node1's mempool is as expected # TODO: test ancestor size limits diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 83017a3f547a..52fa6d3e57d3 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -1,16 +1,16 @@ #!/usr/bin/env python3 # Copyright (c) 2014-2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. +# file COPYING or https://www.opensource.org/licenses/mit-license.php. """Test mempool persistence. By default, bitcoind will dump mempool on shutdown and then reload it on startup. This can be overridden with -the -persistmempool=0 command line option. +the -persistmempool=false command line option. Test is as follows: - - start node0, node1 and node2. node1 has -persistmempool=0 + - start node0, node1 and node2. node1 has -persistmempool=false - create 5 transactions on node2 to its own address. Note that these are not sent to node0 or node1 addresses because we don't want them to be saved in the wallet. @@ -20,23 +20,16 @@ in its mempool. Shutdown node0. This tests that by default the mempool is persistent. - startup node1. Verify that its mempool is empty. Shutdown node1. - This tests that with -persistmempool=0, the mempool is not + This tests that with -persistmempool=false, the mempool is not dumped to disk when the node is shut down. - - Restart node0 with -persistmempool=0. Verify that its mempool is - empty. Shutdown node0. This tests that with -persistmempool=0, + - Restart node0 with -persistmempool=false. Verify that its mempool is + empty. Shutdown node0. This tests that with -persistmempool=false, the mempool is not loaded from disk on start up. - - Restart node0 with -persistmempool. Verify that it has 5 - transactions in its mempool. This tests that -persistmempool=0 + - Restart node0 with -persistmempool=true. Verify that it has 5 + transactions in its mempool. This tests that -persistmempool=false does not overwrite a previously valid mempool stored on disk. - - Remove node0 mempool.dat and verify savemempool RPC recreates it - and verify that node1 can load it and has 5 transaction in its - mempool. - - Verify that savemempool throws when the RPC is called if - node1 can't write to disk. """ -import os -import time from test_framework.test_framework import PivxTestFramework from test_framework.util import * @@ -64,19 +57,19 @@ def run_test(self): assert_equal(len(self.nodes[0].getrawmempool()), 5) assert_equal(len(self.nodes[1].getrawmempool()), 5) - self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.") + self.log.debug("Stop-start node0 and node1. Verify that node0 has the transactions in its mempool and node1 does not.") self.stop_nodes() self.start_node(1) # Give this one a head-start, so we can be "extra-sure" that it didn't load anything later self.start_node(0) self.start_node(2) - # Give bitcoind a second to reload the mempool + # Give pivxd a second to reload the mempool wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5, timeout=1) wait_until(lambda: len(self.nodes[2].getrawmempool()) == 5, timeout=1) - # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now: + # The others loaded their mempool. If node_1 loaded anything, we'd probably notice by now: assert_equal(len(self.nodes[1].getrawmempool()), 0) # Verify accounting of mempool transactions after restart is correct - self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet + #self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet assert_equal(node2_balance, self.nodes[2].getbalance()) self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") @@ -91,27 +84,29 @@ def run_test(self): self.start_node(0) wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5) - mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat') - mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat') - self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") - os.remove(mempooldat0) - self.nodes[0].savemempool() - assert os.path.isfile(mempooldat0) - - self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions") - os.rename(mempooldat0, mempooldat1) - self.stop_nodes() - self.start_node(1, extra_args=[]) - wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5) - - self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") - # to test the exception we are setting bad permissions on a tmp file called mempool.dat.new - # which is an implementation detail that could change and break this test - mempooldotnew1 = mempooldat1 + '.new' - with os.fdopen(os.open(mempooldotnew1, os.O_CREAT, 0o000), 'w'): - pass - assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool) - os.remove(mempooldotnew1) + # Following code is ahead of our current repository state. Future back port. + + # mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat') + # mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat') + # self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") + # os.remove(mempooldat0) + # self.nodes[0].savemempool() + # assert os.path.isfile(mempooldat0) + # + # self.log.debug("Stop nodes, make node1 use mempool.dat from node0. Verify it has 5 transactions") + # os.rename(mempooldat0, mempooldat1) + # self.stop_nodes() + # self.start_node(1, extra_args=[]) + # wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5) + # + # self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") + # # to test the exception we are setting bad permissions on a tmp file called mempool.dat.new + # # which is an implementation detail that could change and break this test + # mempooldotnew1 = mempooldat1 + '.new' + # with os.fdopen(os.open(mempooldotnew1, os.O_CREAT, 0o000), 'w'): + # pass + # assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool) + # os.remove(mempooldotnew1) if __name__ == '__main__': MempoolPersistTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 5eef9e9cfdf1..96a19af45dca 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -88,6 +88,7 @@ 'feature_blockhashcache.py', # ~ 100 sec 'wallet_listtransactions.py', # ~ 97 sec 'mempool_reorg.py', # ~ 92 sec + 'mempool_persist.py', 'wallet_encryption.py', # ~ 89 sec 'wallet_keypool.py', # ~ 88 sec 'wallet_dump.py', # ~ 83 sec @@ -117,7 +118,6 @@ # 'mempool_limit.py', # We currently don't limit our mempool_reorg # 'interface_zmq.py', # 'rpc_getchaintips.py', - # 'mempool_persist.py', # 'rpc_users.py', # 'p2p_mempool.py', # 'mining_prioritisetransaction.py', diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 7af4781c7789..5a56b9973a69 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -207,7 +207,7 @@ def run_test(self): assert_equal(z_add, z_add_3) # Restart, zap, and check balance: 1 PIV * (NUM_HD_ADDS + NUM_SHIELD_ADDS) recovered from seed self.stop_node(1) - self.start_node(1, extra_args=self.extra_args[1] + ['-zapwallettxes']) + self.start_node(1, extra_args=self.extra_args[1] + ['-zapwallettxes=1']) assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + NUM_SHIELD_ADDS) if __name__ == '__main__':