diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 0b8327377..a01e99736 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -43,6 +43,7 @@ BASE_SCRIPTS= [ 'sprout_sapling_migration.py', 'remove_sprout_shielding.py', 'zcjoinsplitdoublespend.py', + 'mempool_packages.py', # vv Tests less than 2m vv 'zcjoinsplit.py', 'mergetoaddress_mixednotes.py', diff --git a/qa/rpc-tests/mempool_packages.py b/qa/rpc-tests/mempool_packages.py new file mode 100755 index 000000000..85fdade84 --- /dev/null +++ b/qa/rpc-tests/mempool_packages.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +# Copyright (c) 2014-2016 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# Test descendant package tracking code + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + ROUND_DOWN, + Decimal, + JSONRPCException, + assert_equal, + connect_nodes, + start_node, + sync_blocks, + sync_mempools, +) + +def satoshi_round(amount): + return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN) + +class MempoolPackagesTest(BitcoinTestFramework): + + def setup_network(self): + self.nodes = [] + self.nodes.append(start_node(0, self.options.tmpdir, ["-maxorphantx=1000", "-relaypriority=0"])) + self.is_network_split = False + self.sync_all() + + # Build a transaction that spends parent_txid:vout + # Return amount sent + def chain_transaction(self, parent_txid, vout, value, fee, num_outputs): + send_value = satoshi_round((value - fee)/num_outputs) + inputs = [ {'txid' : parent_txid, 'vout' : vout} ] + outputs = {} + for i in range(num_outputs): + outputs[self.nodes[0].getnewaddress()] = send_value + rawtx = self.nodes[0].createrawtransaction(inputs, outputs) + signedtx = self.nodes[0].signrawtransaction(rawtx) + txid = self.nodes[0].sendrawtransaction(signedtx['hex']) + fulltx = self.nodes[0].getrawtransaction(txid, 1) + assert(len(fulltx['vout']) == num_outputs) # make sure we didn't generate a change output + return (txid, send_value) + + def run_test(self): + ''' Mine some blocks and have them mature. ''' + self.nodes[0].generate(101) + utxo = self.nodes[0].listunspent(10) + txid = utxo[0]['txid'] + vout = utxo[0]['vout'] + value = utxo[0]['amount'] + + fee = Decimal("0.00005") + # 100 transactions off a confirmed tx should be fine + chain = [] + for i in range(100): + (txid, sent_value) = self.chain_transaction(txid, 0, value, fee, 1) + value = sent_value + chain.append(txid) + + # Check mempool has 100 transactions in it, and descendant + # count and fees should look correct + mempool = self.nodes[0].getrawmempool(True) + assert_equal(len(mempool), 100) + descendant_count = 1 + descendant_fees = 0 + descendant_size = 0 + SATOSHIS = 100000000 + + for x in reversed(chain): + assert_equal(mempool[x]['descendantcount'], descendant_count) + descendant_fees += 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 + + # Adding one more transaction on to the chain should fail. + try: + self.chain_transaction(txid, vout, value, fee, 1) + except JSONRPCException: + print("too-long-ancestor-chain successfully rejected") + + # TODO: test ancestor size limits + + # Now test descendant chain limits + txid = utxo[1]['txid'] + value = utxo[1]['amount'] + vout = utxo[1]['vout'] + + transaction_package = [] + # First create one parent tx with 10 children + (txid, sent_value) = self.chain_transaction(txid, vout, value, fee, 10) + parent_transaction = txid + for i in range(10): + transaction_package.append({'txid': txid, 'vout': i, 'amount': sent_value}) + + for i in range(1000): + utxo = transaction_package.pop(0) + try: + (txid, sent_value) = self.chain_transaction(utxo['txid'], utxo['vout'], utxo['amount'], fee, 10) + for j in range(10): + transaction_package.append({'txid': txid, 'vout': j, 'amount': sent_value}) + if i == 998: + mempool = self.nodes[0].getrawmempool(True) + assert_equal(mempool[parent_transaction]['descendantcount'], 1000) + except JSONRPCException as e: + print(e.error['message']) + assert_equal(i, 999) + print("tx that would create too large descendant package successfully rejected") + + # TODO: test descendant size limits + +if __name__ == '__main__': + MempoolPackagesTest().main() diff --git a/qa/rpc-tests/mergetoaddress_helper.py b/qa/rpc-tests/mergetoaddress_helper.py index 50f28c8fd..44f0351a5 100755 --- a/qa/rpc-tests/mergetoaddress_helper.py +++ b/qa/rpc-tests/mergetoaddress_helper.py @@ -42,14 +42,12 @@ class MergeToAddressHelper: initialize_chain_clean(test.options.tmpdir, 4) def setup_network(self, test, additional_args=[]): - args = ['-debug=zrpcunsafe'] + args = ['-debug=zrpcunsafe', '-limitancestorcount=%d' % self.utxos_to_generate] args += additional_args test.nodes = [] test.nodes.append(start_node(0, test.options.tmpdir, args)) test.nodes.append(start_node(1, test.options.tmpdir, args)) - args2 = ['-debug=zrpcunsafe'] - args2 += additional_args - test.nodes.append(start_node(2, test.options.tmpdir, args2)) + test.nodes.append(start_node(2, test.options.tmpdir, args)) connect_nodes_bi(test.nodes, 0, 1) connect_nodes_bi(test.nodes, 1, 2) connect_nodes_bi(test.nodes, 0, 2) diff --git a/qa/rpc-tests/prioritisetransaction.py b/qa/rpc-tests/prioritisetransaction.py index a4d215702..d7af3634a 100755 --- a/qa/rpc-tests/prioritisetransaction.py +++ b/qa/rpc-tests/prioritisetransaction.py @@ -26,8 +26,16 @@ class PrioritiseTransactionTest (BitcoinTestFramework): def setup_network(self, split=False): self.nodes = [] # Start nodes with tiny block size of 11kb - self.nodes.append(start_node(0, self.options.tmpdir, ["-blockprioritysize=7000", "-blockmaxsize=11000", "-maxorphantx=1000", "-relaypriority=true", "-printpriority=1"])) - self.nodes.append(start_node(1, self.options.tmpdir, ["-blockprioritysize=7000", "-blockmaxsize=11000", "-maxorphantx=1000", "-relaypriority=true", "-printpriority=1"])) + args = [ + "-blockprioritysize=7000", + "-blockmaxsize=11000", + "-maxorphantx=1000", + "-relaypriority=true", + "-printpriority=1", + "-limitancestorcount=900", + ] + self.nodes.append(start_node(0, self.options.tmpdir, args)) + self.nodes.append(start_node(1, self.options.tmpdir, args)) connect_nodes(self.nodes[1], 0) self.is_network_split=False self.sync_all() diff --git a/src/init.cpp b/src/init.cpp index 1072f64d5..546168637 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -449,6 +449,10 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-dropmessagestest=", "Randomly drop 1 of every network messages"); strUsage += HelpMessageOpt("-fuzzmessagestest=", "Randomly fuzz 1 of every network messages"); strUsage += HelpMessageOpt("-stopafterblockimport", strprintf("Stop running after importing blocks from disk (default: %u)", DEFAULT_STOPAFTERBLOCKIMPORT)); + strUsage += HelpMessageOpt("-limitancestorcount=", strprintf("Do not accept transactions if number of in-mempool ancestors is or more (default: %u)", DEFAULT_ANCESTOR_LIMIT)); + strUsage += HelpMessageOpt("-limitancestorsize=", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT)); + strUsage += HelpMessageOpt("-limitdescendantcount=", strprintf("Do not accept transactions if any ancestor would have or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT)); + strUsage += HelpMessageOpt("-limitdescendantsize=", strprintf("Do not accept transactions if any ancestor would have more than kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT)); strUsage += HelpMessageOpt("-nuparams=hexBranchId:activationHeight", "Use given activation height for specified network upgrade (regtest-only)"); strUsage += HelpMessageOpt("-nurejectoldversions", strprintf("Reject peers that don't know about the current epoch (regtest-only) (default: %u)", DEFAULT_NU_REJECT_OLD_VERSIONS)); strUsage += HelpMessageOpt( diff --git a/src/main.cpp b/src/main.cpp index b52f1e0fc..151529689 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1964,6 +1964,17 @@ bool AcceptToMemoryPool( strprintf("%d > %d", nFees, maxTxFee)); } + // Calculate in-mempool ancestors, up to a limit. + CTxMemPool::setEntries setAncestors; + size_t nLimitAncestors = GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); + size_t nLimitAncestorSize = GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; + size_t nLimitDescendants = GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); + size_t nLimitDescendantSize = GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; + std::string errString; + if (!pool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { + return state.DoS(0, false, REJECT_NONSTANDARD, "too-long-mempool-chain", false, errString); + } + // Check against previous transactions // This is done near the end to help prevent CPU exhaustion denial-of-service attacks. std::vector allPrevOutputs; @@ -2027,7 +2038,7 @@ bool AcceptToMemoryPool( { // Store transaction in memory - pool.addUnchecked(hash, entry, !IsInitialBlockDownload(chainparams.GetConsensus())); + pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload(chainparams.GetConsensus())); // Add memory address index if (fAddressIndex) { @@ -3942,13 +3953,23 @@ bool static DisconnectTip(CValidationState &state, const CChainParams& chainpara if (!fBare) { // Resurrect mempool transactions from the disconnected block. + std::vector vHashUpdate; for (const CTransaction &tx : block.vtx) { // ignore validation errors in resurrected transactions list removed; CValidationState stateDummy; - if (tx.IsCoinBase() || !AcceptToMemoryPool(chainparams, mempool, stateDummy, tx, false, NULL)) + if (tx.IsCoinBase() || !AcceptToMemoryPool(chainparams, mempool, stateDummy, tx, false, NULL)) { mempool.remove(tx, removed, true); + } else if (mempool.exists(tx.GetHash())) { + vHashUpdate.push_back(tx.GetHash()); + } } + // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have + // no in-mempool children, which is generally not true when adding + // previously-confirmed transactions back to the mempool. + // UpdateTransactionsFromBlock finds descendants of any transactions in this + // block that were added back and cleans up the mempool state. + mempool.UpdateTransactionsFromBlock(vHashUpdate); if (sproutAnchorBeforeDisconnect != sproutAnchorAfterDisconnect) { // The anchor may not change between block disconnects, // in which case we don't want to evict from the mempool yet! diff --git a/src/main.h b/src/main.h index 0034a4969..863bbcbb2 100644 --- a/src/main.h +++ b/src/main.h @@ -78,6 +78,14 @@ static const CAmount HIGH_TX_FEE_PER_KB = 0.01 * COIN; static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB; /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; +/** Default for -limitancestorcount, max number of in-mempool ancestors */ +static const unsigned int DEFAULT_ANCESTOR_LIMIT = 100; +/** Default for -limitancestorsize, maximum kilobytes of tx + all in-mempool ancestors */ +static const unsigned int DEFAULT_ANCESTOR_SIZE_LIMIT = 1800; +/** Default for -limitdescendantcount, max number of in-mempool descendants */ +static const unsigned int DEFAULT_DESCENDANT_LIMIT = 1000; +/** Default for -limitdescendantsize, maximum kilobytes of in-mempool descendants */ +static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 5000; /** Default for -txexpirydelta, in number of blocks */ static const unsigned int DEFAULT_PRE_BLOSSOM_TX_EXPIRY_DELTA = 20; static const unsigned int DEFAULT_POST_BLOSSOM_TX_EXPIRY_DELTA = DEFAULT_PRE_BLOSSOM_TX_EXPIRY_DELTA * Consensus::BLOSSOM_POW_TARGET_SPACING_RATIO; diff --git a/src/memusage.h b/src/memusage.h index 3899fe61d..7339b36c8 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -90,18 +90,30 @@ static inline size_t DynamicUsage(const prevector& v) return MallocUsage(v.allocated_memory()); } -template -static inline size_t DynamicUsage(const std::set& s) +template +static inline size_t DynamicUsage(const std::set& s) { return MallocUsage(sizeof(stl_tree_node)) * s.size(); } -template -static inline size_t DynamicUsage(const std::map& m) +template +static inline size_t IncrementalDynamicUsage(const std::set& s) +{ + return MallocUsage(sizeof(stl_tree_node)); +} + +template +static inline size_t DynamicUsage(const std::map& m) { return MallocUsage(sizeof(stl_tree_node >)) * m.size(); } +template +static inline size_t IncrementalDynamicUsage(const std::map& m) +{ + return MallocUsage(sizeof(stl_tree_node >)); +} + template static inline size_t DynamicUsage(const std::unique_ptr& p) { diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ae35e09c4..feac2b0fd 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -348,6 +348,9 @@ UniValue mempoolToJSON(bool fVerbose = false) info.pushKV("height", (int)e.GetHeight()); info.pushKV("startingpriority", e.GetPriority(e.GetHeight())); info.pushKV("currentpriority", e.GetPriority(chainActive.Height())); + info.pushKV("descendantcount", e.GetCountWithDescendants()); + info.pushKV("descendantsize", e.GetSizeWithDescendants()); + info.pushKV("descendantfees", e.GetFeesWithDescendants()); const CTransaction& tx = e.GetTx(); set setDepends; for (const CTxIn& txin : tx.vin) @@ -403,6 +406,9 @@ UniValue getrawmempool(const UniValue& params, bool fHelp) " \"height\" : n, (numeric) block height when transaction entered pool\n" " \"startingpriority\" : n, (numeric) priority when transaction entered pool\n" " \"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" " \"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 735ec05bc..236819c97 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -12,6 +12,7 @@ #include #include +#include BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup) @@ -104,6 +105,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) removed.clear(); } +template +void CheckSort(CTxMemPool &pool, std::vector &sortedOrder) +{ + BOOST_CHECK_EQUAL(pool.size(), sortedOrder.size()); + typename CTxMemPool::indexed_transaction_set::nth_index::type::iterator it = pool.mapTx.get().begin(); + int count=0; + for (; it != pool.mapTx.get().end(); ++it, ++count) { + BOOST_CHECK_EQUAL(it->GetTx().GetHash().ToString(), sortedOrder[count]); + } +} + BOOST_AUTO_TEST_CASE(MempoolIndexingTest) { CTxMemPool pool(CFeeRate(0)); @@ -148,38 +160,164 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); BOOST_CHECK_EQUAL(pool.size(), 5); - // Check the fee-rate index is in order, should be tx3, tx5, tx1, tx4, tx2 - CTxMemPool::indexed_transaction_set::nth_index<1>::type::iterator it = pool.mapTx.get<1>().begin(); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx3.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx5.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx1.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx4.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx2.GetHash().ToString()); - BOOST_CHECK(it == pool.mapTx.get<1>().end()); + std::vector sortedOrder; + sortedOrder.resize(5); + sortedOrder[0] = tx3.GetHash().ToString(); // 0 + sortedOrder[1] = tx5.GetHash().ToString(); // 10000 + sortedOrder[2] = tx1.GetHash().ToString(); // 10000 + sortedOrder[3] = tx4.GetHash().ToString(); // 15000 + sortedOrder[4] = tx2.GetHash().ToString(); // 20000 + CheckSort<1>(pool, sortedOrder); + /* low fee but with high fee child */ + /* tx6 -> tx7 -> tx8, tx9 -> tx10 */ + CMutableTransaction tx6 = CMutableTransaction(); + tx6.vout.resize(1); + tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx6.vout[0].nValue = 20 * COIN; + pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6)); + BOOST_CHECK_EQUAL(pool.size(), 6); + // Check that at this point, tx6 is sorted low + sortedOrder.insert(sortedOrder.begin(), tx6.GetHash().ToString()); + CheckSort<1>(pool, sortedOrder); + + CTxMemPool::setEntries setAncestors; + setAncestors.insert(pool.mapTx.find(tx6.GetHash())); + 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(2); + tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx7.vout[0].nValue = 10 * COIN; + tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx7.vout[1].nValue = 1 * COIN; + + CTxMemPool::setEntries setAncestorsCalculated; + std::string dummy; + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK(setAncestorsCalculated == setAncestors); + + pool.addUnchecked(tx7.GetHash(), entry.FromTx(tx7), setAncestors); + BOOST_CHECK_EQUAL(pool.size(), 7); + + // Now tx6 should be sorted higher (high fee child): tx7, tx6, tx2, ... + sortedOrder.erase(sortedOrder.begin()); + sortedOrder.push_back(tx6.GetHash().ToString()); + sortedOrder.push_back(tx7.GetHash().ToString()); + CheckSort<1>(pool, sortedOrder); + + /* low fee child of tx7 */ + CMutableTransaction tx8 = CMutableTransaction(); + tx8.vin.resize(1); + tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0); + tx8.vin[0].scriptSig = CScript() << OP_11; + tx8.vout.resize(1); + tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx8.vout[0].nValue = 10 * COIN; + setAncestors.insert(pool.mapTx.find(tx7.GetHash())); + pool.addUnchecked(tx8.GetHash(), entry.Fee(0LL).Time(2).FromTx(tx8), setAncestors); + + // Now tx8 should be sorted low, but tx6/tx both high + sortedOrder.insert(sortedOrder.begin(), tx8.GetHash().ToString()); + CheckSort<1>(pool, sortedOrder); + + /* low fee child of tx7 */ + CMutableTransaction tx9 = CMutableTransaction(); + tx9.vin.resize(1); + tx9.vin[0].prevout = COutPoint(tx7.GetHash(), 1); + tx9.vin[0].scriptSig = CScript() << OP_11; + tx9.vout.resize(1); + tx9.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx9.vout[0].nValue = 1 * COIN; + pool.addUnchecked(tx9.GetHash(), entry.Fee(0LL).Time(3).FromTx(tx9), setAncestors); + + // tx9 should be sorted low + BOOST_CHECK_EQUAL(pool.size(), 9); + sortedOrder.insert(sortedOrder.begin(), tx9.GetHash().ToString()); + CheckSort<1>(pool, sortedOrder); + + std::vector snapshotOrder = sortedOrder; + + setAncestors.insert(pool.mapTx.find(tx8.GetHash())); + setAncestors.insert(pool.mapTx.find(tx9.GetHash())); + /* tx10 depends on tx8 and tx9 and has a high fee*/ + CMutableTransaction tx10 = CMutableTransaction(); + tx10.vin.resize(2); + tx10.vin[0].prevout = COutPoint(tx8.GetHash(), 0); + tx10.vin[0].scriptSig = CScript() << OP_11; + tx10.vin[1].prevout = COutPoint(tx9.GetHash(), 0); + tx10.vin[1].scriptSig = CScript() << OP_11; + tx10.vout.resize(1); + tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx10.vout[0].nValue = 10 * COIN; + + setAncestorsCalculated.clear(); + BOOST_CHECK_EQUAL(pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(4).FromTx(tx10), setAncestorsCalculated, 100, 1000000, 1000, 1000000, dummy), true); + BOOST_CHECK(setAncestorsCalculated == setAncestors); + + pool.addUnchecked(tx10.GetHash(), entry.FromTx(tx10), setAncestors); + + /** + * tx8 and tx9 should both now be sorted higher + * Final order after tx10 is added: + * + * tx3 = 0 (1) + * tx5 = 10000 (1) + * tx1 = 10000 (1) + * tx4 = 15000 (1) + * tx2 = 20000 (1) + * tx9 = 200k (2 txs) + * tx8 = 200k (2 txs) + * tx10 = 200k (1 tx) + * tx6 = 2.2M (5 txs) + * tx7 = 2.2M (4 txs) + */ + sortedOrder.erase(sortedOrder.begin(), sortedOrder.begin()+2); // take out tx9, tx8 from the beginning + sortedOrder.insert(sortedOrder.begin()+5, tx9.GetHash().ToString()); + sortedOrder.insert(sortedOrder.begin()+6, tx8.GetHash().ToString()); + sortedOrder.insert(sortedOrder.begin()+7, tx10.GetHash().ToString()); // tx10 is just before tx6 + CheckSort<1>(pool, sortedOrder); + + // there should be 10 transactions in the mempool + 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); + CheckSort<1>(pool, snapshotOrder); + + pool.remove(pool.mapTx.find(tx9.GetHash())->GetTx(), removed, true); + pool.remove(pool.mapTx.find(tx8.GetHash())->GetTx(), removed, true); /* Now check the sort on the mining score index. * Final order should be: * + * tx7 (2M) * tx2 (20k) * tx4 (15000) * tx1/tx5 (10000) - * tx3 (0) + * tx3/6 (0) * (Ties resolved by hash) */ - { - CTxMemPool::indexed_transaction_set::nth_index<2>::type::iterator it = pool.mapTx.get<2>().begin(); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx2.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx4.GetHash().ToString()); - if (tx1.GetHash() < tx5.GetHash()) { - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx5.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx1.GetHash().ToString()); - } else { - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx1.GetHash().ToString()); - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx5.GetHash().ToString()); - } - BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx3.GetHash().ToString()); - BOOST_CHECK(it == pool.mapTx.get<2>().end()); + sortedOrder.clear(); + sortedOrder.push_back(tx7.GetHash().ToString()); + sortedOrder.push_back(tx2.GetHash().ToString()); + sortedOrder.push_back(tx4.GetHash().ToString()); + if (tx1.GetHash() < tx5.GetHash()) { + sortedOrder.push_back(tx5.GetHash().ToString()); + sortedOrder.push_back(tx1.GetHash().ToString()); + } else { + sortedOrder.push_back(tx1.GetHash().ToString()); + sortedOrder.push_back(tx5.GetHash().ToString()); } + if (tx3.GetHash() < tx6.GetHash()) { + sortedOrder.push_back(tx6.GetHash().ToString()); + sortedOrder.push_back(tx3.GetHash().ToString()); + } else { + sortedOrder.push_back(tx3.GetHash().ToString()); + sortedOrder.push_back(tx6.GetHash().ToString()); + } + CheckSort<2>(pool, sortedOrder); } BOOST_AUTO_TEST_CASE(RemoveWithoutBranchId) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 13ea7a9e2..679970104 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -22,13 +22,6 @@ using namespace std; -CTxMemPoolEntry::CTxMemPoolEntry(): - nFee(0), nTxSize(0), nModSize(0), nUsageSize(0), nTime(0), dPriority(0.0), - hadNoDependencies(false), spendsCoinbase(false) -{ - nHeight = MEMPOOL_HEIGHT; -} - CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf, @@ -40,7 +33,10 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, nTxSize = ::GetSerializeSize(_tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = _tx.CalculateModifiedSize(nTxSize); nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); - feeRate = CFeeRate(nFee, nTxSize); + + nCountWithDescendants = 1; + nSizeWithDescendants = nTxSize; + nFeesWithDescendants = nFee; feeDelta = 0; } @@ -64,6 +60,244 @@ void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) 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) +{ + // 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); + for (const txiter childEntry : setChildren) { + cacheMap::iterator cacheIt = cachedDescendants.find(childEntry); + if (cacheIt != cachedDescendants.end()) { + // 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++; + } + } + } 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; + } + } + } + // setAllDescendants now contains all in-mempool descendants of updateIt. + // Update and add to cached descendant map + int64_t modifySize = 0; + CAmount modifyFee = 0; + int64_t modifyCount = 0; + for (txiter cit : setAllDescendants) { + if (!setExclude.count(cit->GetTx().GetHash())) { + modifySize += cit->GetTxSize(); + modifyFee += cit->GetFee(); + modifyCount++; + cachedDescendants[updateIt].insert(cit); + } + } + 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. +void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashesToUpdate) +{ + LOCK(cs); + // For each entry in vHashesToUpdate, store the set of in-mempool, but not + // in-vHashesToUpdate transactions, so that we don't have to recalculate + // descendants when we come across a previously seen entry. + cacheMap mapMemPoolDescendantsToUpdate; + + // Use a set for lookups into vHashesToUpdate (these entries are already + // accounted for in the state of their ancestors) + std::set setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end()); + + // Iterate in reverse, so that whenever we are looking at at a transaction + // we are sure that all in-mempool descendants have already been processed. + // This maximizes the benefit of the descendant cache and guarantees that + // setMemPoolChildren will be updated, an assumption made in + // UpdateForDescendants. + BOOST_REVERSE_FOREACH(const uint256 &hash, vHashesToUpdate) { + // we cache the in-mempool children to avoid duplicate updates + setEntries setChildren; + // calculate children from mapNextTx + txiter it = mapTx.find(hash); + if (it == mapTx.end()) { + continue; + } + std::map::iterator 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(); + txiter childIter = mapTx.find(childHash); + assert(childIter != mapTx.end()); + // We can skip updating entries we've encountered before or that + // are in the block (which are already accounted for). + if (setChildren.insert(childIter).second && !setAlreadyIncluded.count(childHash)) { + UpdateChild(it, childIter, true); + 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()); + } + } +} + +bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString) +{ + setEntries parentHashes; + const CTransaction &tx = entry.GetTx(); + + // Get parents of this transaction that are in the mempool + // Entry may or may not already be in the mempool, and GetMemPoolParents() + // is only valid for entries in the mempool, so we iterate mapTx to find + // parents. + // TODO: optimize this so that we only check limits and walk + // tx.vin when called on entries not already in the mempool. + for (unsigned int i = 0; i < tx.vin.size(); i++) { + txiter piter = mapTx.find(tx.vin[i].prevout.hash); + if (piter != mapTx.end()) { + parentHashes.insert(piter); + if (parentHashes.size() + 1 > limitAncestorCount) { + errString = strprintf("too many unconfirmed parents [limit: %u]", limitAncestorCount); + return false; + } + } + } + + size_t totalSizeWithAncestors = entry.GetTxSize(); + + while (!parentHashes.empty()) { + txiter stageit = *parentHashes.begin(); + + setAncestors.insert(stageit); + parentHashes.erase(stageit); + totalSizeWithAncestors += stageit->GetTxSize(); + + if (stageit->GetSizeWithDescendants() + entry.GetTxSize() > limitDescendantSize) { + errString = strprintf("exceeds descendant size limit for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantSize); + return false; + } else if (stageit->GetCountWithDescendants() + 1 > limitDescendantCount) { + errString = strprintf("too many descendants for tx %s [limit: %u]", stageit->GetTx().GetHash().ToString(), limitDescendantCount); + return false; + } else if (totalSizeWithAncestors > limitAncestorSize) { + errString = strprintf("exceeds ancestor size limit [limit: %u]", limitAncestorSize); + return false; + } + + const setEntries & setMemPoolParents = GetMemPoolParents(stageit); + for (const txiter &phash : setMemPoolParents) { + // If this is a new ancestor, add it. + if (setAncestors.count(phash) == 0) { + parentHashes.insert(phash); + } + if (parentHashes.size() + setAncestors.size() + 1 > limitAncestorCount) { + errString = strprintf("too many unconfirmed ancestors [limit: %u]", limitAncestorCount); + return false; + } + } + } + + return true; +} + +void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors) +{ + setEntries parentIters = GetMemPoolParents(it); + // add or remove this tx as a child of each parent + for (txiter piter : parentIters) { + UpdateChild(piter, it, add); + } + const int64_t updateCount = (add ? 1 : -1); + const int64_t updateSize = updateCount * it->GetTxSize(); + const CAmount updateFee = updateCount * it->GetFee(); + for (txiter ancestorIt : setAncestors) { + mapTx.modify(ancestorIt, update_descendant_state(updateSize, updateFee, updateCount)); + } +} + +void CTxMemPool::UpdateChildrenForRemoval(txiter it) +{ + const setEntries &setMemPoolChildren = GetMemPoolChildren(it); + for (txiter updateIt : setMemPoolChildren) { + UpdateParent(updateIt, it, false); + } +} + +void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) +{ + // For each entry, walk back all ancestors and decrement size associated with this + // transaction + const uint64_t nNoLimit = std::numeric_limits::max(); + for (txiter removeIt : entriesToRemove) { + setEntries setAncestors; + const CTxMemPoolEntry &entry = *removeIt; + std::string dummy; + CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + // 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!) + UpdateAncestorsOf(false, removeIt, setAncestors); + } + // After updating all the ancestor sizes, we can now sever the link between each + // transaction being removed and any mempool children (ie, update setMemPoolParents + // for each direct child of a transaction being removed). + for (txiter removeIt : entriesToRemove) { + UpdateChildrenForRemoval(removeIt); + } +} + +void CTxMemPoolEntry::SetDirty() +{ + nCountWithDescendants = 0; + nSizeWithDescendants = nTxSize; + nFeesWithDescendants = nFee; +} + +void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) +{ + if (!IsDirty()) { + nSizeWithDescendants += modifySize; + assert(int64_t(nSizeWithDescendants) > 0); + nFeesWithDescendants += modifyFee; + assert(nFeesWithDescendants >= 0); + nCountWithDescendants += modifyCount; + assert(int64_t(nCountWithDescendants) > 0); + } +} + CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) : nTransactionsUpdated(0) { @@ -110,8 +344,7 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n) nTransactionsUpdated += n; } - -bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate) +bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate) { // Add to memory pool without checking anything. // Used by main.cpp AcceptToMemoryPool(), which DOES do @@ -119,15 +352,37 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, LOCK(cs); weightedTxTree->add(WeightedTxInfo::from(entry.GetTx(), entry.GetFee())); indexed_transaction_set::iterator newit = mapTx.insert(entry).first; + mapLinks.insert(make_pair(newit, TxLinks())); // Update cachedInnerUsage to include contained transaction's usage. + // (When we update the entry for in-mempool parents, memory usage will be + // further updated.) cachedInnerUsage += entry.DynamicMemoryUsage(); const CTransaction& tx = newit->GetTx(); mapRecentlyAddedTx[tx.GetHash()] = &tx; nRecentlyAddedSequence += 1; - for (unsigned int i = 0; i < tx.vin.size(); i++) + std::set setParentTransactions; + for (unsigned int i = 0; i < tx.vin.size(); i++) { mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); + setParentTransactions.insert(tx.vin[i].prevout.hash); + } + // Don't bother worrying about child transactions of this one. + // Normal case of a new transaction arriving is that there can't be any + // children, because such children would be orphans. + // An exception to that is if a transaction enters that used to be in a block. + // In that case, our disconnect block logic will call UpdateTransactionsFromBlock + // to clean up the mess we're leaving here. + + // Update ancestors with information about this tx + for (const uint256 &phash : setParentTransactions) { + txiter pit = mapTx.find(phash); + if (pit != mapTx.end()) { + UpdateParent(newit, pit, true); + } + } + UpdateAncestorsOf(true, newit, setAncestors); + for (const JSDescription &joinsplit : tx.vJoinSplit) { for (const uint256 &nf : joinsplit.nullifiers) { mapSproutNullifiers[nf] = &tx; @@ -156,6 +411,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, return true; } +// START insightexplorer void CTxMemPool::addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view) { LOCK(cs); @@ -188,7 +444,6 @@ void CTxMemPool::addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewC mapAddressInserted.insert(make_pair(txhash, inserted)); } -// START insightexplorer void CTxMemPool::getAddressIndex( const std::vector>& addresses, std::vector>& results) @@ -263,14 +518,78 @@ void CTxMemPool::removeSpentIndex(const uint256 txhash) } // END insightexplorer +void CTxMemPool::removeUnchecked(txiter it) +{ + const uint256 hash = it->GetTx().GetHash(); + mapRecentlyAddedTx.erase(hash); + for (const CTxIn& txin : it->GetTx().vin) + mapNextTx.erase(txin.prevout); + for (const JSDescription& joinsplit : it->GetTx().vJoinSplit) { + for (const uint256& nf : joinsplit.nullifiers) { + mapSproutNullifiers.erase(nf); + } + } + for (const SpendDescription &spendDescription : it->GetTx().vShieldedSpend) { + mapSaplingNullifiers.erase(spendDescription.nullifier); + } + for (const uint256 &orchardNullifier : it->GetTx().GetOrchardBundle().GetNullifiers()) { + mapOrchardNullifiers.erase(orchardNullifier); + } + + totalTxSize -= it->GetTxSize(); + cachedInnerUsage -= it->DynamicMemoryUsage(); + cachedInnerUsage -= memusage::DynamicUsage(mapLinks[it].parents) + memusage::DynamicUsage(mapLinks[it].children); + mapLinks.erase(it); + mapTx.erase(it); + nTransactionsUpdated++; + minerPolicyEstimator->removeTx(hash); + + // insightexplorer + if (fAddressIndex) + removeAddressIndex(hash); + if (fSpentIndex) + removeSpentIndex(hash); +} + +// Calculates descendants of entry that are not already in setDescendants, and adds to +// setDescendants. Assumes entryit is already a tx in the mempool and setMemPoolChildren +// is correct for tx and all descendants. +// Also assumes that if an entry is in setDescendants already, then all +// in-mempool descendants of it are already in setDescendants as well, so that we +// can save time by not iterating over those entries. +void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants) +{ + setEntries stage; + if (setDescendants.count(entryit) == 0) { + stage.insert(entryit); + } + // Traverse down the children of entry, only adding children that are not + // accounted for in setDescendants already (because those children have either + // already been walked, or will be walked in this iteration). + while (!stage.empty()) { + txiter it = *stage.begin(); + setDescendants.insert(it); + stage.erase(it); + + const setEntries &setChildren = GetMemPoolChildren(it); + for (const txiter &childiter : setChildren) { + if (!setDescendants.count(childiter)) { + stage.insert(childiter); + } + } + } +} + void CTxMemPool::remove(const CTransaction &origTx, std::list& removed, bool fRecursive) { // Remove transaction from memory pool { LOCK(cs); - std::deque txToRemove; - txToRemove.push_back(origTx.GetHash()); - if (fRecursive && !mapTx.count(origTx.GetHash())) { + setEntries txToRemove; + 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 // 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 @@ -279,51 +598,23 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem std::map::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); if (it == mapNextTx.end()) continue; - txToRemove.push_back(it->second.ptx->GetHash()); + txiter nextit = mapTx.find(it->second.ptx->GetHash()); + assert(nextit != mapTx.end()); + txToRemove.insert(nextit); } } - while (!txToRemove.empty()) - { - uint256 hash = txToRemove.front(); - txToRemove.pop_front(); - if (!mapTx.count(hash)) - continue; - const CTransaction& tx = mapTx.find(hash)->GetTx(); - if (fRecursive) { - for (unsigned int i = 0; i < tx.vout.size(); i++) { - std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); - if (it == mapNextTx.end()) - continue; - txToRemove.push_back(it->second.ptx->GetHash()); - } + setEntries setAllRemoves; + if (fRecursive) { + for (txiter it : txToRemove) { + CalculateDescendants(it, setAllRemoves); } - mapRecentlyAddedTx.erase(hash); - for (const CTxIn& txin : tx.vin) - mapNextTx.erase(txin.prevout); - for (const JSDescription& joinsplit : tx.vJoinSplit) { - for (const uint256& nf : joinsplit.nullifiers) { - mapSproutNullifiers.erase(nf); - } - } - for (const SpendDescription &spendDescription : tx.vShieldedSpend) { - mapSaplingNullifiers.erase(spendDescription.nullifier); - } - for (const uint256 &orchardNullifier : tx.GetOrchardBundle().GetNullifiers()) { - mapOrchardNullifiers.erase(orchardNullifier); - } - removed.push_back(tx); - totalTxSize -= mapTx.find(hash)->GetTxSize(); - cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); - mapTx.erase(hash); - nTransactionsUpdated++; - minerPolicyEstimator->removeTx(hash); - - // insightexplorer - if (fAddressIndex) - removeAddressIndex(hash); - if (fSpentIndex) - removeSpentIndex(hash); + } else { + setAllRemoves.swap(txToRemove); } + for (txiter it : setAllRemoves) { + removed.push_back(it->GetTx()); + } + RemoveStaged(setAllRemoves); for (CTransaction tx : removed) { weightedTxTree->remove(tx.GetHash()); } @@ -528,6 +819,7 @@ void CTxMemPool::removeWithoutBranchId(uint32_t nMemPoolBranchId) void CTxMemPool::_clear() { + mapLinks.clear(); mapTx.clear(); mapNextTx.clear(); totalTxSize = 0; @@ -564,7 +856,12 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const checkTotal += it->GetTxSize(); innerUsage += it->DynamicMemoryUsage(); const CTransaction& tx = it->GetTx(); + txlinksMap::const_iterator linksiter = mapLinks.find(it); + assert(linksiter != mapLinks.end()); + const TxLinks &links = linksiter->second; + innerUsage += memusage::DynamicUsage(links.parents) + memusage::DynamicUsage(links.children); bool fDependsWait = false; + setEntries setParentCheck; for (const CTxIn &txin : tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); @@ -572,6 +869,7 @@ 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); } else { const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash); assert(coins && coins->IsAvailable(txin.prevout.n)); @@ -583,6 +881,32 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const assert(it3->second.n == i); i++; } + assert(setParentCheck == GetMemPoolParents(it)); + // Check children against mapNextTx + CTxMemPool::setEntries setChildrenCheck; + std::map::const_iterator iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); + int64_t childSizes = 0; + CAmount childFees = 0; + for (; iter != mapNextTx.end() && iter->first.hash == it->GetTx().GetHash(); ++iter) { + txiter childit = mapTx.find(iter->second.ptx->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. + // 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); if (fDependsWait) waitingOnDependants.push_back(&(*it)); @@ -905,8 +1229,8 @@ size_t CTxMemPool::DynamicMemoryUsage() const { // boost::multi_index_contained is implemented. total += memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 9 * sizeof(void*)) * mapTx.size(); - // Two metadata maps inherited from Bitcoin Core - total += memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas); + // Three metadata maps inherited from Bitcoin Core + total += memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks); // Saves iterating over the full map total += cachedInnerUsage; @@ -957,3 +1281,57 @@ void CTxMemPool::EnsureSizeLimit() { remove(mapTx.find(txId)->GetTx(), removed, true); } } + +void CTxMemPool::RemoveStaged(setEntries &stage) { + AssertLockHeld(cs); + UpdateForRemoveFromMempool(stage); + for (const txiter& it : stage) { + removeUnchecked(it); + } +} + +bool CTxMemPool::addUnchecked(const uint256&hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate) +{ + LOCK(cs); + setEntries setAncestors; + uint64_t nNoLimit = std::numeric_limits::max(); + std::string dummy; + CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + return addUnchecked(hash, entry, setAncestors, fCurrentEstimate); +} + +void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add) +{ + setEntries s; + if (add && mapLinks[entry].children.insert(child).second) { + cachedInnerUsage += memusage::IncrementalDynamicUsage(s); + } else if (!add && mapLinks[entry].children.erase(child)) { + cachedInnerUsage -= memusage::IncrementalDynamicUsage(s); + } +} + +void CTxMemPool::UpdateParent(txiter entry, txiter parent, bool add) +{ + setEntries s; + if (add && mapLinks[entry].parents.insert(parent).second) { + cachedInnerUsage += memusage::IncrementalDynamicUsage(s); + } else if (!add && mapLinks[entry].parents.erase(parent)) { + cachedInnerUsage -= memusage::IncrementalDynamicUsage(s); + } +} + +const CTxMemPool::setEntries & CTxMemPool::GetMemPoolParents(txiter entry) const +{ + assert (entry != mapTx.end()); + txlinksMap::const_iterator it = mapLinks.find(entry); + assert(it != mapLinks.end()); + return it->second.parents; +} + +const CTxMemPool::setEntries & CTxMemPool::GetMemPoolChildren(txiter entry) const +{ + assert (entry != mapTx.end()); + txlinksMap::const_iterator it = mapLinks.find(entry); + assert(it != mapLinks.end()); + return it->second.children; +} diff --git a/src/txmempool.h b/src/txmempool.h index 46d155b56..08a50986e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -9,6 +9,7 @@ #include #include +#include #include "amount.h" #include "coins.h" @@ -46,9 +47,25 @@ inline bool AllowFree(double dPriority) /** Fake height value used in CCoins to signify they are only in the memory pool (since 0.8) */ static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; -/** - * CTxMemPool stores these: +class CTxMemPool; + +/** \class CTxMemPoolEntry + * + * CTxMemPoolEntry stores data about the correponding transaction, as well + * as data about all in-mempool transactions that depend on the transaction + * ("descendant" transactions). + * + * When a new entry is added to the mempool, we update the descendant state + * (nCountWithDescendants, nSizeWithDescendants, and nFeesWithDescendants) 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 + * amount of work we're willing to do to avoid consuming too much CPU.) + * */ + class CTxMemPoolEntry { private: @@ -57,7 +74,6 @@ private: size_t nTxSize; //!< ... and avoid recomputing tx size size_t nModSize; //!< ... and modified size for priority size_t nUsageSize; //!< ... and total memory usage - CFeeRate feeRate; //!< ... and fee per kB int64_t nTime; //!< Local time when entering the mempool double dPriority; //!< Priority when entering the mempool unsigned int nHeight; //!< Chain height when entering the mempool @@ -67,19 +83,26 @@ private: int64_t feeDelta; //!< Used for determining the priority of the transaction for mining in a block uint32_t nBranchId; //!< Branch ID this transaction is known to commit to, cached for efficiency + // 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 + // correct. + uint64_t nCountWithDescendants; //! number of descendant transactions + uint64_t nSizeWithDescendants; //! ... and size + CAmount nFeesWithDescendants; //! ... and total fees (all including us) + public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf, bool spendsCoinbase, unsigned int nSigOps, uint32_t nBranchId); - CTxMemPoolEntry(); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return *this->tx; } std::shared_ptr GetSharedTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) const; const CAmount& GetFee() const { return nFee; } - CFeeRate GetFeeRate() const { return feeRate; } size_t GetTxSize() const { return nTxSize; } int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return nHeight; } @@ -88,13 +111,48 @@ public: int64_t GetModifiedFee() const { return nFee + feeDelta; } 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 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; } + bool GetSpendsCoinbase() const { return spendsCoinbase; } uint32_t GetValidatedBranchId() const { return nBranchId; } }; +// Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. +struct update_descendant_state +{ + update_descendant_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount) : + modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount) + {} + + void operator() (CTxMemPoolEntry &e) + { e.UpdateState(modifySize, modifyFee, modifyCount); } + + private: + int64_t modifySize; + CAmount modifyFee; + int64_t modifyCount; +}; + +struct set_dirty +{ + void operator() (CTxMemPoolEntry &e) + { e.SetDirty(); } +}; + struct update_fee_delta { update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { } @@ -115,14 +173,40 @@ struct mempoolentry_txid } }; +/** \class CompareTxMemPoolEntryByFee + * + * Sort an entry by max(feerate of entry's tx, feerate with all descendants). + */ class CompareTxMemPoolEntryByFee { public: bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { - if (a.GetFeeRate() == b.GetFeeRate()) + bool fUseADescendants = UseDescendantFeeRate(a); + bool fUseBDescendants = UseDescendantFeeRate(b); + + double aFees = fUseADescendants ? a.GetFeesWithDescendants() : a.GetFee(); + double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize(); + + double bFees = fUseBDescendants ? b.GetFeesWithDescendants() : b.GetFee(); + 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; + + if (f1 == f2) { return a.GetTime() >= b.GetTime(); - return a.GetFeeRate() < b.GetFeeRate(); + } + return f1 < f2; + } + + // Calculate which feerate to use for an entry (avoiding division). + bool UseDescendantFeeRate(const CTxMemPoolEntry &a) const + { + double f1 = (double)a.GetFee() * a.GetSizeWithDescendants(); + double f2 = (double)a.GetFeesWithDescendants() * a.GetTxSize(); + return f2 > f1; } }; @@ -144,6 +228,15 @@ public: } }; +class CompareTxMemPoolEntryByEntryTime +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + { + return a.GetTime() < b.GetTime(); + } +}; + class CBlockPolicyEstimator; /** An inpoint - a combination of a transaction and an index n into its vin */ @@ -187,11 +280,69 @@ struct TxMempoolInfo * * CTxMemPool::mapTx, and CTxMemPoolEntry bookkeeping: * - * mapTx is a boost::multi_index that sorts the mempool on 3 criteria: + * mapTx is a boost::multi_index that sorts the mempool on 2 criteria: * - transaction hash - * - feerate + * - feerate [we use max(feerate of tx, feerate of tx with all descendants)] * - mining score (feerate modified by any fee deltas from PrioritiseTransaction) * + * Note: the term "descendant" refers to in-mempool transactions that depend on + * this one, while "ancestor" refers to in-mempool transactions that a given + * transaction depends on. + * + * In order for the feerate sort to remain correct, we must update transactions + * in the mempool when new descendants arrive. To facilitate this, we track + * the set of in-mempool direct parents and direct children in mapLinks. Within + * each CTxMemPoolEntry, we track the size and fees of all descendants. + * + * Usually when a new transaction is added to the mempool, it has no in-mempool + * children (because any such children would be an orphan). So in + * addUnchecked(), we: + * - update a new entry's setMemPoolParents to include all in-mempool parents + * - update the new entry's direct parents to include the new tx as a child + * - update all ancestors of the transaction to include the new tx's size/fee + * + * When a transaction is removed from the mempool, we must: + * - update all in-mempool parents to not track the tx in setMemPoolChildren + * - update all ancestors to not include the tx's size/fees in descendant state + * - update all in-mempool children to not include it as a parent + * + * These happen in UpdateForRemoveFromMempool(). (Note that when removing a + * transaction along with its descendants, we must calculate that set of + * transactions to be removed before doing the removal, or else the mempool can + * be in an inconsistent state where it's impossible to walk the ancestors of + * a transaction.) + * + * In the event of a reorg, the assumption that a newly added tx has no + * in-mempool children is false. In particular, the mempool is in an + * inconsistent state while new transactions are being added, because there may + * be descendant transactions of a tx coming from a disconnected block that are + * unreachable from just looking at transactions in the mempool (the linking + * transactions may also be in the disconnected block, waiting to be added). + * Because of this, there's not much benefit in trying to search for in-mempool + * children in addUnchecked(). Instead, in the special case of transactions + * being added from a disconnected block, we require the caller to clean up the + * state, to account for in-mempool, out-of-block descendants for all the + * in-block transactions by calling UpdateTransactionsFromBlock(). Note that + * until this is called, the mempool state is not consistent, and in particular + * mapLinks may not be correct (and therefore functions like + * CalculateMemPoolAncestors() and CalculateDescendants() that rely + * on them to walk the mempool are not generally safe to use). + * + * Computational limits: + * + * Updating all in-mempool ancestors of a newly added transaction can be slow, + * if no bound exists on how many in-mempool ancestors there may be. + * CalculateMemPoolAncestors() takes configurable limits that are designed to + * prevent these calculations from being too CPU intensive. + * + * Adding transactions from a disconnected block can be very time consuming, + * because we don't have a way to limit the number of in-mempool descendants. + * To bound CPU processing, we limit the amount of work we're willing to do + * to properly update the descendant information for a tx being added from + * a disconnected block. If we would exceed the limit, then we instead mark + * the entry as "dirty", and set the feerate for sorting purposes to be equal + * the feerate of the transaction without any descendants. + * */ class CTxMemPool { @@ -275,8 +426,29 @@ public: mutable RecursiveMutex cs; indexed_transaction_set mapTx; typedef indexed_transaction_set::nth_index<0>::type::iterator txiter; + struct CompareIteratorByHash { + bool operator()(const txiter &a, const txiter &b) const { + return a->GetTx().GetHash() < b->GetTx().GetHash(); + } + }; + typedef std::set setEntries; private: + typedef std::map cacheMap; + + struct TxLinks { + setEntries parents; + setEntries children; + }; + + typedef std::map txlinksMap; + txlinksMap mapLinks; + + const setEntries & GetMemPoolParents(txiter entry) const; + const setEntries & GetMemPoolChildren(txiter entry) const; + void UpdateParent(txiter entry, txiter parent, bool add); + void UpdateChild(txiter entry, txiter child, bool add); + // insightexplorer std::map mapAddress; std::map > mapAddressInserted; @@ -306,7 +478,12 @@ public: void check(const CCoinsViewCache *pcoins) const; void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = static_cast(dFrequency * 4294967295.0); } + // addUnchecked must updated state for all ancestors of a given transaction, + // to track size/count of descendant transactions. First version of + // addUnchecked can be used to have it call CalculateMemPoolAncestors(), and + // 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); // START insightexplorer void addAddressIndex(const CTxMemPoolEntry &entry, const CCoinsViewCache &view); @@ -345,6 +522,33 @@ public: void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta) const; void ClearPrioritisation(const uint256 hash); +public: + /** 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); + + /** When adding transactions from a disconnected block back to the mempool, + * new mempool entries may have children in the mempool (which is generally + * not the case when otherwise adding transactions). + * UpdateTransactionsFromBlock() will find child transactions and update the + * descendant state for each transaction in hashesToUpdate (excluding any + * child transactions present in hashesToUpdate, which are already accounted + * for). Note: hashesToUpdate should be the set of transactions from the + * disconnected block that have been accepted back into the mempool. + */ + void UpdateTransactionsFromBlock(const std::vector &hashesToUpdate); + + /** Try to calculate all in-mempool ancestors of entry. + * (these are all calculated including the tx itself) + * limitAncestorCount = max number of ancestors + * limitAncestorSize = max size of ancestors + * limitDescendantCount = max number of descendants any ancestor can have + * limitDescendantSize = max size of descendants any ancestor can have + * errString = populated with error reason if any limits are hit + */ + bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString); + bool nullifierExists(const uint256& nullifier, ShieldedType type) const; std::pair, uint64_t> DrainRecentlyAdded(); @@ -395,6 +599,48 @@ public: bool IsRecentlyEvicted(const uint256& txId); // If the mempool size limit is exceeded, this evicts transactions from the mempool until it is below capacity void EnsureSizeLimit(); + +private: + /** UpdateForDescendants is used by UpdateTransactionsFromBlock to update + * the descendants for a single transaction that has been added to the + * mempool but may have child transactions in the mempool, eg during a + * chain reorg. setExclude is the set of descendant transactions in the + * mempool that must not be accounted for (because any descendants in + * setExclude were added to the mempool after the transaction being + * 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, + 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); + /** Sever link between specified transaction and direct children. */ + void UpdateChildrenForRemoval(txiter entry); + /** Populate setDescendants with all in-mempool descendants of hash. + * Assumes that setDescendants includes all in-mempool descendants of anything + * already in it. */ + void CalculateDescendants(txiter it, setEntries &setDescendants); + + /** Before calling removeUnchecked for a given transaction, + * UpdateForRemoveFromMempool must be called on the entire (dependent) set + * of transactions being removed at the same time. We use each + * CTxMemPoolEntry's setMemPoolParents in order to walk ancestors of a + * given transaction that is removed, so we can't remove intermediate + * transactions in a chain before we've updated all the state for the + * removal. + */ + void removeUnchecked(txiter entry); }; /**