diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index ec5bd2a48..270c77de7 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -28,8 +28,9 @@ testScripts=( 'rawtransactions.py' 'rest.py' 'mempool_spendcoinbase.py' - 'mempool_coinbase_spends.py' + 'mempool_reorg.py' 'mempool_tx_input_limit.py' + 'mempool_nu_activation.py' 'httpbasics.py' 'zapwallettxes.py' 'proxy_test.py' diff --git a/qa/rpc-tests/mempool_nu_activation.py b/qa/rpc-tests/mempool_nu_activation.py new file mode 100755 index 000000000..f54095660 --- /dev/null +++ b/qa/rpc-tests/mempool_nu_activation.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, initialize_chain_clean, \ + start_node, connect_nodes, wait_and_assert_operationid_status + +from decimal import Decimal + +# Test mempool behaviour around network upgrade activation +class MempoolUpgradeActivationTest(BitcoinTestFramework): + + alert_filename = None # Set by setup_network + + def setup_network(self): + args = ["-checkmempool", "-debug=mempool", "-blockmaxsize=4000", "-nuparams=5ba81b19:200"] + self.nodes = [] + 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 + + def setup_chain(self): + print "Initializing test directory "+self.options.tmpdir + initialize_chain_clean(self.options.tmpdir, 2) + + def run_test(self): + self.nodes[1].generate(100) + self.sync_all() + + # Mine 97 blocks. After this, nodes[1] blocks + # 1 to 97 are spend-able. + self.nodes[0].generate(97) + self.sync_all() + + # Shield some ZEC + node1_taddr = self.nodes[1].getnewaddress() + node0_zaddr = self.nodes[0].z_getnewaddress() + recipients = [{'address': node0_zaddr, 'amount': Decimal('10')}] + myopid = self.nodes[1].z_sendmany(node1_taddr, recipients, 1, Decimal('0')) + print wait_and_assert_operationid_status(self.nodes[1], myopid) + self.sync_all() + + # Mine block 198. After this, the mempool expects + # block 199, which is the last Sprout block. + self.nodes[0].generate(1) + self.sync_all() + + # Mempool should be empty. + assert_equal(set(self.nodes[0].getrawmempool()), set()) + + # Check node 0 shielded balance + assert_equal(self.nodes[0].z_getbalance(node0_zaddr), Decimal('10')) + + # Fill the mempool with twice as many transactions as can fit into blocks + node0_taddr = self.nodes[0].getnewaddress() + sprout_txids = [] + while self.nodes[1].getmempoolinfo()['bytes'] < 2 * 4000: + sprout_txids.append(self.nodes[1].sendtoaddress(node0_taddr, Decimal('0.001'))) + self.sync_all() + + # Spends should be in the mempool + sprout_mempool = set(self.nodes[0].getrawmempool()) + assert_equal(sprout_mempool, set(sprout_txids)) + + # Mine block 199. After this, the mempool expects + # block 200, which is the first Overwinter block. + self.nodes[0].generate(1) + self.sync_all() + + # mempool should be empty. + assert_equal(set(self.nodes[0].getrawmempool()), set()) + + # Block 199 should contain a subset of the original mempool + # (with all other transactions having been dropped) + block_txids = self.nodes[0].getblock(self.nodes[0].getbestblockhash())['tx'] + assert(len(block_txids) < len(sprout_txids)) + for txid in block_txids[1:]: # Exclude coinbase + assert(txid in sprout_txids) + + # Create some transparent Overwinter transactions + overwinter_txids = [self.nodes[1].sendtoaddress(node0_taddr, Decimal('0.001')) for i in range(10)] + self.sync_all() + + # Create a shielded Overwinter transaction + recipients = [{'address': node0_taddr, 'amount': Decimal('10')}] + myopid = self.nodes[0].z_sendmany(node0_zaddr, recipients, 1, Decimal('0')) + shielded = wait_and_assert_operationid_status(self.nodes[0], myopid) + assert(shielded != None) + overwinter_txids.append(shielded) + self.sync_all() + + # Spends should be in the mempool + assert_equal(set(self.nodes[0].getrawmempool()), set(overwinter_txids)) + + # Node 0 note should be unspendable + assert_equal(self.nodes[0].z_getbalance(node0_zaddr), Decimal('0')) + + # Invalidate block 199. + self.nodes[0].invalidateblock(self.nodes[0].getbestblockhash()) + + # BUG: Ideally, the mempool should now only contain the transactions + # that were in block 199, the Overwinter transactions having been dropped. + # However, because chainActive is not updated until after the transactions + # in the disconnected block have been re-added to the mempool, the height + # seen by AcceptToMemoryPool is one greater than it should be. This causes + # the block 199 transactions to be validated against the Overwinter rules, + # and rejected because they (obviously) fail. + #assert_equal(set(self.nodes[0].getrawmempool()), set(block_txids[1:])) + assert_equal(set(self.nodes[0].getrawmempool()), set()) + + # Node 0 note should be spendable again + assert_equal(self.nodes[0].z_getbalance(node0_zaddr), Decimal('10')) + +if __name__ == '__main__': + MempoolUpgradeActivationTest().main() diff --git a/qa/rpc-tests/mempool_coinbase_spends.py b/qa/rpc-tests/mempool_reorg.py similarity index 70% rename from qa/rpc-tests/mempool_coinbase_spends.py rename to qa/rpc-tests/mempool_reorg.py index 041de831b..ad12dadf2 100755 --- a/qa/rpc-tests/mempool_coinbase_spends.py +++ b/qa/rpc-tests/mempool_reorg.py @@ -9,7 +9,8 @@ # from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, start_node, connect_nodes +from test_framework.authproxy import JSONRPCException +from test_framework.util import assert_equal, assert_raises, start_node, connect_nodes # Create one-input, one-output, no-fee transaction: @@ -49,16 +50,25 @@ class MempoolCoinbaseTest(BitcoinTestFramework): # 3. Indirect (coinbase and child both in chain) : spend_103 and spend_103_1 # Use invalidatblock to make all of the above coinbase spends invalid (immature coinbase), # and make sure the mempool code behaves correctly. - b = [ self.nodes[0].getblockhash(n) for n in range(102, 105) ] + b = [ self.nodes[0].getblockhash(n) for n in range(101, 105) ] coinbase_txids = [ self.nodes[0].getblock(h)['tx'][0] for h in b ] - spend_101_raw = self.create_tx(coinbase_txids[0], node1_address, 10) - spend_102_raw = self.create_tx(coinbase_txids[1], node0_address, 10) - spend_103_raw = self.create_tx(coinbase_txids[2], node0_address, 10) + spend_101_raw = self.create_tx(coinbase_txids[1], node1_address, 10) + spend_102_raw = self.create_tx(coinbase_txids[2], node0_address, 10) + spend_103_raw = self.create_tx(coinbase_txids[3], node0_address, 10) + + # Create a block-height-locked transaction which will be invalid after reorg + timelock_tx = self.nodes[0].createrawtransaction([{"txid": coinbase_txids[0], "vout": 0}], {node0_address: 10}) + # Set the time lock + timelock_tx = timelock_tx.replace("ffffffff", "11111111", 1) + timelock_tx = timelock_tx[:-8] + hex(self.nodes[0].getblockcount() + 2)[2:] + "000000" + timelock_tx = self.nodes[0].signrawtransaction(timelock_tx)["hex"] + assert_raises(JSONRPCException, self.nodes[0].sendrawtransaction, timelock_tx) # Broadcast and mine spend_102 and 103: spend_102_id = self.nodes[0].sendrawtransaction(spend_102_raw) spend_103_id = self.nodes[0].sendrawtransaction(spend_103_raw) self.nodes[0].generate(1) + assert_raises(JSONRPCException, self.nodes[0].sendrawtransaction, timelock_tx) # Create 102_1 and 103_1: spend_102_1_raw = self.create_tx(spend_102_id, node1_address, 10) @@ -66,8 +76,8 @@ class MempoolCoinbaseTest(BitcoinTestFramework): # Broadcast and mine 103_1: spend_103_1_id = self.nodes[0].sendrawtransaction(spend_103_1_raw) - [spend_103_1_id] # hush pyflakes - self.nodes[0].generate(1) + last_block = self.nodes[0].generate(1) + timelock_tx_id = self.nodes[0].sendrawtransaction(timelock_tx) # ... now put spend_101 and spend_102_1 in memory pools: spend_101_id = self.nodes[0].sendrawtransaction(spend_101_raw) @@ -75,7 +85,11 @@ class MempoolCoinbaseTest(BitcoinTestFramework): self.sync_all() - assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id ])) + assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id, timelock_tx_id ])) + + for node in self.nodes: + node.invalidateblock(last_block[0]) + assert_equal(set(self.nodes[0].getrawmempool()), set([ spend_101_id, spend_102_1_id, spend_103_1_id ])) # Use invalidateblock to re-org back and make all those coinbase spends # immature/invalid: diff --git a/src/consensus/upgrades.h b/src/consensus/upgrades.h index 0c5462c2e..6a9173264 100644 --- a/src/consensus/upgrades.h +++ b/src/consensus/upgrades.h @@ -26,6 +26,9 @@ struct NUInfo { extern const struct NUInfo NetworkUpgradeInfo[]; +// Consensus branch id to identify pre-overwinter (Sprout) consensus rules. +static const uint32_t SPROUT_BRANCH_ID = NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId; + /** * Checks the state of a given network upgrade based on block height. * Caller must check that the height is >= 0 (and handle unknown heights). diff --git a/src/gtest/test_mempool.cpp b/src/gtest/test_mempool.cpp index b9c47b211..30203aa41 100644 --- a/src/gtest/test_mempool.cpp +++ b/src/gtest/test_mempool.cpp @@ -87,7 +87,7 @@ TEST(Mempool, PriorityStatsDoNotCrash) { unsigned int nHeight = 92045; double dPriority = view.GetPriority(tx, nHeight); - CTxMemPoolEntry entry(tx, nFees, nTime, dPriority, nHeight, true); + CTxMemPoolEntry entry(tx, nFees, nTime, dPriority, nHeight, true, false, SPROUT_BRANCH_ID); // Check it does not crash (ie. the death test fails) EXPECT_NONFATAL_FAILURE(EXPECT_DEATH(testPool.addUnchecked(tx.GetHash(), entry), ""), ""); diff --git a/src/init.cpp b/src/init.cpp index bd6e2e79e..16306c011 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -914,7 +914,10 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) InitWarning(_("Warning: Unsupported argument -benchmark ignored, use -debug=bench.")); // Checkmempool and checkblockindex default to true in regtest mode - mempool.setSanityCheck(GetBoolArg("-checkmempool", chainparams.DefaultConsistencyChecks())); + int ratio = std::min(std::max(GetArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000); + if (ratio != 0) { + mempool.setSanityCheck(1.0 / ratio); + } fCheckBlockIndex = GetBoolArg("-checkblockindex", chainparams.DefaultConsistencyChecks()); fCheckpointsEnabled = GetBoolArg("-checkpoints", true); diff --git a/src/main.cpp b/src/main.cpp index a17cebe3d..612067d89 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1280,7 +1280,23 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa CAmount nFees = nValueIn-nValueOut; double dPriority = view.GetPriority(tx, chainActive.Height()); - CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), mempool.HasNoInputsOf(tx)); + // Keep track of transactions that spend a coinbase, which we re-scan + // during reorgs to ensure COINBASE_MATURITY is still met. + bool fSpendsCoinbase = false; + BOOST_FOREACH(const CTxIn &txin, tx.vin) { + const CCoins *coins = view.AccessCoins(txin.prevout.hash); + if (coins->IsCoinBase()) { + fSpendsCoinbase = true; + break; + } + } + + // Grab the branch ID we expect this transaction to commit to. We don't + // yet know if it does, but if the entry gets added to the mempool, then + // it has passed ContextualCheckInputs and therefore this is correct. + auto consensusBranchId = CurrentEpochBranchId(chainActive.Height() + 1, Params().GetConsensus()); + + CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), mempool.HasNoInputsOf(tx), fSpendsCoinbase, consensusBranchId); unsigned int nSize = entry.GetTxSize(); // Accept a tx if it contains joinsplits and has at least the default fee specified by z_sendmany. @@ -2511,11 +2527,13 @@ void static UpdateTip(CBlockIndex *pindexNew) { } } -/** Disconnect chainActive's tip. */ +/** + * Disconnect chainActive's tip. You probably want to call mempool.removeForReorg and + * mempool.removeWithoutBranchId after this, with cs_main held. + */ bool static DisconnectTip(CValidationState &state, bool fBare = false) { CBlockIndex *pindexDelete = chainActive.Tip(); assert(pindexDelete); - mempool.check(pcoinsTip); // Read block from disk. CBlock block; if (!ReadBlockFromDisk(block, pindexDelete)) @@ -2549,8 +2567,6 @@ bool static DisconnectTip(CValidationState &state, bool fBare = false) { // in which case we don't want to evict from the mempool yet! mempool.removeWithAnchor(anchorBeforeDisconnect); } - mempool.removeCoinbaseSpends(pcoinsTip, pindexDelete->nHeight); - mempool.check(pcoinsTip); } // Update chainActive and related variables. @@ -2577,10 +2593,10 @@ static int64_t nTimePostConnect = 0; /** * Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. + * You probably want to call mempool.removeWithoutBranchId after this, with cs_main held. */ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock *pblock) { assert(pindexNew->pprev == chainActive.Tip()); - mempool.check(pcoinsTip); // Read block from disk. int64_t nTime1 = GetTimeMicros(); CBlock block; @@ -2620,7 +2636,6 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * // Remove conflicting transactions from the mempool. list txConflicted; mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload()); - mempool.check(pcoinsTip); // Update chainActive & related variables. UpdateTip(pindexNew); // Tell wallet about transactions that went from mempool @@ -2724,9 +2739,11 @@ static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMo const CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); // Disconnect active blocks which are no longer in the best chain. + bool fBlocksDisconnected = false; while (chainActive.Tip() && chainActive.Tip() != pindexFork) { if (!DisconnectTip(state)) return false; + fBlocksDisconnected = true; } // Build list of new blocks to connect. @@ -2734,43 +2751,50 @@ static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMo bool fContinue = true; int nHeight = pindexFork ? pindexFork->nHeight : -1; while (fContinue && nHeight != pindexMostWork->nHeight) { - // Don't iterate the entire list of potential improvements toward the best tip, as we likely only need - // a few blocks along the way. - int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight); - vpindexToConnect.clear(); - vpindexToConnect.reserve(nTargetHeight - nHeight); - CBlockIndex *pindexIter = pindexMostWork->GetAncestor(nTargetHeight); - while (pindexIter && pindexIter->nHeight != nHeight) { - vpindexToConnect.push_back(pindexIter); - pindexIter = pindexIter->pprev; - } - nHeight = nTargetHeight; - - // Connect new blocks. - BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { - if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL)) { - if (state.IsInvalid()) { - // The block violates a consensus rule. - if (!state.CorruptionPossible()) - InvalidChainFound(vpindexToConnect.back()); - state = CValidationState(); - fInvalidFound = true; - fContinue = false; - break; + // Don't iterate the entire list of potential improvements toward the best tip, as we likely only need + // a few blocks along the way. + int nTargetHeight = std::min(nHeight + 32, pindexMostWork->nHeight); + vpindexToConnect.clear(); + vpindexToConnect.reserve(nTargetHeight - nHeight); + CBlockIndex *pindexIter = pindexMostWork->GetAncestor(nTargetHeight); + while (pindexIter && pindexIter->nHeight != nHeight) { + vpindexToConnect.push_back(pindexIter); + pindexIter = pindexIter->pprev; + } + nHeight = nTargetHeight; + + // Connect new blocks. + BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { + if (!ConnectTip(state, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL)) { + if (state.IsInvalid()) { + // The block violates a consensus rule. + if (!state.CorruptionPossible()) + InvalidChainFound(vpindexToConnect.back()); + state = CValidationState(); + fInvalidFound = true; + fContinue = false; + break; + } else { + // A system error occurred (disk space, database error, ...). + return false; + } } else { - // A system error occurred (disk space, database error, ...). - return false; - } - } else { - PruneBlockIndexCandidates(); - if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) { - // We're in a better position than we were. Return temporarily to release the lock. - fContinue = false; - break; + PruneBlockIndexCandidates(); + if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) { + // We're in a better position than we were. Return temporarily to release the lock. + fContinue = false; + break; + } } } } + + if (fBlocksDisconnected) { + mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); } + mempool.removeWithoutBranchId( + CurrentEpochBranchId(chainActive.Tip()->nHeight + 1, Params().GetConsensus())); + mempool.check(pcoinsTip); // Callbacks/notifications for a new best chain. if (fInvalidFound) @@ -2856,6 +2880,9 @@ bool InvalidateBlock(CValidationState& state, CBlockIndex *pindex) { // ActivateBestChain considers blocks already in chainActive // unconditionally valid already, so force disconnect away from it. if (!DisconnectTip(state)) { + mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + mempool.removeWithoutBranchId( + CurrentEpochBranchId(chainActive.Tip()->nHeight + 1, Params().GetConsensus())); return false; } } @@ -2871,6 +2898,9 @@ bool InvalidateBlock(CValidationState& state, CBlockIndex *pindex) { } InvalidChainFound(pindex); + mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + mempool.removeWithoutBranchId( + CurrentEpochBranchId(chainActive.Tip()->nHeight + 1, Params().GetConsensus())); return true; } diff --git a/src/main.h b/src/main.h index 150211d15..ebdb48515 100644 --- a/src/main.h +++ b/src/main.h @@ -518,7 +518,7 @@ bool InvalidateBlock(CValidationState& state, CBlockIndex *pindex); /** Remove invalidity status from a block and its descendants. */ bool ReconsiderBlock(CValidationState& state, CBlockIndex *pindex); -/** The currently-connected chain of blocks. */ +/** The currently-connected chain of blocks (protected by cs_main). */ extern CChain chainActive; /** Global variable that points to the active CCoinsView (protected by cs_main) */ diff --git a/src/miner.cpp b/src/miner.cpp index 542cebec8..6b239c3d1 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -155,10 +155,10 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) // This vector will be sorted into a priority queue: vector vecPriority; vecPriority.reserve(mempool.mapTx.size()); - for (map::iterator mi = mempool.mapTx.begin(); + for (CTxMemPool::indexed_transaction_set::iterator mi = mempool.mapTx.begin(); mi != mempool.mapTx.end(); ++mi) { - const CTransaction& tx = mi->second.GetTx(); + const CTransaction& tx = mi->GetTx(); int64_t nLockTimeCutoff = (STANDARD_LOCKTIME_VERIFY_FLAGS & LOCKTIME_MEDIAN_TIME_PAST) ? nMedianTimePast @@ -198,7 +198,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) } mapDependers[txin.prevout.hash].push_back(porphan); porphan->setDependsOn.insert(txin.prevout.hash); - nTotalIn += mempool.mapTx[txin.prevout.hash].GetTx().vout[txin.prevout.n].nValue; + nTotalIn += mempool.mapTx.find(txin.prevout.hash)->GetTx().vout[txin.prevout.n].nValue; continue; } const CCoins* coins = view.AccessCoins(txin.prevout.hash); @@ -230,7 +230,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) porphan->feeRate = feeRate; } else - vecPriority.push_back(TxPriority(dPriority, feeRate, &mi->second.GetTx())); + vecPriority.push_back(TxPriority(dPriority, feeRate, &(mi->GetTx()))); } // Collect transactions into block diff --git a/src/rpcblockchain.cpp b/src/rpcblockchain.cpp index 363872ede..bbed8cba1 100644 --- a/src/rpcblockchain.cpp +++ b/src/rpcblockchain.cpp @@ -226,10 +226,9 @@ UniValue mempoolToJSON(bool fVerbose = false) { LOCK(mempool.cs); UniValue o(UniValue::VOBJ); - BOOST_FOREACH(const PAIRTYPE(uint256, CTxMemPoolEntry)& entry, mempool.mapTx) + BOOST_FOREACH(const CTxMemPoolEntry& e, mempool.mapTx) { - const uint256& hash = entry.first; - const CTxMemPoolEntry& e = entry.second; + const uint256& hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); info.push_back(Pair("size", (int)e.GetTxSize())); info.push_back(Pair("fee", ValueFromAmount(e.GetFee()))); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0996e13c4..325789572 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include "consensus/upgrades.h" #include "main.h" #include "txmempool.h" #include "util.h" @@ -17,6 +18,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) { // Test CTxMemPool::remove functionality + TestMemPoolEntryHelper entry; // Parent transaction with three children, // and three grand-children: CMutableTransaction txParent; @@ -60,17 +62,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) BOOST_CHECK_EQUAL(removed.size(), 0); // Just the parent: - testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); + testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); testPool.remove(txParent, removed, true); BOOST_CHECK_EQUAL(removed.size(), 1); removed.clear(); // Parent, children, grandchildren: - testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); + testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); for (int i = 0; i < 3; i++) { - testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); - testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); + testPool.addUnchecked(txChild[i].GetHash(), entry.FromTx(txChild[i])); + testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: testPool.remove(txChild[0], removed, true); @@ -90,8 +92,8 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) for (int i = 0; i < 3; i++) { - testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); - testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); + testPool.addUnchecked(txChild[i].GetHash(), entry.FromTx(txChild[i])); + testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // 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): @@ -101,4 +103,113 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) removed.clear(); } +BOOST_AUTO_TEST_CASE(MempoolIndexingTest) +{ + 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)); + + /* 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; + entry.nTime = 1; + entry.dPriority = 10.0; + 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 tx2, tx4, tx1, tx5, tx3 + CTxMemPool::indexed_transaction_set::nth_index<1>::type::iterator it = pool.mapTx.get<1>().begin(); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx2.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx4.GetHash().ToString()); + 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<1>().end()); +} + +BOOST_AUTO_TEST_CASE(RemoveWithoutBranchId) { + CTxMemPool pool(CFeeRate(0)); + TestMemPoolEntryHelper entry; + entry.nFee = 10000LL; + entry.hadNoDependencies = true; + + // Add some Sprout transactions + for (auto i = 1; i < 11; i++) { + CMutableTransaction tx = CMutableTransaction(); + tx.vout.resize(1); + tx.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx.vout[0].nValue = i * COIN; + pool.addUnchecked(tx.GetHash(), entry.BranchId(NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId).FromTx(tx)); + } + BOOST_CHECK_EQUAL(pool.size(), 10); + + // Check the pool only contains Sprout transactions + for (CTxMemPool::indexed_transaction_set::const_iterator it = pool.mapTx.begin(); it != pool.mapTx.end(); it++) { + BOOST_CHECK_EQUAL(it->GetValidatedBranchId(), NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId); + } + + // Add some dummy transactions + for (auto i = 1; i < 11; i++) { + CMutableTransaction tx = CMutableTransaction(); + tx.vout.resize(1); + tx.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx.vout[0].nValue = i * COIN + 100; + pool.addUnchecked(tx.GetHash(), entry.BranchId(NetworkUpgradeInfo[Consensus::UPGRADE_TESTDUMMY].nBranchId).FromTx(tx)); + } + BOOST_CHECK_EQUAL(pool.size(), 20); + + // Add some Overwinter transactions + for (auto i = 1; i < 11; i++) { + CMutableTransaction tx = CMutableTransaction(); + tx.vout.resize(1); + tx.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx.vout[0].nValue = i * COIN + 200; + pool.addUnchecked(tx.GetHash(), entry.BranchId(NetworkUpgradeInfo[Consensus::UPGRADE_OVERWINTER].nBranchId).FromTx(tx)); + } + BOOST_CHECK_EQUAL(pool.size(), 30); + + // Remove transactions that are not for Overwinter + pool.removeWithoutBranchId(NetworkUpgradeInfo[Consensus::UPGRADE_OVERWINTER].nBranchId); + BOOST_CHECK_EQUAL(pool.size(), 10); + + // Check the pool only contains Overwinter transactions + for (CTxMemPool::indexed_transaction_set::const_iterator it = pool.mapTx.begin(); it != pool.mapTx.end(); it++) { + BOOST_CHECK_EQUAL(it->GetValidatedBranchId(), NetworkUpgradeInfo[Consensus::UPGRADE_OVERWINTER].nBranchId); + } + + // Roll back to Sprout + pool.removeWithoutBranchId(NetworkUpgradeInfo[Consensus::BASE_SPROUT].nBranchId); + BOOST_CHECK_EQUAL(pool.size(), 0); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 0ef66f6c7..770cdbe76 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -143,6 +143,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CMutableTransaction tx,tx2; CScript script; uint256 hash; + TestMemPoolEntryHelper entry; + entry.nFee = 11; + entry.dPriority = 111.0; + entry.nHeight = 11; LOCK(cs_main); fCheckpointsEnabled = false; @@ -281,7 +285,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { tx.vout[0].nValue -= 10; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + bool spendsCoinbase = (i == 0) ? true : false; // only first tx spends coinbase + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); @@ -301,7 +306,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) { tx.vout[0].nValue -= 350; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + bool spendsCoinbase = (i == 0) ? true : false; // only first tx spends coinbase + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); @@ -310,7 +316,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // orphan in mempool hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).FromTx(tx)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); delete pblocktemplate; mempool.clear(); @@ -320,7 +326,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].prevout.hash = txFirst[1]->GetHash(); tx.vout[0].nValue = 39000LL; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin.resize(2); tx.vin[1].scriptSig = CScript() << OP_1; @@ -328,7 +334,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[1].prevout.n = 0; tx.vout[0].nValue = 49000LL; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); delete pblocktemplate; mempool.clear(); @@ -339,7 +345,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vin[0].scriptSig = CScript() << OP_0 << OP_1; tx.vout[0].nValue = 0; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); delete pblocktemplate; mempool.clear(); @@ -352,12 +358,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) script = CScript() << OP_0; tx.vout[0].scriptPubKey = GetScriptForDestination(CScriptID(script)); hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vin[0].prevout.hash = hash; tx.vin[0].scriptSig = CScript() << (std::vector)script; tx.vout[0].nValue -= 10000; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); delete pblocktemplate; mempool.clear(); @@ -368,10 +374,10 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].nValue = 49000LL; tx.vout[0].scriptPubKey = CScript() << OP_1; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); tx.vout[0].scriptPubKey = CScript() << OP_2; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); delete pblocktemplate; mempool.clear(); @@ -397,7 +403,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].scriptPubKey = CScript() << OP_1; tx.nLockTime = chainActive.Tip()->nHeight+1; hash = tx.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx)); BOOST_CHECK(!CheckFinalTx(tx, LOCKTIME_MEDIAN_TIME_PAST)); // time locked @@ -411,7 +417,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx2.vout[0].scriptPubKey = CScript() << OP_1; tx2.nLockTime = chainActive.Tip()->GetMedianTimePast()+1; hash = tx2.GetHash(); - mempool.addUnchecked(hash, CTxMemPoolEntry(tx2, 11, GetTime(), 111.0, 11)); + mempool.addUnchecked(hash, entry.Time(GetTime()).SpendsCoinbase(true).FromTx(tx2)); BOOST_CHECK(!CheckFinalTx(tx2, LOCKTIME_MEDIAN_TIME_PAST)); BOOST_CHECK(pblocktemplate = CreateNewBlock(scriptPubKey)); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 3fcd39661..7b0a632cb 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -16,6 +16,7 @@ BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) { CTxMemPool mpool(CFeeRate(1000)); + TestMemPoolEntryHelper entry; CAmount basefee(2000); double basepri = 1e6; CAmount deltaFee(100); @@ -63,7 +64,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 5; k++) { // add 4 fee txs for every priority tx tx.vin[0].prevout.n = 10000*blocknum+100*j+k; // make transaction unique uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, CTxMemPoolEntry(tx, feeV[k/4][j], GetTime(), priV[k/4][j], blocknum, mpool.HasNoInputsOf(tx))); + mpool.addUnchecked(hash, entry.Fee(feeV[k/4][j]).Time(GetTime()).Priority(priV[k/4][j]).Height(blocknum).FromTx(tx, &mpool)); txHashes[j].push_back(hash); } } @@ -132,7 +133,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 5; k++) { // add 4 fee txs for every priority tx tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, CTxMemPoolEntry(tx, feeV[k/4][j], GetTime(), priV[k/4][j], blocknum, mpool.HasNoInputsOf(tx))); + mpool.addUnchecked(hash, entry.Fee(feeV[k/4][j]).Time(GetTime()).Priority(priV[k/4][j]).Height(blocknum).FromTx(tx, &mpool)); txHashes[j].push_back(hash); } } @@ -168,7 +169,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (int k = 0; k < 5; k++) { // add 4 fee txs for every priority tx tx.vin[0].prevout.n = 10000*blocknum+100*j+k; uint256 hash = tx.GetHash(); - mpool.addUnchecked(hash, CTxMemPoolEntry(tx, feeV[k/4][j], GetTime(), priV[k/4][j], blocknum, mpool.HasNoInputsOf(tx))); + mpool.addUnchecked(hash, entry.Fee(feeV[k/4][j]).Time(GetTime()).Priority(priV[k/4][j]).Height(blocknum).FromTx(tx, &mpool)); CTransaction btx; if (mpool.lookup(hash, btx)) block.push_back(btx); diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 0a39f2944..02a3a50d2 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -12,6 +12,7 @@ #include "main.h" #include "random.h" #include "txdb.h" +#include "txmempool.h" #include "ui_interface.h" #include "util.h" #ifdef ENABLE_WALLET @@ -102,6 +103,13 @@ TestingSetup::~TestingSetup() boost::filesystem::remove_all(pathTemp); } + +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPool *pool) { + return CTxMemPoolEntry(tx, nFee, nTime, dPriority, nHeight, + pool ? pool->HasNoInputsOf(tx) : hadNoDependencies, + spendsCoinbase, nBranchId); +} + void Shutdown(void* parg) { exit(0); diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index d1c426373..ae528d682 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -1,6 +1,7 @@ #ifndef BITCOIN_TEST_TEST_BITCOIN_H #define BITCOIN_TEST_TEST_BITCOIN_H +#include "consensus/upgrades.h" #include "pubkey.h" #include "txdb.h" @@ -36,4 +37,34 @@ struct TestingSetup: public JoinSplitTestingSetup { ~TestingSetup(); }; +class CTxMemPoolEntry; +class CTxMemPool; + +struct TestMemPoolEntryHelper +{ + // Default values + CAmount nFee; + int64_t nTime; + double dPriority; + unsigned int nHeight; + bool hadNoDependencies; + bool spendsCoinbase; + uint32_t nBranchId; + + TestMemPoolEntryHelper() : + nFee(0), nTime(0), dPriority(0.0), nHeight(1), + hadNoDependencies(false), spendsCoinbase(false), + nBranchId(SPROUT_BRANCH_ID) { } + + CTxMemPoolEntry FromTx(CMutableTransaction &tx, CTxMemPool *pool = NULL); + + // Change the default value + TestMemPoolEntryHelper &Fee(CAmount _fee) { nFee = _fee; return *this; } + TestMemPoolEntryHelper &Time(int64_t _time) { nTime = _time; return *this; } + TestMemPoolEntryHelper &Priority(double _priority) { dPriority = _priority; return *this; } + TestMemPoolEntryHelper &Height(unsigned int _height) { nHeight = _height; return *this; } + TestMemPoolEntryHelper &HadNoDependencies(bool _hnd) { hadNoDependencies = _hnd; return *this; } + TestMemPoolEntryHelper &SpendsCoinbase(bool _flag) { spendsCoinbase = _flag; return *this; } + TestMemPoolEntryHelper &BranchId(uint32_t _branchId) { nBranchId = _branchId; return *this; } +}; #endif diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ed5653230..c69e77d05 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -11,6 +11,7 @@ #include "main.h" #include "policy/fees.h" #include "streams.h" +#include "timedata.h" #include "util.h" #include "utilmoneystr.h" #include "version.h" @@ -18,20 +19,24 @@ using namespace std; CTxMemPoolEntry::CTxMemPoolEntry(): - nFee(0), nTxSize(0), nModSize(0), nUsageSize(0), nTime(0), dPriority(0.0), hadNoDependencies(false) + 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): + unsigned int _nHeight, bool poolHasNoInputsOf, + bool _spendsCoinbase, uint32_t _nBranchId): tx(_tx), nFee(_nFee), nTime(_nTime), dPriority(_dPriority), nHeight(_nHeight), - hadNoDependencies(poolHasNoInputsOf) + hadNoDependencies(poolHasNoInputsOf), + spendsCoinbase(_spendsCoinbase), nBranchId(_nBranchId) { nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = tx.CalculateModifiedSize(nTxSize); nUsageSize = RecursiveDynamicUsage(tx); + feeRate = CFeeRate(nFee, nTxSize); } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -54,7 +59,7 @@ CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : // Sanity checks off by default for performance, because otherwise // accepting transactions becomes O(N^2) where N is the number // of transactions in the pool - fSanityCheck = false; + nCheckFrequency = 0; minerPolicyEstimator = new CBlockPolicyEstimator(_minRelayFee); } @@ -96,8 +101,8 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Used by main.cpp AcceptToMemoryPool(), which DOES do // all the appropriate checks. LOCK(cs); - mapTx[hash] = entry; - const CTransaction& tx = mapTx[hash].GetTx(); + mapTx.insert(entry); + const CTransaction& tx = mapTx.find(hash)->GetTx(); for (unsigned int i = 0; i < tx.vin.size(); i++) mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); BOOST_FOREACH(const JSDescription &joinsplit, tx.vjoinsplit) { @@ -139,7 +144,7 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem txToRemove.pop_front(); if (!mapTx.count(hash)) continue; - const CTransaction& tx = mapTx[hash].GetTx(); + 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)); @@ -157,8 +162,8 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem } removed.push_back(tx); - totalTxSize -= mapTx[hash].GetTxSize(); - cachedInnerUsage -= mapTx[hash].DynamicMemoryUsage(); + totalTxSize -= mapTx.find(hash)->GetTxSize(); + cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); mapTx.erase(hash); nTransactionsUpdated++; minerPolicyEstimator->removeTx(hash); @@ -166,22 +171,26 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem } } -void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight) +void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) { - // Remove transactions spending a coinbase which are now immature + // Remove transactions spending a coinbase which are now immature and no-longer-final transactions LOCK(cs); list transactionsToRemove; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - const CTransaction& tx = it->second.GetTx(); - BOOST_FOREACH(const CTxIn& txin, tx.vin) { - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); - if (it2 != mapTx.end()) - continue; - const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); - if (fSanityCheck) assert(coins); - if (!coins || (coins->IsCoinBase() && ((signed long)nMemPoolHeight) - coins->nHeight < COINBASE_MATURITY)) { - transactionsToRemove.push_back(tx); - break; + 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); + } else if (it->GetSpendsCoinbase()) { + BOOST_FOREACH(const CTxIn& txin, tx.vin) { + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); + if (it2 != mapTx.end()) + continue; + const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); + if (nCheckFrequency != 0) assert(coins); + if (!coins || (coins->IsCoinBase() && ((signed long)nMemPoolHeight) - coins->nHeight < COINBASE_MATURITY)) { + transactionsToRemove.push_back(tx); + break; + } } } } @@ -201,8 +210,8 @@ void CTxMemPool::removeWithAnchor(const uint256 &invalidRoot) LOCK(cs); list transactionsToRemove; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - const CTransaction& tx = it->second.GetTx(); + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + const CTransaction& tx = it->GetTx(); BOOST_FOREACH(const JSDescription& joinsplit, tx.vjoinsplit) { if (joinsplit.anchor == invalidRoot) { transactionsToRemove.push_back(tx); @@ -258,8 +267,10 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned i BOOST_FOREACH(const CTransaction& tx, vtx) { uint256 hash = tx.GetHash(); - if (mapTx.count(hash)) - entries.push_back(mapTx[hash]); + + indexed_transaction_set::iterator i = mapTx.find(hash); + if (i != mapTx.end()) + entries.push_back(*i); } BOOST_FOREACH(const CTransaction& tx, vtx) { @@ -272,6 +283,28 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned i minerPolicyEstimator->processBlock(nBlockHeight, entries, fCurrentEstimate); } +/** + * Called whenever the tip changes. Removes transactions which don't commit to + * the given branch ID from the mempool. + */ +void CTxMemPool::removeWithoutBranchId(uint32_t nMemPoolBranchId) +{ + LOCK(cs); + std::list transactionsToRemove; + + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + const CTransaction& tx = it->GetTx(); + if (it->GetValidatedBranchId() != nMemPoolBranchId) { + transactionsToRemove.push_back(tx); + } + } + + for (const CTransaction& tx : transactionsToRemove) { + std::list removed; + remove(tx, removed, true); + } +} + void CTxMemPool::clear() { LOCK(cs); @@ -284,7 +317,10 @@ void CTxMemPool::clear() void CTxMemPool::check(const CCoinsViewCache *pcoins) const { - if (!fSanityCheck) + if (nCheckFrequency == 0) + return; + + if (insecure_rand() >= nCheckFrequency) return; LogPrint("mempool", "Checking mempool with %u transactions and %u inputs\n", (unsigned int)mapTx.size(), (unsigned int)mapNextTx.size()); @@ -296,17 +332,17 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const LOCK(cs); list waitingOnDependants; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; - checkTotal += it->second.GetTxSize(); - innerUsage += it->second.DynamicMemoryUsage(); - const CTransaction& tx = it->second.GetTx(); + checkTotal += it->GetTxSize(); + innerUsage += it->DynamicMemoryUsage(); + const CTransaction& tx = it->GetTx(); bool fDependsWait = false; BOOST_FOREACH(const CTxIn &txin, tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) { - const CTransaction& tx2 = it2->second.GetTx(); + const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; } else { @@ -344,7 +380,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const intermediates.insert(std::make_pair(tree.root(), tree)); } if (fDependsWait) - waitingOnDependants.push_back(&it->second); + waitingOnDependants.push_back(&(*it)); else { CValidationState state; assert(ContextualCheckInputs(tx, state, mempoolDuplicate, false, 0, false, Params().GetConsensus(), NULL)); @@ -368,8 +404,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const } for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { uint256 hash = it->second.ptx->GetHash(); - map::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->second.GetTx(); + indexed_transaction_set::const_iterator it2 = mapTx.find(hash); + const CTransaction& tx = it2->GetTx(); assert(it2 != mapTx.end()); assert(&tx == it->second.ptx); assert(tx.vin.size() > it->second.n); @@ -378,8 +414,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const for (std::map::const_iterator it = mapNullifiers.begin(); it != mapNullifiers.end(); it++) { uint256 hash = it->second->GetHash(); - map::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->second.GetTx(); + indexed_transaction_set::const_iterator it2 = mapTx.find(hash); + const CTransaction& tx = it2->GetTx(); assert(it2 != mapTx.end()); assert(&tx == it->second); } @@ -394,16 +430,16 @@ void CTxMemPool::queryHashes(vector& vtxid) LOCK(cs); vtxid.reserve(mapTx.size()); - for (map::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) - vtxid.push_back((*mi).first); + for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) + vtxid.push_back(mi->GetTx().GetHash()); } bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const { LOCK(cs); - map::const_iterator i = mapTx.find(hash); + indexed_transaction_set::const_iterator i = mapTx.find(hash); if (i == mapTx.end()) return false; - result = i->second.GetTx(); + result = i->GetTx(); return true; } @@ -516,5 +552,6 @@ bool CCoinsViewMemPool::HaveCoins(const uint256 &txid) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - return memusage::DynamicUsage(mapTx) + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 6 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 6 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; } diff --git a/src/txmempool.h b/src/txmempool.h index cd0f9a54b..2cb2c8f05 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -13,6 +13,10 @@ #include "primitives/transaction.h" #include "sync.h" +#undef foreach +#include "boost/multi_index_container.hpp" +#include "boost/multi_index/ordered_index.hpp" + class CAutoFile; inline double AllowFreeThreshold() @@ -41,25 +45,54 @@ 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 bool hadNoDependencies; //! Not dependent on any other txs when it entered the mempool + bool spendsCoinbase; //! keep track of transactions that spend a coinbase + uint32_t nBranchId; //! Branch ID this transaction is known to commit to, cached for efficiency public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, - int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf = false); + int64_t _nTime, double _dPriority, unsigned int _nHeight, + bool poolHasNoInputsOf, bool spendsCoinbase, uint32_t nBranchId); CTxMemPoolEntry(); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) 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; } bool WasClearAtEntry() const { return hadNoDependencies; } size_t DynamicMemoryUsage() const { return nUsageSize; } + + bool GetSpendsCoinbase() const { return spendsCoinbase; } + uint32_t GetValidatedBranchId() const { return nBranchId; } +}; + +// extracts a TxMemPoolEntry's transaction hash +struct mempoolentry_txid +{ + typedef uint256 result_type; + result_type operator() (const CTxMemPoolEntry &entry) const + { + return entry.GetTx().GetHash(); + } +}; + +class CompareTxMemPoolEntryByFee +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + { + if (a.GetFeeRate() == b.GetFeeRate()) + return a.GetTime() < b.GetTime(); + return a.GetFeeRate() > b.GetFeeRate(); + } }; class CBlockPolicyEstimator; @@ -91,7 +124,7 @@ public: class CTxMemPool { private: - bool fSanityCheck; //! Normally false, true if -checkmempool or -regtest + uint32_t nCheckFrequency; //! Value n means that n times in 2^32 we check. unsigned int nTransactionsUpdated; CBlockPolicyEstimator* minerPolicyEstimator; @@ -99,8 +132,21 @@ private: uint64_t cachedInnerUsage; //! sum of dynamic memory usage of all the map elements (NOT the maps themselves) public: + typedef boost::multi_index_container< + CTxMemPoolEntry, + boost::multi_index::indexed_by< + // sorted by txid + boost::multi_index::ordered_unique, + // sorted by fee rate + boost::multi_index::ordered_non_unique< + boost::multi_index::identity, + CompareTxMemPoolEntryByFee + > + > + > indexed_transaction_set; + mutable CCriticalSection cs; - std::map mapTx; + indexed_transaction_set mapTx; std::map mapNextTx; std::map mapNullifiers; std::map > mapDeltas; @@ -115,15 +161,16 @@ public: * check does nothing. */ void check(const CCoinsViewCache *pcoins) const; - void setSanityCheck(bool _fSanityCheck) { fSanityCheck = _fSanityCheck; } + void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = dFrequency * 4294967296.0; } bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); void remove(const CTransaction &tx, std::list& removed, bool fRecursive = false); void removeWithAnchor(const uint256 &invalidRoot); - void removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight); + void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::list& removed); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate = true); + void removeWithoutBranchId(uint32_t nMemPoolBranchId); void clear(); void queryHashes(std::vector& vtxid); void pruneSpent(const uint256& hash, CCoins &coins);