Rework mempool accounts view

src/txmempool.cpp

@@ -333,6 +333,7 @@ CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator)
     // accepting transactions becomes O(N^2) where N is the number
     // of transactions in the pool
     nCheckFrequency = 0;
+    accountsViewDirty = false;
 }
 
 bool CTxMemPool::isSpent(const COutPoint& outpoint) const
@@ -570,59 +571,27 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
 {
     AssertLockHeld(cs);
 
-    setEntries staged;
     std::vector<const CTxMemPoolEntry*> entries;
     for (const auto& tx : vtx) {
         auto it = mapTx.find(tx->GetHash());
         if (it != mapTx.end()) {
-            staged.insert(it);
             entries.push_back(&*it);
         }
     }
     // Before the txs in the new block have been removed from the mempool, update policy estimates
     if (minerPolicyEstimator) {minerPolicyEstimator->processBlock(nBlockHeight, entries);}
 
-    for (auto& it : staged) {
-        auto& tx = it->GetTx();
-        removeConflicts(tx);
-        ClearPrioritisation(tx.GetHash());
-    }
-
-    RemoveStaged(staged, true, MemPoolRemovalReason::BLOCK);
-
-    if (pcustomcsview) { // can happen in tests
-        // check entire mempool
-        CAmount txfee = 0;
-        accountsView().Discard();
-        CCustomCSView viewDuplicate(accountsView());
-        CCoinsViewCache mempoolDuplicate(&::ChainstateActive().CoinsTip());
-
-        setEntries staged;
-        // Check custom TX consensus types are now not in conflict with account layer
-        auto& txsByEntryTime = mapTx.get<entry_time>();
-        for (auto it = txsByEntryTime.begin(); it != txsByEntryTime.end(); ++it) {
-            CValidationState state;
-            const auto& tx = it->GetTx();
-            if (!Consensus::CheckTxInputs(tx, state, mempoolDuplicate, &viewDuplicate, nBlockHeight, txfee, Params())) {
-                LogPrintf("%s: Remove conflicting TX: %s\n", __func__, tx.GetHash().GetHex());
-                staged.insert(mapTx.project<0>(it));
-                continue;
-            }
-            auto res = ApplyCustomTx(viewDuplicate, mempoolDuplicate, tx, Params().GetConsensus(), nBlockHeight);
-            if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) {
-                LogPrintf("%s: Remove conflicting custom TX: %s\n", __func__, tx.GetHash().GetHex());
-                staged.insert(mapTx.project<0>(it));
-            }
-        }
-
-        for (auto& it : staged) {
-            auto& tx = it->GetTx();
-            removeConflicts(tx);
-            ClearPrioritisation(tx.GetHash());
+    for (const auto& tx : vtx) {
+        auto it = mapTx.find(tx->GetHash());
+        if (it != mapTx.end()) {
+            RemoveStaged({it}, true, MemPoolRemovalReason::BLOCK);
         }
+        removeConflicts(*tx);
+        ClearPrioritisation(tx->GetHash());
+    }
 
-        RemoveStaged(staged, true, MemPoolRemovalReason::BLOCK);
-        viewDuplicate.Flush();
+    if (pcustomcsview) {
+        rebuildAccountsView(nBlockHeight, &::ChainstateActive().CoinsTip());
     }
 
     lastRollingFeeUpdate = GetTime();
@@ -975,27 +944,10 @@ size_t CTxMemPool::DynamicMemoryUsage() const {
 void CTxMemPool::RemoveStaged(const setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) {
     AssertLockHeld(cs);
     UpdateForRemoveFromMempool(stage, updateDescendants);
-    std::set<uint256> txids;
     for (txiter it : stage) {
-        txids.insert(it->GetTx().GetHash());
         removeUnchecked(it, reason);
     }
-    if (pcustomcsview && !txids.empty()) {
-        auto& view = accountsView();
-        std::map<uint32_t, uint256> orderedTxs;
-        auto it = NewKVIterator<CUndosView::ByUndoKey>(UndoKey{}, view.GetStorage().GetRaw());
-        for (; it.Valid() && !txids.empty(); it.Next()) {
-            auto& key = it.Key();
-            auto itTx = txids.find(key.txid);
-            if (itTx != txids.end()) {
-                orderedTxs.emplace(key.height, key.txid);
-                txids.erase(itTx);
-            }
-        }
-        for (auto it = orderedTxs.rbegin(); it != orderedTxs.rend(); ++it) {
-            view.OnUndoTx(it->second, it->first);
-        }
-    }
+    accountsViewDirty = accountsViewDirty || !stage.empty();
 }
 
 int CTxMemPool::Expire(int64_t time) {
@@ -1134,6 +1086,48 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
     }
 }
 
+void CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCache)
+{
+    if (!pcustomcsview || !accountsViewDirty) {
+        return;
+    }
+
+    CAmount txfee = 0;
+    accountsView().Discard();
+    CCustomCSView viewDuplicate(accountsView());
+
+    setEntries staged;
+    std::vector<CTransactionRef> vtx;
+    // Check custom TX consensus types are now not in conflict with account layer
+    auto& txsByEntryTime = mapTx.get<entry_time>();
+    for (auto it = txsByEntryTime.begin(); it != txsByEntryTime.end(); ++it) {
+        CValidationState state;
+        const auto& tx = it->GetTx();
+        if (!Consensus::CheckTxInputs(tx, state, coinsCache, &viewDuplicate, height, txfee, Params())) {
+            LogPrintf("%s: Remove conflicting TX: %s\n", __func__, tx.GetHash().GetHex());
+            staged.insert(mapTx.project<0>(it));
+            vtx.push_back(it->GetSharedTx());
+            continue;
+        }
+        auto res = ApplyCustomTx(viewDuplicate, coinsCache, tx, Params().GetConsensus(), height);
+        if (!res && (res.code & CustomTxErrCodes::Fatal)) {
+            LogPrintf("%s: Remove conflicting custom TX: %s\n", __func__, tx.GetHash().GetHex());
+            staged.insert(mapTx.project<0>(it));
+            vtx.push_back(it->GetSharedTx());
+        }
+    }
+
+    RemoveStaged(staged, true, MemPoolRemovalReason::BLOCK);
+
+    for (const auto& tx : vtx) {
+        removeConflicts(*tx);
+        ClearPrioritisation(tx->GetHash());
+    }
+
+    viewDuplicate.Flush();
+    accountsViewDirty = false;
+}
+
 uint64_t CTxMemPool::CalculateDescendantMaximum(txiter entry) const {
     // find parent with highest descendant count
     std::vector<txiter> candidates;

src/txmempool.h

@@ -548,6 +548,7 @@ class CTxMemPool
 
     std::vector<indexed_transaction_set::const_iterator> GetSortedDepthAndScore() const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
+    bool accountsViewDirty;
     std::unique_ptr<CCustomCSView> acview;
 public:
     indirectmap<COutPoint, const CTransaction*> mapNextTx GUARDED_BY(cs);
@@ -704,6 +705,7 @@ class CTxMemPool
     boost::signals2::signal<void (CTransactionRef, MemPoolRemovalReason)> NotifyEntryRemoved;
 
     CCustomCSView& accountsView();
+    void rebuildAccountsView(int height, const CCoinsViewCache& coinsCache);
 private:
     /** UpdateForDescendants is used by UpdateTransactionsFromBlock to update
      *  the descendants for a single transaction that has been added to the

src/validation.cpp

@@ -613,7 +613,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
 
         const auto height = GetSpendHeight(view);
 
-        // it does not need to check mempool anymore it has view there
+        // rebuild accounts view if dirty
+        pool.rebuildAccountsView(height, view);
 
         CAmount nFees = 0;
         if (!Consensus::CheckTxInputs(tx, state, view, &mnview, height, nFees, chainparams)) {
@@ -911,14 +912,14 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
 
         // Store transaction in memory
         pool.addUnchecked(entry, setAncestors, validForFeeEstimation);
+        mnview.Flush();
 
         // trim mempool and check if tx was trimmed
         if (!bypass_limits) {
             LimitMempoolSize(pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
             if (!pool.exists(hash))
                 return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INSUFFICIENTFEE, "mempool full");
         }
-        mnview.Flush();
     }
 
     GetMainSignals().TransactionAddedToMempool(ptx);

test/functional/rpc_mn_basic.py

@@ -148,9 +148,11 @@ def run_test(self):
         self.nodes[2].generate(35)
         connect_nodes_bi(self.nodes, 0, 2)
         self.sync_blocks(self.nodes[0:3])
+
         assert_equal(len(self.nodes[0].listmasternodes()), 8)
-        # fundingTx is removed for a block
-        assert_equal(len(self.nodes[0].getrawmempool()), 1) # auto auth
+        mempool = self.nodes[0].getrawmempool()
+        assert(idnode0 in mempool and fundingTx in mempool)
+        assert_equal(len(mempool), 3) # + auto auth
 
         collateral0 = self.nodes[0].getnewaddress("", "legacy")
         self.nodes[0].createmasternode(collateral0)