Revamping block validation interface interaction with wallet

src/init.cpp

@@ -1908,7 +1908,7 @@ bool AppInitMain()
 
 #ifdef ENABLE_WALLET
     if (pwalletMain) {
-        pwalletMain->postInitProcess(threadGroup);
+        pwalletMain->postInitProcess(scheduler);
 
         // StakeMiner thread disabled by default on regtest
         if (gArgs.GetBoolArg("-staking", !Params().IsRegTestNet() && DEFAULT_STAKING)) {

src/miner.cpp

@@ -521,9 +521,6 @@ bool ProcessBlockFound(const std::shared_ptr<const CBlock>& pblock, CWallet& wal
     if (reservekey)
         reservekey->KeepKey();
 
-    // Inform about the new block
-    GetMainSignals().BlockFound(pblock->GetHash());
-
     // Process this block the same as if we had received it from another node
     CValidationState state;
     if (!ProcessNewBlock(state, nullptr, pblock, nullptr)) {

src/net.cpp

@@ -2142,7 +2142,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c
     threadMessageHandler = std::thread(&TraceThread<std::function<void()> >, "msghand", std::function<void()>(std::bind(&CConnman::ThreadMessageHandler, this)));
 
     // Dump network addresses
-    scheduler.scheduleEvery(std::bind(&CConnman::DumpData, this), DUMP_ADDRESSES_INTERVAL);
+    scheduler.scheduleEvery(std::bind(&CConnman::DumpData, this), DUMP_ADDRESSES_INTERVAL * 1000);
 
     return true;
 }

src/net_processing.h

@@ -41,8 +41,8 @@ class PeerLogicValidation : public CValidationInterface {
     PeerLogicValidation(CConnman* connmanIn);
     ~PeerLogicValidation() = default;
 
-    virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload);
-    virtual void BlockChecked(const CBlock& block, const CValidationState& state);
+    void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;
+    void BlockChecked(const CBlock& block, const CValidationState& state) override;
 };
 
 struct CNodeStateStats {

src/sapling/saplingscriptpubkeyman.cpp

@@ -300,11 +300,11 @@ void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t n
     }
 }
 
-void SaplingScriptPubKeyMan::DecrementNoteWitnesses(const CBlockIndex* pindex)
+void SaplingScriptPubKeyMan::DecrementNoteWitnesses(int nChainHeight)
 {
     LOCK(wallet->cs_wallet);
     for (std::pair<const uint256, CWalletTx>& wtxItem : wallet->mapWallet) {
-        ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize);
+        ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, nChainHeight, nWitnessCacheSize);
     }
     nWitnessCacheSize -= 1;
     nWitnessCacheNeedsUpdate = true;

src/sapling/saplingscriptpubkeyman.h

@@ -72,7 +72,7 @@ class SaplingNoteData
      * Block height corresponding to the most current witness.
      *
      * When we first create a SaplingNoteData in SaplingScriptPubKeyMan::FindMySaplingNotes, this is set to
-     * -1 as a placeholder. The next time CWallet::ChainTip is called, we can
+     * -1 as a placeholder. The next time CWallet::BlockConnected/CWallet::BlockDisconnected is called, we can
      * determine what height the witness cache for this note is valid for (even
      * if no witnesses were cached), and so can set the correct value in
      * SaplingScriptPubKeyMan::IncrementNoteWitnesses and SaplingScriptPubKeyMan::DecrementNoteWitnesses.
@@ -163,9 +163,9 @@ class SaplingScriptPubKeyMan {
                                 const CBlock* pblock,
                                 SaplingMerkleTree& saplingTree);
     /**
-     * pindex is the old tip being disconnected.
+     * nChainHeight is the old tip height being disconnected.
      */
