Merge #16433: txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN

0000ff0aa763e3be524ac4537a41048a26529fb2 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke)

Pull request description:

  The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default.

ACKs for top commit:
  practicalswift:
    utACK 0000ff0aa763e3be524ac4537a41048a26529fb2
  promag:
    ACK 0000ff0aa763e3be524ac4537a41048a26529fb2.
  jonasschnelli:
    utACK 0000ff0aa763e3be524ac4537a41048a26529fb2

Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
This commit is contained in:
Jonas Schnelli 2019-07-30 22:01:06 +02:00 committed by Vijay Das Manikpuri
parent 8a84a9b7c0
commit 551ce4e3e4
No known key found for this signature in database
GPG Key ID: DB1D81B01DB7C46E
5 changed files with 22 additions and 20 deletions

View File

@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
size_t poolSize = pool.size();
pool.removeRecursive(*block.vtx[2]);
pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::MANUAL);
BOOST_CHECK_EQUAL(pool.size(), poolSize - 1);
CBlock block2;

View File

@ -14,6 +14,8 @@
BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup)
static constexpr auto REMOVAL_REASON_DUMMY = MemPoolRemovalReason::MANUAL;
BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
{
// Test CTxMemPool::remove functionality
@ -59,13 +61,13 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
// Nothing in pool, remove should do nothing:
unsigned int poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txParent));
testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
// Just the parent:
testPool.addUnchecked(entry.FromTx(txParent));
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txParent));
testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1);
// Parent, children, grandchildren:
@ -77,18 +79,18 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
}
// Remove Child[0], GrandChild[0] should be removed:
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txChild[0]));
testPool.removeRecursive(CTransaction(txChild[0]), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2);
// ... make sure grandchild and child are gone:
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txGrandChild[0]));
testPool.removeRecursive(CTransaction(txGrandChild[0]), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txChild[0]));
testPool.removeRecursive(CTransaction(txChild[0]), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize);
// Remove parent, all children/grandchildren should go:
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txParent));
testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5);
BOOST_CHECK_EQUAL(testPool.size(), 0U);
@ -101,7 +103,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
// 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):
poolSize = testPool.size();
testPool.removeRecursive(CTransaction(txParent));
testPool.removeRecursive(CTransaction(txParent), REMOVAL_REASON_DUMMY);
BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6);
BOOST_CHECK_EQUAL(testPool.size(), 0U);
}
@ -283,11 +285,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
BOOST_CHECK_EQUAL(pool.size(), 10U);
// Now try removing tx10 and verify the sort order returns to normal
pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
CheckSort<descendant_score>(pool, snapshotOrder);
pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx());
pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
}
BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)

View File

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

View File

@ -351,12 +351,12 @@ struct TxMempoolInfo
* this is passed to the notification signal.
*/
enum class MemPoolRemovalReason {
UNKNOWN = 0, //!< Manually removed or unknown reason
EXPIRY, //!< Expired from mempool
SIZELIMIT, //!< Removed in size limiting
REORG, //!< Removed for reorganization
BLOCK, //!< Removed for block
CONFLICT, //!< Removed for conflict with in-block transaction
MANUAL //!< Removed manually
};
class SaltedTxidHasher
@ -614,7 +614,7 @@ public:
bool getSpentIndex(CSpentIndexKey &key, CSpentIndexValue &value);
bool removeSpentIndex(const uint256 txhash);
void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN);
void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason);
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 removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs);
@ -659,7 +659,7 @@ public:
* Set updateDescendants to true when removing a tx that was in a block, so
* that any in-mempool descendants have their ancestor state updated.
*/
void RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs);
void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
/** When adding transactions from a disconnected block back to the mempool,
* new mempool entries may have children in the mempool (which is generally
@ -777,7 +777,7 @@ private:
* transactions in a chain before we've updated all the state for the
* removal.
*/
void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
public:
/** EpochGuard: RAII-style guard for using epoch-based graph traversal algorithms.
* When walking ancestors or descendants, we generally want to avoid

View File

@ -1422,13 +1422,13 @@ void CWallet::BlockConnected(const CBlock& block, const std::vector<CTransaction
for (const CTransactionRef& ptx : vtxConflicted) {
SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */);
// UNKNOWN because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(ptx, MemPoolRemovalReason::UNKNOWN);
// MANUAL because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(ptx, MemPoolRemovalReason::MANUAL);
}
for (size_t i = 0; i < block.vtx.size(); i++) {
SyncTransaction(block.vtx[i], block_hash, i);
// UNKNOWN because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(block.vtx[i], MemPoolRemovalReason::UNKNOWN);
// MANUAL because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(block.vtx[i], MemPoolRemovalReason::MANUAL);
}
m_last_block_processed = block_hash;