Repository: bitcoin/bitcoin
File: src/validation.h (C/C++)

Code before:

    //! A queue for script verifications that have to be performed by worker threads.
    CCheckQueue<CScriptCheck> m_script_check_queue;

    //! Timers and counters used for benchmarking validation in both background
    //! and active chainstates.
    SteadyClock::duration GUARDED_BY(::cs_main) time_check{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_forks{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_connect{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_verify{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_undo{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_index{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_total{};
    int64_t GUARDED_BY(::cs_main) num_blocks_total{0};
    SteadyClock::duration GUARDED_BY(::cs_main) time_connect_total{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_flush{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_chainstate{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_post_connect{};

public:
    using Options = kernel::ChainstateManagerOpts;

    explicit ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options);

    //! Function to restart active indexes; set dynamically to avoid a circular
    //! dependency on `base/index.cpp`.
    std::function<void()> snapshot_download_completed = std::function<void()>();

    const CChainParams& GetParams() const { return m_options.chainparams; }
    const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); }
    bool ShouldCheckBlockIndex() const;
    const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); }
    const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
    kernel::Notifications& GetNotifications() const { return m_options.notifications; };

    /**
     * Make various assertions about the state of the block index.
     *
     * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index.
     */
    void CheckBlockIndex();

    /**
     * Alias for ::cs_main.
     * Should be used in new code to make it easier to make ::cs_main a member
     * of this class.
     * Generally, methods of this class should be annotated to require this
     * mutex. This will make calling code more verbose, but also help to:
     * - Clarify that the method will acquire a mutex that heavily affects
     *   overall performance.
     * - Force call sites to think how long they need to acquire the mutex to

Review comment (suggestion):
Super nitty, I was just curious about what the possible ways of doing this are, and maybe this is preferable:

Making `GetAllImpl` a static member, so it doesn't need to be passed both a `self` and a `this` pointer:
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 8fb8a876b3f..58bcd9e84e6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5646,14 +5646,14 @@ std::optional<uint256> ChainstateManager::SnapshotBlockhash() const
 }
 
 template <typename T>
-auto ChainstateManager::GetAllImpl(T* self) const
+auto ChainstateManager::GetAllImpl(T* self)
     -> std::vector<std::conditional_t<std::is_const_v<T>, const Chainstate*, Chainstate*>>
 {
     LOCK(::cs_main);
     using ChainstatePtr = std::conditional_t<std::is_const_v<T>, const Chainstate*, Chainstate*>;
     std::vector<ChainstatePtr> out;
 
-    for (ChainstatePtr cs : {m_ibd_chainstate.get(), m_snapshot_chainstate.get()}) {
+    for (ChainstatePtr cs : {self->m_ibd_chainstate.get(), self->m_snapshot_chainstate.get()}) {
         if (self->IsUsable(cs)) out.push_back(cs);
     }
 
diff --git a/src/validation.h b/src/validation.h
index 40420e11223..3ccb25dc29f 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -968,7 +968,7 @@ private:
     //! @see GetAll() const
     //! @see GetAll()
     template <typename T>
-    auto GetAllImpl(T* self) const
+    static auto GetAllImpl(T* self)
         -> std::vector<std::conditional_t<std::is_const_v<T>, const Chainstate*, Chainstate*>>;
 
 public:
```

Code after:
        return cs && !cs->m_disabled;
    }

    //! A queue for script verifications that have to be performed by worker threads.
    CCheckQueue<CScriptCheck> m_script_check_queue;

    //! Timers and counters used for benchmarking validation in both background
    //! and active chainstates.
    SteadyClock::duration GUARDED_BY(::cs_main) time_check{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_forks{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_connect{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_verify{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_undo{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_index{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_total{};
    int64_t GUARDED_BY(::cs_main) num_blocks_total{0};
    SteadyClock::duration GUARDED_BY(::cs_main) time_connect_total{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_flush{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_chainstate{};
    SteadyClock::duration GUARDED_BY(::cs_main) time_post_connect{};

public:
    using Options = kernel::ChainstateManagerOpts;

    explicit ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options);

    //! Function to restart active indexes; set dynamically to avoid a circular
    //! dependency on `base/index.cpp`.
    std::function<void()> snapshot_download_completed = std::function<void()>();

    const CChainParams& GetParams() const { return m_options.chainparams; }
    const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); }
    bool ShouldCheckBlockIndex() const;
    const arith_uint256& MinimumChainWork() const { return *Assert(m_options.minimum_chain_work); }
    const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
    kernel::Notifications& GetNotifications() const { return m_options.notifications; };

    /**
     * Make various assertions about the state of the block index.
     *
     * By default this only executes fully when using the Regtest chain; see: m_options.check_block_index.
     */
    void CheckBlockIndex() const;

    /**
     * Alias for ::cs_main.
     * Should be used in new code to make it easier to make ::cs_main a member
     * of this class.
     * Generally, methods of this class should be annotated to require this
     * mutex. This will make calling code more verbose, but also help to:
     * - Clarify that the method will acquire a mutex that heavily affects