Remove remaining chainActive references from CWallet

ryanofsky · Empact · ryanofsky · commit ca76bcc917db · 2019-01-10T06:56:54.000-08:00
This commit does not change behavior.

Co-authored-by: Ben Woosley &lt;ben.woosley@gmail.com&gt;
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
@@ -61,6 +61,11 @@ class LockImpl : public Chain::Lock
         assert(block != nullptr);
         return block->GetMedianTimePast();
     }
+    bool haveBlockOnDisk(int height) override
+    {
+        CBlockIndex* block = ::chainActive[height];
+        return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
+    }
     Optional<int> findFirstBlockWithTime(int64_t time) override
     {
         CBlockIndex* block = ::chainActive.FindEarliestAtLeast(time);
@@ -111,6 +116,21 @@ class LockImpl : public Chain::Lock
         }
         return nullopt;
     }
+    bool isPotentialTip(const uint256& hash) override
+    {
+        if (::chainActive.Tip()->GetBlockHash() == hash) return true;
+        auto it = ::mapBlockIndex.find(hash);
+        return it != ::mapBlockIndex.end() && it->second->GetAncestor(::chainActive.Height()) == ::chainActive.Tip();
+    }
+    CBlockLocator getLocator() override { return ::chainActive.GetLocator(); }
+    Optional<int> findLocatorFork(const CBlockLocator& locator) override
+    {
+        LockAnnotation lock(::cs_main);
+        if (CBlockIndex* fork = FindForkInGlobalIndex(::chainActive, locator)) {
+            return fork->nHeight;
+        }
+        return nullopt;
+    }
 };
 
 class LockingStateImpl : public LockImpl, public UniqueLock<CCriticalSection>
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
@@ -15,6 +15,7 @@
 class CBlock;
 class CScheduler;
 class uint256;
+struct CBlockLocator;
 
 namespace interfaces {
 
@@ -57,6 +58,10 @@ class Chain
         //! Get block median time past.
         virtual int64_t getBlockMedianTimePast(int height) = 0;
 
+        //! Check that the full block is available on disk (ie has not been
+        //! pruned), and contains transactions.
+        virtual bool haveBlockOnDisk(int height) = 0;
+
         //! Return height of the first block in the chain with timestamp equal
         //! or greater than the given time, or nothing if there is no block with
         //! a high enough timestamp.
@@ -79,6 +84,17 @@ class Chain
         //! of the specified block. Also return the height of the specified
         //! block as an optional output parameter.
         virtual Optional<int> findFork(const uint256& hash, Optional<int>* height) = 0;
+
+        //! Return true if block hash points to the current chain tip, or to a
+        //! possible descendant of the current chain tip that isn't currently
+        //! connected.
+        virtual bool isPotentialTip(const uint256& hash) = 0;
+
+        //! Get locator for the current chain tip.
+        virtual CBlockLocator getLocator() = 0;
+
+        //! Return height of block on the chain using locator.
+        virtual Optional<int> findLocatorFork(const CBlockLocator& locator) = 0;
     };
 
     //! Return Lock interface. Chain is locked when this is called, and
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -1127,7 +1127,7 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
         TransactionRemovedFromMempool(pblock->vtx[i]);
     }
 
-    m_last_block_processed = pindex;
+    m_last_block_processed = pindex->GetBlockHash();
 }
 
 void CWallet::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock) {
@@ -1152,9 +1152,8 @@ void CWallet::BlockUntilSyncedToCurrentChain() {
         // protected by cs_wallet instead of cs_main, but as long as we need
         // cs_main here anyway, it's easier to just call it cs_main-protected.
         auto locked_chain = chain().lock();
-        const CBlockIndex* initialChainTip = chainActive.Tip();
 
-        if (m_last_block_processed && m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) {
+        if (!m_last_block_processed.IsNull() && locked_chain->isPotentialTip(m_last_block_processed)) {
             return;
         }
     }
@@ -4041,7 +4040,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
         }
 
         auto locked_chain = chain.assumeLocked();  // Temporary. Removed in upcoming lock cleanup
-        walletInstance->ChainStateFlushed(chainActive.GetLocator());
+        walletInstance->ChainStateFlushed(locked_chain->getLocator());
     } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) {
         // Make it impossible to disable private keys after creation
         InitError(strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile));
@@ -4128,58 +4127,68 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
     // Try to top up keypool. No-op if the wallet is locked.
     walletInstance->TopUpKeyPool();
 
