From eebbfb84b6098c07f7763fad0c425caf18baf3ef Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 18 Mar 2016 21:24:04 -0700 Subject: [PATCH] Keep mempool consistent after conflicting withdraw-spent --- src/test/mempool_tests.cpp | 6 ++++-- src/test/policyestimator_tests.cpp | 11 ++++++----- src/txmempool.cpp | 13 ++++++++++++- src/txmempool.h | 4 ++-- src/validation.cpp | 2 +- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 91f549fe48..e8e4044251 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -417,7 +417,8 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* after tx6 is mined, tx7 should move up in the sort */ std::vector vtx; vtx.push_back(MakeTransactionRef(tx6)); - pool.removeForBlock(vtx, 1); + std::set > setWithdrawsSpent; + pool.removeForBlock(vtx, 1, setWithdrawsSpent); sortedOrder.erase(sortedOrder.begin()+1); // Ties are broken by hash @@ -560,7 +561,8 @@ 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 - pool.removeForBlock(vtx, 1); + std::set > setWithdrawsSpent; + pool.removeForBlock(vtx, 1, setWithdrawsSpent); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); // ... then feerate should drop 1/2 each halflife diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 0c060801bc..468f80e94f 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -38,6 +38,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) for (unsigned int i = 0; i < 128; i++) garbage.push_back('X'); CMutableTransaction tx; + std::set > dummyWithdraws; tx.vin.resize(1); tx.vin[0].scriptSig = garbage; tx.vout.resize(1); @@ -72,7 +73,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[9-h].pop_back(); } } - mpool.removeForBlock(block, ++blocknum); + mpool.removeForBlock(block, ++blocknum, dummyWithdraws); block.clear(); if (blocknum == 30) { // At this point we should need to combine 5 buckets to get enough data points @@ -116,7 +117,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); + mpool.removeForBlock(block, ++blocknum, dummyWithdraws); BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { @@ -136,7 +137,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].push_back(hash); } } - mpool.removeForBlock(block, ++blocknum); + mpool.removeForBlock(block, ++blocknum, dummyWithdraws); } int answerFound; @@ -155,7 +156,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) txHashes[j].pop_back(); } } - mpool.removeForBlock(block, 265); + mpool.removeForBlock(block, 265, dummyWithdraws); block.clear(); BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); for (int i = 2; i < 10;i++) { @@ -176,7 +177,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) } } - mpool.removeForBlock(block, ++blocknum); + mpool.removeForBlock(block, ++blocknum, dummyWithdraws); block.clear(); } BOOST_CHECK(mpool.estimateFee(1) == CFeeRate(0)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 6560daddc3..d832d45fec 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -601,7 +601,8 @@ 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) +void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, + const std::set >& setWithdrawsSpent) { LOCK(cs); std::vector entries; @@ -626,6 +627,16 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); } + for (std::set >::const_iterator it = setWithdrawsSpent.begin(); it != setWithdrawsSpent.end(); it++) { + std::map, uint256>::const_iterator it2 = mapWithdrawsSpentToTxid.find(*it); + if (it2 != mapWithdrawsSpentToTxid.end()) { + txiter txit = mapTx.find(it2->second); + assert(txit != mapTx.end()); + setEntries stage; + stage.insert(txit); + RemoveStaged(stage, true); + } + } lastRollingFeeUpdate = GetTime(); blockSinceLastRollingFeeBump = true; } diff --git a/src/txmempool.h b/src/txmempool.h index 0eac67ea3c..3d204aa41d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -543,8 +543,8 @@ public: void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx); - void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight); - + void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, + const std::set >& setWithdrawsSpent); void clear(); void _clear(); //lock free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); diff --git a/src/validation.cpp b/src/validation.cpp index 15f82e6cc3..8c2e4da917 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2347,7 +2347,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool.; - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, setWithdrawsSpent); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams);