Merge dashpay#6640: perf: reuse result of BuildNewListFromBlock

2f74b8d perf: reuse result of BuildNewListFromBlock (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Method `BuildNewListFromBlock` is called twice for each block during validation. These calculation are pretty heavy assuming that there's thousands of masternodes. It is not much for 1 block but that's a lot of CPU calculations during full re-index or initial indexation for 2M of blocks.

  ## What was done?
  Methods `CalcCbTxMerkleRootMNList` and  `CDeterministicMNManager::ProcessBlock` during block calculation re-use same pre-calculated SML instead internal re-calculation.

  ## How Has This Been Tested?
  Benchmarks are done on branch including this changes: dashpay#6632
  ```
  $ getblockcount
  2256425
  $ getblockhash 2250000
  00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0
  $ debug bench
  $ invalidateblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0
  null
  $ reconsiderblock 00000000000000062539a6cd3adece4a10598e729ef571bfded9d4b704af69f0
  ```

  I run it several times in different days and results are differ, but it is consistently faster.
  There's logs for 3 different runs, see sum of `m_dmnman` and `CheckCbTxMerkleRoots`.

  Run time for ~5000blocks:
  ```
  <PR+patches>
  2025-04-16T19:31:08Z [bench]         - m_dmnman: 1.16ms [5.80s]     <---
  2025-04-16T19:31:08Z [bench]           - GetTxPayload: 0.22ms [1.38s]
  2025-04-16T19:31:08Z [bench]           - CalcCbTxMerkleRootMNList: 0.22ms [1.77s]
  2025-04-16T19:31:08Z [bench]             - CachedGetQcHashesQcIndexedHashes: 0.65ms [5.90s]
  2025-04-16T19:31:08Z [bench]             - Loop: 0.01ms [0.72s]
  2025-04-16T19:31:08Z [bench]             - ComputeMerkleRoot: 0.05ms [0.31s]
  2025-04-16T19:31:08Z [bench]           - CalcCbTxMerkleRootQuorums: 0.73ms [7.01s]
  2025-04-16T19:31:08Z [bench]         - CheckCbTxMerkleRoots: 2.39ms [16.84s]     <---
  2025-04-16T19:31:08Z [bench] - Connect block: 10.88ms [105.65s (17.59ms/blk)]

  <develop+patches>
  2025-04-16T20:02:37Z [bench]         - m_dmnman: 1.12ms [6.38s]     <---
  2025-04-16T20:02:37Z [bench]           - GetTxPayload: 0.20ms [1.40s]
  2025-04-16T20:02:37Z [bench]             - BuildNewListFromBlock: 0.33ms [2.97s]
  2025-04-16T20:02:37Z [bench]             - CSimplifiedMNList: 1.08ms [6.56s]
  2025-04-16T20:02:37Z [bench]           - CalcCbTxMerkleRootMNList: 1.79ms [11.86s]
  2025-04-16T20:02:37Z [bench]             - CachedGetQcHashesQcIndexedHashes: 0.71ms [6.02s]
  2025-04-16T20:02:37Z [bench]             - Loop: 0.02ms [0.74s]
  2025-04-16T20:02:37Z [bench]             - ComputeMerkleRoot: 0.05ms [0.30s]
  2025-04-16T20:02:37Z [bench]           - CalcCbTxMerkleRootQuorums: 0.80ms [7.14s]
  2025-04-16T20:02:37Z [bench]         - CheckCbTxMerkleRoots: 2.80ms [20.46s]     <---
  2025-04-16T20:02:37Z [bench] - Connect block: 7.35ms [111.00s (18.45ms/blk)]
  ```

  Run time for 6000 blocks:
  ```
  <PR+patches>
          - m_dmnman: 2.54ms [6.81s]        <---
            - GetTxPayload: 0.21ms [1.52s]
            - CalcCbTxMerkleRootMNList: 0.75ms [2.66s]
              - CachedGetQcHashesQcIndexedHashes: 1.09ms [7.06s]
              - Loop: 0.02ms [0.73s]
              - ComputeMerkleRoot: 0.05ms [0.30s]
            - CalcCbTxMerkleRootQuorums: 1.18ms [8.10s]
          - CheckCbTxMerkleRoots: 4.54ms [21.03s]     <---
  ...
  - Connect block: 14.29ms [113.38s (17.61ms/blk)]

  <develop+patches>
          - m_dmnman: 2.17ms [6.64s]        <---
            - GetTxPayload: 0.24ms [1.45s]
              - BuildNewListFromBlock: 0.61ms [3.05s]
              - CSimplifiedMNList: 2.78ms [7.95s]
            - CalcCbTxMerkleRootMNList: 3.87ms [13.83s]
              - CachedGetQcHashesQcIndexedHashes: 0.69ms [6.80s]
              - Loop: 0.02ms [0.71s]
              - ComputeMerkleRoot: 0.05ms [0.29s]
            - CalcCbTxMerkleRootQuorums: 0.76ms [7.81s]
          - CheckCbTxMerkleRoots: 4.89ms [23.09s]     <---
  ...
  - Connect block: 16.90ms [114.69s (17.80ms/blk)]
  ```

  Run time for 9000 blocks:
  ```
  <PR+patches>
          - m_dmnman: 1.96ms [10.34s]     <---
            - GetTxPayload: 0.23ms [1.98s]
            - CalcCbTxMerkleRootMNList: 0.41ms [3.75s]
              - CachedGetQcHashesQcIndexedHashes: 0.81ms [10.01s]
              - Loop: 0.02ms [0.96s]
              - ComputeMerkleRoot: 0.04ms [0.40s]
            - CalcCbTxMerkleRootQuorums: 0.89ms [11.39s]
          - CheckCbTxMerkleRoots: 3.48ms [30.06s]     <---
  ...
  - Connect block: 15.21ms [152.76s (18.04ms/blk)]

  <develop+patches>
          - m_dmnman: 2.04ms [11.14s]     <---
            - GetTxPayload: 0.20ms [2.00s]
              - BuildNewListFromBlock: 1.09ms [4.23s]
              - CSimplifiedMNList: 3.08ms [13.82s]
            - CalcCbTxMerkleRootMNList: 4.88ms [24.59s]
              - CachedGetQcHashesQcIndexedHashes: 0.87ms [10.22s]
              - Loop: 0.01ms [0.93s]
              - ComputeMerkleRoot: 0.05ms [0.39s]
            - CalcCbTxMerkleRootQuorums: 0.94ms [11.56s]
          - CheckCbTxMerkleRoots: 6.03ms [38.16s]     <---
  ...
  - Connect block: 24.45ms [157.15s (18.55ms/blk)]
  ```

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    light ACK 2f74b8d
  PastaPastaPasta:
    utACK 2f74b8d

Tree-SHA512: 61a69501ff096bfdc903abd0ca79a6aeb98b6e71ddde5773639914586232c869d1588471c7232ee10753f2a8bd044668928ce0272dfc569646412525e12cae48

Author: PastaPastaPasta

src/evo/cbtx.cpp

@@ -4,7 +4,6 @@
 
 #include <consensus/validation.h>
 #include <evo/cbtx.h>
-#include <evo/deterministicmns.h>
 #include <evo/simplifiedmns.h>
 #include <evo/specialtx.h>
 #include <llmq/blockprocessor.h>
@@ -61,9 +60,9 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati
 }
 
 // This can only be done after the block has been fully processed, as otherwise we won't have the finished MN list
-bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeterministicMNManager& dmnman,
-                          llmq::CQuorumSnapshotManager& qsnapman, const llmq::CQuorumBlockProcessor& quorum_block_processor,
-                          BlockValidationState& state, const CCoinsViewCache& view)
+bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex,
+                          const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml,
+                          BlockValidationState& state)
 {
     if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
         return true;
@@ -87,7 +86,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeter
         static int64_t nTimeMerkleQuorum = 0;
 
         uint256 calculatedMerkleRoot;
-        if (!CalcCbTxMerkleRootMNList(block, pindex->pprev, calculatedMerkleRoot, state, dmnman, qsnapman, view)) {
+        if (!CalcCbTxMerkleRootMNList(calculatedMerkleRoot, std::move(sml), state)) {
             // pass the state returned by the function above
             return false;
         }
@@ -116,31 +115,13 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeter
     return true;
 }
 
-bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet,
-                              BlockValidationState& state, CDeterministicMNManager& dmnman,
-                              llmq::CQuorumSnapshotManager& qsnapman, const CCoinsViewCache& view)
+bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state)
 {
     try {
-        static std::atomic<int64_t> nTimeDMN = 0;
-        static std::atomic<int64_t> nTimeSMNL = 0;
         static std::atomic<int64_t> nTimeMerkle = 0;
 
         int64_t nTime1 = GetTimeMicros();
 
-        CDeterministicMNList tmpMNList;
-        if (!dmnman.BuildNewListFromBlock(block, pindexPrev, state, view, tmpMNList, qsnapman, false)) {
-            // pass the state returned by the function above
-            return false;
-        }
-
-        int64_t nTime2 = GetTimeMicros(); nTimeDMN += nTime2 - nTime1;
-        LogPrint(BCLog::BENCHMARK, "            - BuildNewListFromBlock: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), nTimeDMN * 0.000001);
-
-        CSimplifiedMNList sml(tmpMNList);
-
-        int64_t nTime3 = GetTimeMicros(); nTimeSMNL += nTime3 - nTime2;
-        LogPrint(BCLog::BENCHMARK, "            - CSimplifiedMNList: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeSMNL * 0.000001);
-
         static Mutex cached_mutex;
         static CSimplifiedMNList smlCached GUARDED_BY(cached_mutex);
         static uint256 merkleRootCached GUARDED_BY(cached_mutex);
@@ -158,8 +139,10 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev
         bool mutated = false;
         merkleRootRet = sml.CalcMerkleRoot(&mutated);
 
-        int64_t nTime4 = GetTimeMicros(); nTimeMerkle += nTime4 - nTime3;
-        LogPrint(BCLog::BENCHMARK, "            - CalcMerkleRoot: %.2fms [%.2fs]\n", 0.001 * (nTime4 - nTime3), nTimeMerkle * 0.000001);
+        int64_t nTime2 = GetTimeMicros();
+        nTimeMerkle += nTime2 - nTime1;
+        LogPrint(BCLog::BENCHMARK, "            - CalcMerkleRoot: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1),
+                 nTimeMerkle * 0.000001);
 
         smlCached = std::move(sml);
         merkleRootCached = merkleRootRet;

src/evo/cbtx.h

@@ -14,14 +14,12 @@
 class BlockValidationState;
 class CBlock;
 class CBlockIndex;
-class CCoinsViewCache;
-class CDeterministicMNManager;
 class TxValidationState;
+class CSimplifiedMNList;
 
 namespace llmq {
 class CChainLocksHandler;
 class CQuorumBlockProcessor;
-class CQuorumSnapshotManager;
 }// namespace llmq
 
 // Forward declaration from core_io to get rid of circular dependency
@@ -87,12 +85,10 @@ template<> struct is_serializable_enum<CCbTx::Version> : std::true_type {};
 
 bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state);
 
-bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeterministicMNManager& dmnman,
-                          llmq::CQuorumSnapshotManager& qsnapman, const llmq::CQuorumBlockProcessor& quorum_block_processor,
-                          BlockValidationState& state, const CCoinsViewCache& view);
-bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet,
-                              BlockValidationState& state, CDeterministicMNManager& dmnman,
-                              llmq::CQuorumSnapshotManager& qsnapman, const CCoinsViewCache& view);
+bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex,
+                          const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml,
+                          BlockValidationState& state);
+bool CalcCbTxMerkleRootMNList(uint256& merkleRootRet, CSimplifiedMNList&& sml, BlockValidationState& state);
 bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev,
                                const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet,
                                BlockValidationState& state);

src/evo/deterministicmns.cpp

@@ -551,7 +551,7 @@ void CDeterministicMNList::RemoveMN(const uint256& proTxHash)
 
 bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex,
                                            BlockValidationState& state, const CCoinsViewCache& view,
-                                           llmq::CQuorumSnapshotManager& qsnapman, bool fJustCheck,
+                                           llmq::CQuorumSnapshotManager& qsnapman, const CDeterministicMNList& newList,
                                            std::optional<MNListUpdates>& updatesRet)
 {
     AssertLockHeld(cs_main);
@@ -561,23 +561,12 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
         return true;
     }
 
-    CDeterministicMNList oldList, newList;
+    CDeterministicMNList oldList;
     CDeterministicMNListDiff diff;
 
     int nHeight = pindex->nHeight;
 
     try {
-        if (!BuildNewListFromBlock(block, pindex->pprev, state, view, newList, qsnapman, true)) {
-            // pass the state returned by the function above
-            return false;
-        }
-
-        if (fJustCheck) {
-            return true;
-        }
-
-        newList.SetBlockHash(pindex->GetBlockHash());
-
         LOCK(cs);
 
         oldList = GetListForBlockInternal(pindex->pprev);

src/evo/deterministicmns.h

@@ -592,8 +592,9 @@ class CDeterministicMNManager
     ~CDeterministicMNManager() = default;
 
     bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,
-                      const CCoinsViewCache& view, llmq::CQuorumSnapshotManager& qsnapman, bool fJustCheck,
-                      std::optional<MNListUpdates>& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs, cs_main);
+                      const CCoinsViewCache& view, llmq::CQuorumSnapshotManager& qsnapman,
+                      const CDeterministicMNList& newList, std::optional<MNListUpdates>& updatesRet)
+        EXCLUSIVE_LOCKS_REQUIRED(!cs, cs_main);
     bool UndoBlock(gsl::not_null<const CBlockIndex*> pindex, std::optional<MNListUpdates>& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     void UpdatedBlockTip(gsl::not_null<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs);

src/evo/specialtxman.cpp

@@ -7,12 +7,13 @@
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
+#include <evo/assetlocktx.h>
 #include <evo/cbtx.h>
 #include <evo/creditpool.h>
 #include <evo/deterministicmns.h>
 #include <evo/mnhftx.h>
 #include <evo/providertx.h>
-#include <evo/assetlocktx.h>
+#include <evo/simplifiedmns.h>
 #include <hash.h>
 #include <llmq/blockprocessor.h>
 #include <llmq/commitment.h>
@@ -173,16 +174,26 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB
         nTimeQuorum += nTime3 - nTime2;
         LogPrint(BCLog::BENCHMARK, "        - m_qblockman: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeQuorum * 0.000001);
 
-        if (!m_dmnman.ProcessBlock(block, pindex, state, view, m_qsnapman, fJustCheck, updatesRet)) {
-            // pass the state returned by the function above
-            return false;
+
+        CDeterministicMNList mn_list;
+        if (DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_DIP0003)) {
+            if (!m_dmnman.BuildNewListFromBlock(block, pindex->pprev, state, view, mn_list, m_qsnapman, true)) {
+                // pass the state returned by the function above
+                return false;
+            }
+            mn_list.SetBlockHash(pindex->GetBlockHash());
+
+            if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, view, m_qsnapman, mn_list, updatesRet)) {
+                // pass the state returned by the function above
+                return false;
+            }
         }
 
         int64_t nTime4 = GetTimeMicros();
         nTimeDMN += nTime4 - nTime3;
         LogPrint(BCLog::BENCHMARK, "        - m_dmnman: %.2fms [%.2fs]\n", 0.001 * (nTime4 - nTime3), nTimeDMN * 0.000001);
 