-    void DecrementNoteWitnesses(const CBlockIndex* pindex);
+    void DecrementNoteWitnesses(int nChainHeight);
 
     /**
      * Update mapSaplingNullifiersToNotes

src/scheduler.cpp

@@ -93,20 +93,20 @@ void CScheduler::schedule(CScheduler::Function f, boost::chrono::system_clock::t
     newTaskScheduled.notify_one();
 }
 
-void CScheduler::scheduleFromNow(CScheduler::Function f, int64_t deltaSeconds)
+void CScheduler::scheduleFromNow(CScheduler::Function f, int64_t deltaMilliSeconds)
 {
-    schedule(f, boost::chrono::system_clock::now() + boost::chrono::seconds(deltaSeconds));
+    schedule(f, boost::chrono::system_clock::now() + boost::chrono::milliseconds(deltaMilliSeconds));
 }
 
-static void Repeat(CScheduler* s, CScheduler::Function f, int64_t deltaSeconds)
+static void Repeat(CScheduler* s, CScheduler::Function f, int64_t deltaMilliSeconds)
 {
     f();
-    s->scheduleFromNow(std::bind(&Repeat, s, f, deltaSeconds), deltaSeconds);
+    s->scheduleFromNow(std::bind(&Repeat, s, f, deltaMilliSeconds), deltaMilliSeconds);
 }
 
-void CScheduler::scheduleEvery(CScheduler::Function f, int64_t deltaSeconds)
+void CScheduler::scheduleEvery(CScheduler::Function f, int64_t deltaMilliSeconds)
 {
-    scheduleFromNow(std::bind(&Repeat, this, f, deltaSeconds), deltaSeconds);
+    scheduleFromNow(std::bind(&Repeat, this, f, deltaMilliSeconds), deltaMilliSeconds);
 }
 
 size_t CScheduler::getQueueInfo(boost::chrono::system_clock::time_point &first,

src/scheduler.h

@@ -44,15 +44,15 @@ class CScheduler
     // Call func at/after time t
     void schedule(Function f, boost::chrono::system_clock::time_point t);
 
-    // Convenience method: call f once deltaSeconds from now
-    void scheduleFromNow(Function f, int64_t deltaSeconds);
+    // Convenience method: call f once deltaMilliSeconds from now
+    void scheduleFromNow(Function f, int64_t deltaMilliSeconds);
 
     // Another convenience method: call f approximately
-    // every deltaSeconds forever, starting deltaSeconds from now.
+    // every deltaMilliSeconds forever, starting deltaMilliSeconds from now.
     // To be more precise: every time f is finished, it
-    // is rescheduled to run deltaSeconds later. If you
+    // is rescheduled to run deltaMilliSeconds later. If you
     // need more accurate scheduling, don't use this method.
-    void scheduleEvery(Function f, int64_t deltaSeconds);
+    void scheduleEvery(Function f, int64_t deltaMilliSeconds);
 
     // To keep things as simple as possible, there is no unschedule.
 

src/test/mempool_tests.cpp

@@ -53,18 +53,18 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
 
 
     CTxMemPool testPool(CFeeRate(0));
-    std::vector<CTransactionRef> removed;
 
     // Nothing in pool, remove should do nothing:
-    testPool.removeRecursive(txParent, &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 0);
+    unsigned int poolSize = testPool.size();
+    testPool.removeRecursive(txParent);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize);
 
     // Just the parent:
     testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
-    testPool.removeRecursive(txParent, &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 1);
-    removed.clear();
-    
+    poolSize = testPool.size();
+    testPool.removeRecursive(txParent);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1);
+
     // Parent, children, grandchildren:
     testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent));
     for (int i = 0; i < 3; i++)
@@ -73,19 +73,21 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
         testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i]));
     }
     // Remove Child[0], GrandChild[0] should be removed:
-    testPool.removeRecursive(txChild[0], &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 2);
-    removed.clear();
+    poolSize = testPool.size();
+    testPool.removeRecursive(txChild[0]);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2);
     // ... make sure grandchild and child are gone:
-    testPool.removeRecursive(txGrandChild[0], &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 0);
-    testPool.removeRecursive(txChild[0], &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 0);
+    poolSize = testPool.size();
+    testPool.removeRecursive(txGrandChild[0]);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize);
+    poolSize = testPool.size();
+    testPool.removeRecursive(txChild[0]);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize);
     // Remove parent, all children/grandchildren should go:
-    testPool.removeRecursive(txParent, &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 5);
+    poolSize = testPool.size();
+    testPool.removeRecursive(txParent);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
-    removed.clear();
 
     // Add children and grandchildren, but NOT the parent (simulate the parent being in a block)
     for (int i = 0; i < 3; i++)
@@ -95,10 +97,10 @@ 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):
-    testPool.removeRecursive(txParent, &removed);
-    BOOST_CHECK_EQUAL(removed.size(), 6);
+    poolSize = testPool.size();
+    testPool.removeRecursive(txParent);
+    BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6);
     BOOST_CHECK_EQUAL(testPool.size(), 0);
-    removed.clear();
 }
 
 template<typename name>
@@ -415,7 +417,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
     /* after tx6 is mined, tx7 should move up in the sort */
     std::vector<CTransactionRef> vtx;
     vtx.emplace_back(MakeTransactionRef(tx6));
-    pool.removeForBlock(vtx, 1, nullptr, false);
+    pool.removeForBlock(vtx, 1);
 
     sortedOrder.erase(sortedOrder.begin()+1);
     // Ties are broken by hash

src/txmempool.cpp

