Make removed and conflicted arguments optional to remove

This commit is contained in:
Pieter Wuille 2016-08-15 13:10:57 +02:00
parent f48211b700
commit 51f278329d
7 changed files with 31 additions and 35 deletions

View file

@ -2768,10 +2768,9 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara
std::vector<uint256> vHashUpdate; std::vector<uint256> vHashUpdate;
BOOST_FOREACH(const CTransaction &tx, block.vtx) { BOOST_FOREACH(const CTransaction &tx, block.vtx) {
// ignore validation errors in resurrected transactions // ignore validation errors in resurrected transactions
list<CTransaction> removed;
CValidationState stateDummy; CValidationState stateDummy;
if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL, true)) { if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL, true)) {
mempool.removeRecursive(tx, removed); mempool.removeRecursive(tx);
} else if (mempool.exists(tx.GetHash())) { } else if (mempool.exists(tx.GetHash())) {
vHashUpdate.push_back(tx.GetHash()); vHashUpdate.push_back(tx.GetHash());
} }
@ -2840,7 +2839,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams,
int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4;
LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001);
// Remove conflicting transactions from the mempool.; // Remove conflicting transactions from the mempool.;
mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload()); mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, &txConflicted, !IsInitialBlockDownload());
// Update chainActive & related variables. // Update chainActive & related variables.
UpdateTip(pindexNew, chainparams); UpdateTip(pindexNew, chainparams);

View file

@ -81,7 +81,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
std::list<CTransaction> removed; std::list<CTransaction> removed;
pool.removeRecursive(block.vtx[2], removed); pool.removeRecursive(block.vtx[2], &removed);
BOOST_CHECK_EQUAL(removed.size(), 1); BOOST_CHECK_EQUAL(removed.size(), 1);
CBlock block2; CBlock block2;

View file

