Skip to content

Commit a1dcf2e

Browse files
committed
Merge #9240: Remove txConflicted
a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos) bf663f8 remove external usage of mempool conflict tracking (Alex Morcos)
2 parents d38b0d7 + a874ab5 commit a1dcf2e

File tree

5 files changed

+36
-45
lines changed

5 files changed

+36
-45
lines changed

src/test/blockencodings_tests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
8080

8181
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
8282

83-
std::vector<CTransactionRef> removed;
84-
pool.removeRecursive(*block.vtx[2], &removed);
85-
BOOST_CHECK_EQUAL(removed.size(), 1);
83+
size_t poolSize = pool.size();
84+
pool.removeRecursive(*block.vtx[2]);
85+
BOOST_CHECK_EQUAL(pool.size(), poolSize - 1);
8686

8787
CBlock block2;
8888
std::vector<CTransactionRef> vtx_missing;

src/test/mempool_tests.cpp

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,17 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
5555

5656

5757
CTxMemPool testPool(CFeeRate(0));
58-
std::vector<CTransactionRef> removed;
5958

6059
// Nothing in pool, remove should do nothing:
61-
testPool.removeRecursive(txParent, &removed);
62-
BOOST_CHECK_EQUAL(removed.size(), 0);
60+
unsigned int poolSize = testPool.size();
61+
testPool.removeRecursive(txParent);
62+
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
6363

6464
// Just the parent:
6565
testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
66-
testPool.removeRecursive(txParent, &removed);
67-
BOOST_CHECK_EQUAL(removed.size(), 1);
68-
removed.clear();
66+
poolSize = testPool.size();
67+
testPool.removeRecursive(txParent);
68+
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1);
6969

7070
// Parent, children, grandchildren:
7171
testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
@@ -75,19 +75,21 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
7575
testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
7676
}
7777
// Remove Child[0], GrandChild[0] should be removed:
78-
testPool.removeRecursive(txChild[0], &removed);
79-
BOOST_CHECK_EQUAL(removed.size(), 2);
80-
removed.clear();
78+
poolSize = testPool.size();
79+
testPool.removeRecursive(txChild[0]);
80+
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2);
8181
// ... make sure grandchild and child are gone:
82-
testPool.removeRecursive(txGrandChild[0], &removed);
83-
BOOST_CHECK_EQUAL(removed.size(), 0);
84-
testPool.removeRecursive(txChild[0], &removed);
85-
BOOST_CHECK_EQUAL(removed.size(), 0);
82+
poolSize = testPool.size();
83+
testPool.removeRecursive(txGrandChild[0]);
84+
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
85+
poolSize = testPool.size();
86+
testPool.removeRecursive(txChild[0]);
87+
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
8688
// Remove parent, all children/grandchildren should go:
87-
testPool.removeRecursive(txParent, &removed);
88-
BOOST_CHECK_EQUAL(removed.size(), 5);
89+
poolSize = testPool.size();
90+
testPool.removeRecursive(txParent);
91+
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5);
8992
BOOST_CHECK_EQUAL(testPool.size(), 0);
90-
removed.clear();
9193

9294
// Add children and grandchildren, but NOT the parent (simulate the parent being in a block)
9395
for (int i = 0; i < 3; i++)
@@ -97,10 +99,10 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
9799
}
98100
// Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be
99101
// put into the mempool (maybe because it is non-standard):
100-
testPool.removeRecursive(txParent, &removed);
101-
BOOST_CHECK_EQUAL(removed.size(), 6);
102+
poolSize = testPool.size();
103+
testPool.removeRecursive(txParent);
104+
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6);
102105
BOOST_CHECK_EQUAL(testPool.size(), 0);
103-
removed.clear();
104106
}
105107

106108
template<typename name>
@@ -412,7 +414,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
412414
/* after tx6 is mined, tx7 should move up in the sort */
413415
std::vector<CTransactionRef> vtx;
414416
vtx.push_back(MakeTransactionRef(tx6));
415-
pool.removeForBlock(vtx, 1, NULL, false);
417+
pool.removeForBlock(vtx, 1);
416418

417419
sortedOrder.erase(sortedOrder.begin()+1);
418420
sortedOrder.pop_back();

src/txmempool.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
503503
}
504504
}
505505