@@ -368,13 +368,12 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n)
     nTransactionsUpdated += n;
 }
 
-
 bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate)
 {
+    NotifyEntryAdded(entry.GetSharedTx());
     // Add to memory pool without checking anything.
     // Used by AcceptToMemoryPool(), which DOES do all the appropriate checks.
     LOCK(cs);
-
     indexed_transaction_set::iterator newit = mapTx.insert(entry).first;
     mapLinks.insert(make_pair(newit, TxLinks()));
 
@@ -433,8 +432,9 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
     return true;
 }
 
-void CTxMemPool::removeUnchecked(txiter it)
+void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
 {
+    NotifyEntryRemoved(it->GetSharedTx(), reason);
     AssertLockHeld(cs);
     const CTransaction& tx = it->GetTx();
     for (const CTxIn& txin : tx.vin)
@@ -483,7 +483,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants
     }
 }
 
-void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vector<CTransactionRef>* removed)
+void CTxMemPool::removeRecursive(const CTransaction& origTx, MemPoolRemovalReason reason)
 {
     // Remove transaction from memory pool
     {
@@ -510,12 +510,8 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vector<CTransa
         for (const txiter& it : txToRemove) {
             CalculateDescendants(it, setAllRemoves);
         }
-        if (removed) {
-            for (const txiter& it : setAllRemoves) {
-                removed->emplace_back(it->GetSharedTx());
-            }
-        }
-        RemoveStaged(setAllRemoves, false);
+
+        RemoveStaged(setAllRemoves, false, reason);
     }
 }
 
@@ -547,7 +543,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
     for (txiter it : txToRemove) {
         CalculateDescendants(it, setAllRemoves);
     }
-    RemoveStaged(setAllRemoves, false);
+    RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
 }
 
 void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
@@ -574,7 +570,7 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot)
     }
 }
 
-void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector<CTransactionRef>* removed)
+void CTxMemPool::removeConflicts(const CTransaction& tx)
 {
     // Remove transactions which depend on inputs of tx, recursively
     std::list<CTransaction> result;
@@ -584,7 +580,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector<CTransactio
         if (it != mapNextTx.end()) {
             const CTransaction& txConflict = *it->second;
             if (txConflict != tx) {
-                removeRecursive(txConflict, removed);
+                removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT);
+                ClearPrioritisation(txConflict.GetHash());
             }
         }
     }
@@ -595,7 +592,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector<CTransactio
             if (it != mapSaplingNullifiers.end()) {
                 const CTransaction& txConflict = *it->second;
                 if (txConflict != tx) {
-                    removeRecursive(txConflict, removed);
+                    removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT);
+                    ClearPrioritisation(txConflict.GetHash());
                 }
             }
         }
@@ -606,7 +604,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector<CTransactio
  * Called when a block is connected. Removes from mempool and updates the miner fee estimator.
  */
 void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight,
-                                std::vector<CTransactionRef>* conflicts, bool fCurrentEstimate)
+                                bool fCurrentEstimate)
 {
     LOCK(cs);
     std::vector<CTxMemPoolEntry> entries;
@@ -621,9 +619,9 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
         if (it != mapTx.end()) {
             setEntries stage;
             stage.insert(it);
-            RemoveStaged(stage, true);
+            RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
         }
-        removeConflicts(*tx, conflicts);
+        removeConflicts(*tx);
         ClearPrioritisation(tx->GetHash());
     }
     // After the txs in the new block have been removed from the mempool, update policy estimates
@@ -1064,12 +1062,12 @@ size_t CTxMemPool::DynamicMemoryUsage() const
             memusage::DynamicUsage(mapSaplingNullifiers);
 }
 
-void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants)
+void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason)
 {
     AssertLockHeld(cs);
     UpdateForRemoveFromMempool(stage, updateDescendants);
     for (const txiter& it : stage) {
-        removeUnchecked(it);
+        removeUnchecked(it, reason);
     }
 }
 
@@ -1086,7 +1084,7 @@ int CTxMemPool::Expire(int64_t time)
     for (const txiter& removeit : toremove) {
         CalculateDescendants(removeit, stage);
     }
-    RemoveStaged(stage, false);
+    RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY);
     return stage.size();
 }
 
@@ -1197,7 +1195,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
             for (txiter it: stage)
                 txn.push_back(it->GetTx());
         }
-        RemoveStaged(stage, false);
+        RemoveStaged(stage, false, MemPoolRemovalReason::SIZELIMIT);
         if (pvNoSpendsRemaining) {
             for (const CTransaction& tx: txn) {
                 for (const CTxIn& txin: tx.vin) {