@ -58,12 +58,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
std::list<CTransaction> removed; std::list<CTransaction> removed;
// Nothing in pool, remove should do nothing: // Nothing in pool, remove should do nothing:
testPool.removeRecursive(txParent, removed); testPool.removeRecursive(txParent, &removed);
BOOST_CHECK_EQUAL(removed.size(), 0); BOOST_CHECK_EQUAL(removed.size(), 0);
// Just the parent: // Just the parent:
testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
testPool.removeRecursive(txParent, removed); testPool.removeRecursive(txParent, &removed);
BOOST_CHECK_EQUAL(removed.size(), 1); BOOST_CHECK_EQUAL(removed.size(), 1);
removed.clear(); removed.clear();
@ -75,16 +75,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
} }
// Remove Child[0], GrandChild[0] should be removed: // Remove Child[0], GrandChild[0] should be removed:
testPool.removeRecursive(txChild[0], removed); testPool.removeRecursive(txChild[0], &removed);
BOOST_CHECK_EQUAL(removed.size(), 2); BOOST_CHECK_EQUAL(removed.size(), 2);
removed.clear(); removed.clear();
// ... make sure grandchild and child are gone: // ... make sure grandchild and child are gone:
testPool.removeRecursive(txGrandChild[0], removed); testPool.removeRecursive(txGrandChild[0], &removed);
BOOST_CHECK_EQUAL(removed.size(), 0); BOOST_CHECK_EQUAL(removed.size(), 0);
testPool.removeRecursive(txChild[0], removed); testPool.removeRecursive(txChild[0], &removed);
BOOST_CHECK_EQUAL(removed.size(), 0); BOOST_CHECK_EQUAL(removed.size(), 0);
// Remove parent, all children/grandchildren should go: // Remove parent, all children/grandchildren should go:
testPool.removeRecursive(txParent, removed); testPool.removeRecursive(txParent, &removed);
BOOST_CHECK_EQUAL(removed.size(), 5); BOOST_CHECK_EQUAL(removed.size(), 5);
BOOST_CHECK_EQUAL(testPool.size(), 0); BOOST_CHECK_EQUAL(testPool.size(), 0);
removed.clear(); removed.clear();
@ -97,7 +97,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
} }
// Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // 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): // put into the mempool (maybe because it is non-standard):
testPool.removeRecursive(txParent, removed); testPool.removeRecursive(txParent, &removed);
BOOST_CHECK_EQUAL(removed.size(), 6); BOOST_CHECK_EQUAL(removed.size(), 6);
BOOST_CHECK_EQUAL(testPool.size(), 0); BOOST_CHECK_EQUAL(testPool.size(), 0);
removed.clear(); removed.clear();
@ -281,12 +281,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
BOOST_CHECK_EQUAL(pool.size(), 10); BOOST_CHECK_EQUAL(pool.size(), 10);
// Now try removing tx10 and verify the sort order returns to normal // Now try removing tx10 and verify the sort order returns to normal
std::list<CTransaction> removed; pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed);
CheckSort<descendant_score>(pool, snapshotOrder); CheckSort<descendant_score>(pool, snapshotOrder);
pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed); pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed); pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx());
/* Now check the sort on the mining score index. /* Now check the sort on the mining score index.
* Final order should be: * Final order should be:
* *
@ -413,8 +412,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
/* after tx6 is mined, tx7 should move up in the sort */ /* after tx6 is mined, tx7 should move up in the sort */
std::vector<CTransaction> vtx; std::vector<CTransaction> vtx;
vtx.push_back(tx6); vtx.push_back(tx6);
std::list<CTransaction> dummy; pool.removeForBlock(vtx, 1, NULL, false);
pool.removeForBlock(vtx, 1, dummy, false);
sortedOrder.erase(sortedOrder.begin()+1); sortedOrder.erase(sortedOrder.begin()+1);
sortedOrder.pop_back(); sortedOrder.pop_back();
@ -549,12 +547,11 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool));
std::vector<CTransaction> vtx; std::vector<CTransaction> vtx;
std::list<CTransaction> conflicts;
SetMockTime(42); SetMockTime(42);
SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000);
// ... we should keep the same min fee until we get a block // ... we should keep the same min fee until we get a block
pool.removeForBlock(vtx, 1, conflicts); pool.removeForBlock(vtx, 1);
SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE); SetMockTime(42 + 2*CTxMemPool::ROLLING_FEE_HALFLIFE);
BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), (maxFeeRateRemoved.GetFeePerK() + 1000)/2);
// ... then feerate should drop 1/2 each halflife // ... then feerate should drop 1/2 each halflife

View file

@ -137,8 +137,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey,
// Test that packages above the min relay fee do get included, even if one // Test that packages above the min relay fee do get included, even if one
// of the transactions is below the min relay fee // of the transactions is below the min relay fee
// Remove the low fee transaction and replace with a higher fee transaction // Remove the low fee transaction and replace with a higher fee transaction
std::list<CTransaction> dummy; mempool.removeRecursive(tx);
mempool.removeRecursive(tx, dummy);
tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee tx.vout[0].nValue -= 2; // Now we should be just over the min relay fee
hashLowFeeTx = tx.GetHash(); hashLowFeeTx = tx.GetHash();
mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse+2).FromTx(tx)); mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse+2).FromTx(tx));

View file