-        if (fCheckCbTxMerkleRoots && !CheckCbTxMerkleRoots(block, pindex, m_dmnman, m_qsnapman, m_qblockman, state, view)) {
+        if (fCheckCbTxMerkleRoots && !CheckCbTxMerkleRoots(block, pindex, m_qblockman, CSimplifiedMNList(mn_list), state)) {
             // pass the state returned by the function above
             return false;
         }

src/node/miner.cpp

@@ -28,6 +28,7 @@
 #include <evo/cbtx.h>
 #include <evo/chainhelper.h>
 #include <evo/creditpool.h>
+#include <evo/deterministicmns.h>
 #include <evo/mnhftx.h>
 #include <evo/simplifiedmns.h>
 #include <governance/governance.h>
@@ -224,7 +225,11 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
         cbTx.nHeight = nHeight;
 
         BlockValidationState state;
-        if (!CalcCbTxMerkleRootMNList(*pblock, pindexPrev, cbTx.merkleRootMNList, state, m_dmnman, m_qsnapman, m_chainstate.CoinsTip())) {
+        CDeterministicMNList mn_list;
+        if (!m_dmnman.BuildNewListFromBlock(*pblock, pindexPrev, state, m_chainstate.CoinsTip(), mn_list, m_qsnapman, true)) {
+            throw std::runtime_error(strprintf("%s: BuildNewListFromBlock failed: %s", __func__, state.ToString()));
+        }
+        if (!CalcCbTxMerkleRootMNList(cbTx.merkleRootMNList, CSimplifiedMNList(mn_list), state)) {
             throw std::runtime_error(strprintf("%s: CalcCbTxMerkleRootMNList failed: %s", __func__, state.ToString()));
         }
         if (fDIP0008Active_context) {

src/test/util/setup_common.cpp

@@ -51,6 +51,7 @@
 #include <evo/deterministicmns.h>
 #include <evo/evodb.h>
 #include <evo/mnhftx.h>
+#include <evo/simplifiedmns.h>
 #include <evo/specialtx.h>
 #include <flat-database.h>
 #include <governance/governance.h>
@@ -486,7 +487,11 @@ CBlock TestChainSetup::CreateBlock(
         auto cbTx = GetTxPayload<CCbTx>(*block.vtx[0]);
         Assert(cbTx.has_value());
         BlockValidationState state;
-        if (!CalcCbTxMerkleRootMNList(block, chainstate.m_chain.Tip(), cbTx->merkleRootMNList, state, *m_node.dmnman, *m_node.llmq_ctx->qsnapman, chainstate.CoinsTip())) {
+        CDeterministicMNList mn_list;
+        if (!m_node.dmnman->BuildNewListFromBlock(block, chainstate.m_chain.Tip(), state, chainstate.CoinsTip(), mn_list, *m_node.llmq_ctx->qsnapman, true)) {
+            Assert(false);
+        }
+        if (!CalcCbTxMerkleRootMNList(cbTx->merkleRootMNList, CSimplifiedMNList(mn_list), state)) {
             Assert(false);
         }
         if (!CalcCbTxMerkleRootQuorums(block, chainstate.m_chain.Tip(), *m_node.llmq_ctx->quorum_block_processor, cbTx->merkleRootQuorums, state)) {