506-
void CTxMemPool::removeRecursive(const CTransaction &origTx, std::vector<CTransactionRef>* removed)
506+
void CTxMemPool::removeRecursive(const CTransaction &origTx)
507507
{
508508
// Remove transaction from memory pool
509509
{
@@ -530,11 +530,6 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, std::vector<CTransa
530530
BOOST_FOREACH(txiter it, txToRemove) {
531531
CalculateDescendants(it, setAllRemoves);
532532
}
533-
if (removed) {
534-
BOOST_FOREACH(txiter it, setAllRemoves) {
535-
removed->emplace_back(it->GetSharedTx());
536-
}
537-
}
538533
RemoveStaged(setAllRemoves, false);
539534
}
540535
}
@@ -576,7 +571,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
576571
RemoveStaged(setAllRemoves, false);
577572
}
578573

579-
void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector<CTransactionRef>* removed)
574+
void CTxMemPool::removeConflicts(const CTransaction &tx)
580575
{
581576
// Remove transactions which depend on inputs of tx, recursively
582577
LOCK(cs);
@@ -586,7 +581,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector<CTransactio
586581
const CTransaction &txConflict = *it->second;
587582
if (txConflict != tx)
588583
{
589-
removeRecursive(txConflict, removed);
584+
removeRecursive(txConflict);
590585
ClearPrioritisation(txConflict.GetHash());
591586
}
592587
}
@@ -597,7 +592,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector<CTransactio
597592
* Called when a block is connected. Removes from mempool and updates the miner fee estimator.
598593
*/
599594
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
600-
std::vector<CTransactionRef>* conflicts, bool fCurrentEstimate)
595+
bool fCurrentEstimate)
601596
{
602597
LOCK(cs);
603598
std::vector<CTxMemPoolEntry> entries;
@@ -617,7 +612,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
617612
stage.insert(it);
618613
RemoveStaged(stage, true);
619614
}
620-
removeConflicts(*tx, conflicts);
615+
removeConflicts(*tx);
621616
ClearPrioritisation(tx->GetHash());
622617
}
623618
// After the txs in the new block have been removed from the mempool, update policy estimates

src/txmempool.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,11 +527,11 @@ class CTxMemPool
527527
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true);
528528
bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true);
529529

530-
void removeRecursive(const CTransaction &tx, std::vector<CTransactionRef>* removed = NULL);
530+
void removeRecursive(const CTransaction &tx);
531531
void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags);
532-
void removeConflicts(const CTransaction &tx, std::vector<CTransactionRef>* removed = NULL);
532+
void removeConflicts(const CTransaction &tx);
533533
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
534-
std::vector<CTransactionRef>* conflicts = NULL, bool fCurrentEstimate = true);
534+
bool fCurrentEstimate = true);
535535
void clear();
536536
void _clear(); //lock free
537537
bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb);

src/validation.cpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,11 +2145,10 @@ static int64_t nTimeChainState = 0;
21452145
static int64_t nTimePostConnect = 0;
21462146

21472147
/**
2148-
* Used to track conflicted transactions removed from mempool and transactions
2149-
* applied to the UTXO state as a part of a single ActivateBestChainStep call.
2148+
* Used to track blocks whose transactions were applied to the UTXO state as a
2149+
* part of a single ActivateBestChainStep call.
21502150
*/
21512151
struct ConnectTrace {
2152-
std::vector<CTransactionRef> txConflicted;
21532152
std::vector<std::pair<CBlockIndex*, std::shared_ptr<const CBlock> > > blocksConnected;
21542153
};
21552154

@@ -2200,7 +2199,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams,
22002199
int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4;
22012200
LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001);
22022201
// Remove conflicting transactions from the mempool.;
2203-
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload());
2202+
mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, !IsInitialBlockDownload());
22042203
// Update chainActive & related variables.
22052204
UpdateTip(pindexNew, chainparams);
22062205

@@ -2425,11 +2424,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
24252424

24262425
// throw all transactions though the signal-interface
24272426
// while _not_ holding the cs_main lock
2428-
for (const auto& tx : connectTrace.txConflicted)
2429-
{
2430-
GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK);
2431-
}
2432-
// ... and about transactions that got confirmed:
24332427
for (const auto& pair : connectTrace.blocksConnected) {
24342428
assert(pair.second);
24352429
const CBlock& block = *(pair.second);

0 commit comments

Comments
 (0)