-    LockAnnotation lock(::cs_main); // Temporary, for FindForkInGlobalIndex below. Removed in upcoming commit.
     auto locked_chain = chain.lock();
     LOCK(walletInstance->cs_wallet);
 
-    CBlockIndex *pindexRescan = chainActive.Genesis();
+    int rescan_height = 0;
     if (!gArgs.GetBoolArg("-rescan", false))
     {
         WalletBatch batch(*walletInstance->database);
         CBlockLocator locator;
-        if (batch.ReadBestBlock(locator))
-            pindexRescan = FindForkInGlobalIndex(chainActive, locator);
+        if (batch.ReadBestBlock(locator)) {
+            if (const Optional<int> fork_height = locked_chain->findLocatorFork(locator)) {
+                rescan_height = *fork_height;
+            }
+        }
     }
 
-    walletInstance->m_last_block_processed = chainActive.Tip();
+    const Optional<int> tip_height = locked_chain->getHeight();
+    if (tip_height) {
+        walletInstance->m_last_block_processed = locked_chain->getBlockHash(*tip_height);
+    } else {
+        walletInstance->m_last_block_processed.SetNull();
+    }
 
-    if (chainActive.Tip() && chainActive.Tip() != pindexRescan)
+    if (tip_height && *tip_height != rescan_height)
     {
         //We can't rescan beyond non-pruned blocks, stop and throw an error
         //this might happen if a user uses an old wallet within a pruned node
         // or if he ran -disablewallet for a longer time, then decided to re-enable
         if (fPruneMode)
         {
-            CBlockIndex *block = chainActive.Tip();
-            while (block && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA) && block->pprev->nTx > 0 && pindexRescan != block)
-                block = block->pprev;
+            int block_height = *tip_height;
+            while (block_height > 0 && locked_chain->haveBlockOnDisk(block_height - 1) && rescan_height != block_height) {
+                --block_height;
+            }
 
-            if (pindexRescan != block) {
+            if (rescan_height != block_height) {
                 InitError(_("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"));
                 return nullptr;
             }
         }
 
         uiInterface.InitMessage(_("Rescanning..."));
-        walletInstance->WalletLogPrintf("Rescanning last %i blocks (from block %i)...\n", chainActive.Height() - pindexRescan->nHeight, pindexRescan->nHeight);
+        walletInstance->WalletLogPrintf("Rescanning last %i blocks (from block %i)...\n", *tip_height - rescan_height, rescan_height);
 
         // No need to read and scan block if block was created before
         // our wallet birthday (as adjusted for block time variability)
-        while (pindexRescan && walletInstance->nTimeFirstKey && (pindexRescan->GetBlockTime() < (walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW))) {
-            pindexRescan = chainActive.Next(pindexRescan);
+        if (walletInstance->nTimeFirstKey) {
+            if (Optional<int> first_block = locked_chain->findFirstBlockWithTimeAndHeight(walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW, rescan_height)) {
+                rescan_height = *first_block;
+            }
         }
 
         nStart = GetTimeMillis();
         {
             WalletRescanReserver reserver(walletInstance.get());
             uint256 stop_block, failed_block;
-            if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(pindexRescan->GetBlockHash(), {} /* stop block */, reserver, failed_block, stop_block, true /* update */))) {
+            if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), {} /* stop block */, reserver, failed_block, stop_block, true /* update */))) {
                 InitError(_("Failed to rescan the wallet during initialization"));
                 return nullptr;
             }
         }
         walletInstance->WalletLogPrintf("Rescan completed in %15dms\n", GetTimeMillis() - nStart);
-        walletInstance->ChainStateFlushed(chainActive.GetLocator());
+        walletInstance->ChainStateFlushed(locked_chain->getLocator());
         walletInstance->database->IncrementUpdateCounter();
 
         // Restore wallet transaction metadata after -zapwallettxes=1
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -88,7 +88,6 @@ static const bool DEFAULT_DISABLE_WALLET = false;
 //! Pre-calculated constants for input size estimation in *virtual size*
 static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91;
 
-class CBlockIndex;
 class CCoinControl;
 class COutput;
 class CReserveKey;
@@ -716,10 +715,8 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
      * Note that this is *not* how far we've processed, we may need some rescan
      * to have seen all transactions in the chain, but is only used to track
      * live BlockConnected callbacks.
-     *
-     * Protected by cs_main (see BlockUntilSyncedToCurrentChain)
      */
-    const CBlockIndex* m_last_block_processed = nullptr;
+    uint256 m_last_block_processed;
 
 public:
     /*