@ -46,7 +46,6 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
for (unsigned int i = 0; i < 128; i++) for (unsigned int i = 0; i < 128; i++)
garbage.push_back('X'); garbage.push_back('X');
CMutableTransaction tx; CMutableTransaction tx;
std::list<CTransaction> dummyConflicted;
tx.vin.resize(1); tx.vin.resize(1);
tx.vin[0].scriptSig = garbage; tx.vin[0].scriptSig = garbage;
tx.vout.resize(1); tx.vout.resize(1);
@ -81,7 +80,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[9-h].pop_back(); txHashes[9-h].pop_back();
} }
} }
mpool.removeForBlock(block, ++blocknum, dummyConflicted); mpool.removeForBlock(block, ++blocknum);
block.clear(); block.clear();
if (blocknum == 30) { if (blocknum == 30) {
// At this point we should need to combine 5 buckets to get enough data points // At this point we should need to combine 5 buckets to get enough data points
@ -125,7 +124,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// Mine 50 more blocks with no transactions happening, estimates shouldn't change // 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 // We haven't decayed the moving average enough so we still have enough data points in every bucket
while (blocknum < 250) while (blocknum < 250)
mpool.removeForBlock(block, ++blocknum, dummyConflicted); mpool.removeForBlock(block, ++blocknum);
for (int i = 1; i < 10;i++) { for (int i = 1; i < 10;i++) {
BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee); BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() < origFeeEst[i-1] + deltaFee);
@ -146,7 +145,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[j].push_back(hash); txHashes[j].push_back(hash);
} }
} }
mpool.removeForBlock(block, ++blocknum, dummyConflicted); mpool.removeForBlock(block, ++blocknum);
} }
int answerFound; int answerFound;
@ -167,7 +166,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
txHashes[j].pop_back(); txHashes[j].pop_back();
} }
} }
mpool.removeForBlock(block, 265, dummyConflicted); mpool.removeForBlock(block, 265);
block.clear(); block.clear();
for (int i = 1; i < 10;i++) { for (int i = 1; i < 10;i++) {
BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee); BOOST_CHECK(mpool.estimateFee(i).GetFeePerK() > origFeeEst[i-1] - deltaFee);
@ -187,7 +186,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
block.push_back(*ptx); block.push_back(*ptx);
} }
} }
mpool.removeForBlock(block, ++blocknum, dummyConflicted); mpool.removeForBlock(block, ++blocknum);
block.clear(); block.clear();
} }
for (int i = 1; i < 10; i++) { for (int i = 1; i < 10; i++) {

View file

@ -503,7 +503,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
} }
} }
void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransaction>& removed) void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransaction>* removed)
{ {
// Remove transaction from memory pool // Remove transaction from memory pool
{ {
@ -530,8 +530,10 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransact
BOOST_FOREACH(txiter it, txToRemove) { BOOST_FOREACH(txiter it, txToRemove) {
CalculateDescendants(it, setAllRemoves); CalculateDescendants(it, setAllRemoves);
} }
BOOST_FOREACH(txiter it, setAllRemoves) { if (removed) {
removed.push_back(it->GetTx()); BOOST_FOREACH(txiter it, setAllRemoves) {
removed->push_back(it->GetTx());
}
} }
RemoveStaged(setAllRemoves, false); RemoveStaged(setAllRemoves, false);
} }
@ -574,7 +576,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
RemoveStaged(setAllRemoves, false); RemoveStaged(setAllRemoves, false);
} }
void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction>& removed) void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction>* removed)
{ {
// Remove transactions which depend on inputs of tx, recursively // Remove transactions which depend on inputs of tx, recursively
LOCK(cs); LOCK(cs);
@ -595,7 +597,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction>
* Called when a block is connected. Removes from mempool and updates the miner fee estimator. * Called when a block is connected. Removes from mempool and updates the miner fee estimator.
*/ */
void CTxMemPool::removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight, void CTxMemPool::removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight,
std::list<CTransaction>& conflicts, bool fCurrentEstimate) std::list<CTransaction>* conflicts, bool fCurrentEstimate)
{ {
LOCK(cs); LOCK(cs);
std::vector<CTxMemPoolEntry> entries; std::vector<CTxMemPoolEntry> entries;

View file

@ -521,11 +521,11 @@ public:
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true);
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
void removeRecursive(const CTransaction &tx, std::list<CTransaction>& removed); void removeRecursive(const CTransaction &tx, std::list<CTransaction>* removed = NULL);
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags);
void removeConflicts(const CTransaction &tx, std::list<CTransaction>& removed); void removeConflicts(const CTransaction &tx, std::list<CTransaction>* removed = NULL);
void removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight, void removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight,
std::list<CTransaction>& conflicts, bool fCurrentEstimate = true); std::list<CTransaction>* conflicts = NULL, bool fCurrentEstimate = true);
void clear(); void clear();
void _clear(); //lock free void _clear(); //lock free
bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);