From 8591de929ee93ca3d9e1fc4239699c4c91a0485b Mon Sep 17 00:00:00 2001 From: Gregory Sanders Date: Mon, 20 May 2019 13:57:56 -0400 Subject: [PATCH] kill useless mapPeginsSpentToTxid --- src/test/mempool_tests.cpp | 93 +----------------------------- src/test/policyestimator_tests.cpp | 11 ++-- src/txmempool.cpp | 37 +----------- src/txmempool.h | 4 +- src/validation.cpp | 2 +- 5 files changed, 11 insertions(+), 136 deletions(-) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 4c84c7fa86..e945df3dc1 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -390,8 +390,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* after tx6 is mined, tx7 should move up in the sort */ std::vector vtx; vtx.push_back(MakeTransactionRef(tx6)); - std::set> setPeginsSpentDummy; - pool.removeForBlock(vtx, 1, setPeginsSpentDummy); + pool.removeForBlock(vtx, 1); sortedOrder.erase(sortedOrder.begin()+1); // Ties are broken by hash @@ -420,93 +419,6 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) CheckSort(pool, sortedOrder); } -// ELEMENTS: -BOOST_AUTO_TEST_CASE(PeginSpentTest) -{ - CBlockPolicyEstimator feeEst; - CTxMemPool pool(&feeEst); - LOCK(pool.cs); - - std::set > setPeginsSpent; - TestMemPoolEntryHelper entry; - - std::pair pegin1, pegin2, pegin3; - GetRandBytes(pegin1.first.begin(), pegin1.first.size()); - GetRandBytes(pegin2.first.begin(), pegin2.first.size()); - GetRandBytes(pegin3.first.begin(), pegin3.first.size()); - GetRandBytes(pegin1.second.hash.begin(), pegin1.second.hash.size()); - GetRandBytes(pegin2.second.hash.begin(), pegin2.second.hash.size()); - pegin3.second.hash = pegin2.second.hash; - pegin1.second.n = 0; - pegin2.second.n = 0; - pegin3.second.n = 1; - - CMutableTransaction tx; - tx.vin.resize(1); - tx.vout.resize(1); - tx.vout[0].nValue = 0; - pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx)); - BOOST_CHECK(pool.mapPeginsSpentToTxid.empty()); - - setPeginsSpent = {pegin1}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(2); - tx.vout[1].nValue = 0; - const uint256 tx2Hash(tx.GetHash()); - pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx)); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx2Hash.ToString()); - - setPeginsSpent = {pegin2}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(3); - tx.vout[2].nValue = 0; - const uint256 tx3Hash(tx.GetHash()); - pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx)); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString()); - - setPeginsSpent = {pegin3}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(4); - tx.vout[3].nValue = 0; - CTransactionRef txref(MakeTransactionRef(tx)); - pool.removeForBlock({txref}, 1, setPeginsSpent); - - BOOST_CHECK_EQUAL(pool.size(), 3); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid.size(), 2); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx2Hash.ToString()); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString()); - - setPeginsSpent = {pegin1}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(5); - tx.vout[4].nValue = 0; - txref = MakeTransactionRef(tx); - pool.removeForBlock({txref}, 2, setPeginsSpent); - - BOOST_CHECK_EQUAL(pool.size(), 2); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid.size(), 1); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin2].ToString(), tx3Hash.ToString()); - - setPeginsSpent = {pegin1, pegin3}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(6); - tx.vout[5].nValue = 0; - const uint256 tx4Hash(tx.GetHash()); - pool.addUnchecked(entry.PeginsSpent(setPeginsSpent).FromTx(tx)); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin1].ToString(), tx4Hash.ToString()); - BOOST_CHECK_EQUAL(pool.mapPeginsSpentToTxid[pegin3].ToString(), tx4Hash.ToString()); - - setPeginsSpent = {pegin2, pegin3}; - GetRandBytes(tx.vin[0].prevout.hash.begin(), tx.vin[0].prevout.hash.size()); - tx.vout.resize(7); - tx.vout[6].nValue = 0; - txref = MakeTransactionRef(tx); - pool.removeForBlock({txref}, 3, setPeginsSpent); - - BOOST_CHECK_EQUAL(pool.size(), 1); - BOOST_CHECK(pool.mapPeginsSpentToTxid.empty()); -} - BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { CTxMemPool pool; @@ -637,8 +549,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); // ... we should keep the same min fee until we get a block - std::set> setPeginsSpentDummy; - pool.removeForBlock(vtx, 1, setPeginsSpentDummy); + pool.removeForBlock(vtx, 1); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0)); // ... then feerate should drop 1/2 each halflife diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index ee6e34e3ac..51668cbe64 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -23,7 +23,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) CAmount basefee(2000); CAmount deltaFee(100); std::vector feeV; - std::set> setPeginsSpentDummy; // Populate vectors of increasing fees for (int j = 0; j < 10; j++) { @@ -75,7 +74,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[9-h].pop_back(); } } - mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy); + mpool.removeForBlock(block, ++blocknum); block.clear(); // Check after just a few txs that combining buckets works as expected if (blocknum == 3) { @@ -114,7 +113,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) // Mine 50 more blocks with no transactions happening, estimates shouldn't change // We haven't decayed the moving average enough so we still have enough data points in every bucket while (blocknum < 250) - mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy); + mpool.removeForBlock(block, ++blocknum); BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { @@ -134,7 +133,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].push_back(hash); } } - mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy); + mpool.removeForBlock(block, ++blocknum); } for (int i = 1; i < 10;i++) { @@ -151,7 +150,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].pop_back(); } } - mpool.removeForBlock(block, 266, setPeginsSpentDummy); + mpool.removeForBlock(block, 266); block.clear(); BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { @@ -172,7 +171,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } } - mpool.removeForBlock(block, ++blocknum, setPeginsSpentDummy); + mpool.removeForBlock(block, ++blocknum); block.clear(); } BOOST_CHECK(feeEst.estimateFee(1) == CFeeRate(0)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 6182777edb..60ae8bd855 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -403,13 +403,6 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces vTxHashes.emplace_back(tx.GetWitnessHash(), newit); newit->vTxHashesIdx = vTxHashes.size() - 1; - - // ELEMENTS: - typedef std::pair PeginPair; - for(const PeginPair& it : entry.setPeginsSpent) { - std::pair, uint256>::iterator, bool> ret = mapPeginsSpentToTxid.insert(std::make_pair(it, tx.GetHash())); - assert(ret.second); - } } void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) @@ -419,12 +412,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) for (const CTxIn& txin : it->GetTx().vin) mapNextTx.erase(txin.prevout); - // ELEMENTS: - typedef std::pair PeginPair; - for (const PeginPair& it2 : it->setPeginsSpent) { - mapPeginsSpentToTxid.erase(it2); - } - if (vTxHashes.size() > 1) { vTxHashes[it->vTxHashesIdx] = std::move(vTxHashes.back()); vTxHashes[it->vTxHashesIdx].second->vTxHashesIdx = it->vTxHashesIdx; @@ -561,7 +548,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) /** * Called when a block is connected. Removes from mempool and updates the miner fee estimator. */ -void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, const std::set>& setPeginsSpent, bool pak_transition) +void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, bool pak_transition) { LOCK(cs); std::vector entries; @@ -587,22 +574,6 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne ClearPrioritisation(tx->GetHash()); } - // ELEMENTS: - // Eject any conflicting pegins - for (std::set >::const_iterator it = setPeginsSpent.begin(); it != setPeginsSpent.end(); it++) { - std::map, uint256>::const_iterator it2 = mapPeginsSpentToTxid.find(*it); - if (it2 != mapPeginsSpentToTxid.end()) { - uint256 tx_id = it2->second; - txiter txit = mapTx.find(tx_id); - assert(txit != mapTx.end()); - const CTransaction& tx = txit->GetTx(); - setEntries stage; - stage.insert(txit); - RemoveStaged(stage, true); - removeRecursive(tx, MemPoolRemovalReason::CONFLICT); - ClearPrioritisation(tx_id); - } - } // Eject any newly-invalid peg-outs based on changing block commitment const CChainParams& chainparams = Params(); if (pak_transition && chainparams.GetEnforcePak()) { @@ -786,10 +757,6 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const for (std::set >::const_iterator it = setGlobalPeginsSpent.begin(); it != setGlobalPeginsSpent.end(); it++) { assert(!pcoins->IsPeginSpent(*it)); } - for (std::map, uint256>::const_iterator it = mapPeginsSpentToTxid.begin(); it != mapPeginsSpentToTxid.end(); it++) { - assert(setGlobalPeginsSpent.erase(it->first)); - } - assert(setGlobalPeginsSpent.size() == 0); // END ELEMENTS // @@ -988,7 +955,7 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { // ELEMENTS: bool CCoinsViewMemPool::IsPeginSpent(const std::pair &outpoint) const { - return mempool.mapPeginsSpentToTxid.count(outpoint) || base->IsPeginSpent(outpoint); + return base->IsPeginSpent(outpoint); } size_t CTxMemPool::DynamicMemoryUsage() const { diff --git a/src/txmempool.h b/src/txmempool.h index 2f2904dffc..81c47e9e2b 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -505,8 +505,6 @@ class CTxMemPool const setEntries & GetMemPoolParents(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); const setEntries & GetMemPoolChildren(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); - // ELEMENTS: - std::map, uint256> mapPeginsSpentToTxid; private: typedef std::map cacheMap; @@ -554,7 +552,7 @@ class CTxMemPool void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, - const std::set>& setPeginsSpent, bool pak_transition=false); + bool pak_transition=false); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free diff --git a/src/validation.cpp b/src/validation.cpp index 2f064b37bd..bb44d9624e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2650,7 +2650,7 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp // ELEMENTS: We also eject now-invalid peg-outs based on block transition if not config list set // If config is set, this means all peg-outs have been filtered for that list already and other // functionaries aren't matching your list. Operator should restart with no list or new matching list. - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, setPeginsSpent, (paklist && !g_paklist_config)); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, (paklist && !g_paklist_config)); disconnectpool.removeForBlock(blockConnecting.vtx); // Update chainActive & related variables. chainActive.SetTip(pindexNew);