validation: Guard all chainstates with cs_main

Since these chainstates are:

1. Also vulnerable to the race condition described in the previous
   commit
2. Documented as having similar semantics as m_active_chainstate

we should also protect them with ::cs_main.

2 files changed, 10 insertions, 8 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 38df71b994..8f7d36bfd3 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5159,7 +5159,7 @@ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pin
 }
 
 Optional<uint256> ChainstateManager::SnapshotBlockhash() const {
-    LOCK(::cs_main);  // for m_active_chainstate access
+    LOCK(::cs_main);
     if (m_active_chainstate != nullptr) {
         // If a snapshot chainstate exists, it will always be our active.
         return m_active_chainstate->m_from_snapshot_blockhash;
@@ -5169,6 +5169,7 @@ Optional<uint256> ChainstateManager::SnapshotBlockhash() const {
 
 std::vector<CChainState*> ChainstateManager::GetAll()
 {
+    LOCK(::cs_main);
     std::vector<CChainState*> out;
 
     if (!IsSnapshotValidated() && m_ibd_chainstate) {
@@ -5213,11 +5214,13 @@ CChainState& ChainstateManager::ActiveChainstate() const
 
 bool ChainstateManager::IsSnapshotActive() const
 {
-    return m_snapshot_chainstate && WITH_LOCK(::cs_main, return m_active_chainstate) == m_snapshot_chainstate.get();
+    LOCK(::cs_main);
+    return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get();
 }
 
 CChainState& ChainstateManager::ValidatedChainstate() const
 {
+    LOCK(::cs_main);
     if (m_snapshot_chainstate && IsSnapshotValidated()) {
         return *m_snapshot_chainstate.get();
     }
@@ -5227,6 +5230,7 @@ CChainState& ChainstateManager::ValidatedChainstate() const
 
 bool ChainstateManager::IsBackgroundIBD(CChainState* chainstate) const
 {
+    LOCK(::cs_main);
     return (m_snapshot_chainstate && chainstate == m_ibd_chainstate.get());
 }
 
@@ -5242,12 +5246,10 @@ void ChainstateManager::Unload()
 
 void ChainstateManager::Reset()
 {
+    LOCK(::cs_main);
     m_ibd_chainstate.reset();
     m_snapshot_chainstate.reset();
-    {
-        LOCK(::cs_main);
-        m_active_chainstate = nullptr;
-    }
+    m_active_chainstate = nullptr;
     m_snapshot_validated = false;
 }
 
diff --git a/src/validation.h b/src/validation.h
index fc7add85b7..e85c7bbf1a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -802,7 +802,7 @@ private:
     //! This is especially important when, e.g., calling ActivateBestChain()
     //! on all chainstates because we are not able to hold ::cs_main going into
     //! that call.
-    std::unique_ptr<CChainState> m_ibd_chainstate;
+    std::unique_ptr<CChainState> m_ibd_chainstate GUARDED_BY(::cs_main);
 
     //! A chainstate initialized on the basis of a UTXO snapshot. If this is
     //! non-null, it is always our active chainstate.
@@ -815,7 +815,7 @@ private:
     //! This is especially important when, e.g., calling ActivateBestChain()
     //! on all chainstates because we are not able to hold ::cs_main going into
     //! that call.
-    std::unique_ptr<CChainState> m_snapshot_chainstate;
+    std::unique_ptr<CChainState> m_snapshot_chainstate GUARDED_BY(::cs_main);
 
     //! Points to either the ibd or snapshot chainstate; indicates our
     //! most-work chain.