Merge #6719: perf: parse CbTx only once perf block

0c7cfd9 refactor: make CheckCreditPoolDiffForBlock private and tidy up it (Konstantin Akimov)
a7fe377 refactor: bench time is properly aligned, variables properly renamed (Konstantin Akimov)
ebb9dc0 perf: parse CCbTx once during block validation for Credit Poool (Konstantin Akimov)
59c2407 refactor: remove ProcessSpecialTx and UndoSpecialTx which do nothing (Konstantin Akimov)
60bf96d perf: exclude parsing CCbTx if flag fCheckCbTxMerkleRoots is not set (Konstantin Akimov)
b911929 perf: parse CCbTx once during block validation (check CbTx) (Konstantin Akimov)
d4f4173 perf: parse CCbTx once during block validation (merkle roots and CL) (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Coinbase Transaction (CCbTx) is parsed several times:
   - itself validation (during CheckCbTx)
   - for merkle roots (during CheckCbTxMerkleRoots)
   - for CL validation (during CheckCbTxBestChainlock)
   - for Credit Pool (during CheckCreditPoolDiffForBlock)

  For pre-v20 blocks this parsing has been relatively cheap, but once V20 has been activated, each CbTx has CL signature and its deserialization become expensive due to heavy call of `CBLSWrapper<bls::G2Element, 96ul, CBLSSignature>::Unserialize<CDataStream>(CDataStream&, bool)`.

  ## What was done?
  CCbTx is parsed only once and reused for all related checks.

  This PR makes validation of blocks after v20 activation for ~6% faster.
  Validation of pre-v20 block is expected to be improved insignificantly (<1% of total time).

  ## How Has This Been Tested?
  Invalidated + reconsidered ~8.5k blocks (~2 weeks) after v20 activation.

  Develop:
  <img width="704" alt="image" src="https://github.com/user-attachments/assets/08b63507-4cbf-4e2c-917e-1287ff17210d" />
  ```
  [bench]         - Loop: 0.22ms [10.01s]
  [bench]           - GetTxPayload: 0.21ms [2.16s]
  [bench]         - CheckCbTxMerkleRoots: 3.23ms [28.46s]
  [bench]         - CheckCbTxBestChainlock: 3.16ms [24.65s]
  [bench]       - ProcessSpecialTxsInBlock: 8.15ms [101.19s (11.66ms/blk)]
  [bench]       - CheckCreditPoolDiffForBlock: 0.22ms [2.98s (0.34ms/blk)]
  [bench]     - Dash specific: 0.42ms [4.98s (0.57ms/blk)]
  [bench]   - Connect total: 8.87ms [113.20s (13.04ms/blk)]
  [bench] - Connect block: 9.26ms [116.51s (13.42ms/blk)]
  ```

  PR:
  <img width="704" alt="image" src="https://github.com/user-attachments/assets/697d9b73-83f4-419e-9ecc-fa9c24e1eafc" />
  ```
  [bench]       - GetTxPayload: 0.24ms [2.34s]
  [bench]       - Loop: 0.02ms [7.89s]
  [bench]       - CheckCreditPoolDiffForBlock: 0.01ms [0.76s]
  [bench]       - CheckCbTxMerkleRoots: 3.17ms [26.50s]
  [bench]       - CheckCbTxBestChainlock: 3.08ms [22.77s]
  [bench]       - ProcessSpecialTxsInBlock: 8.02ms [98.70s (11.37ms/blk)]
  [bench]     - Dash specific: 0.27ms [1.97s (0.23ms/blk)]
  [bench]   - Connect total: 8.58ms [107.65s (12.40ms/blk)]
  [bench] - Connect block: 8.92ms [110.97s (12.78ms/blk)]
  ```

  Some insignificant lines (not relevant to PR) are removed from both screenshots and bench logs, only affected functions are shown.

  ## 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

ACKs for top commit:
  UdjinM6:
    utACK 0c7cfd9
  PastaPastaPasta:
    utACK 0c7cfd9

Tree-SHA512: b684f8b3073870ac40cb4de19066e9424307e2c39c753e0bc3c584d491c8dbc1634a35b63fe906acfe03bf9e1b78f0642cee4c0c3715533570ea6693fa4767ea

Author: PastaPastaPasta

src/evo/cbtx.cpp

@@ -19,22 +19,8 @@
 #include <deploymentstatus.h>
 
 
-bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state)
+bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state)
 {
-    if (tx.nType != TRANSACTION_COINBASE) {
-        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-type");
-    }
-
-    if (!tx.IsCoinBase()) {
-        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-invalid");
-    }
-
-    const auto opt_cbTx = GetTxPayload<CCbTx>(tx);
-    if (!opt_cbTx) {
-        return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload");
-    }
-    const auto& cbTx = *opt_cbTx;
-
     if (cbTx.nVersion == CCbTx::Version::INVALID || cbTx.nVersion >= CCbTx::Version::UNKNOWN) {
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version");
     }
@@ -59,31 +45,15 @@ 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,
+bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, const CBlockIndex* pindex,
                           const llmq::CQuorumBlockProcessor& quorum_block_processor, CSimplifiedMNList&& sml,
                           BlockValidationState& state)
 {
-    if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
-        return true;
-    }
-
-    static int64_t nTimePayload = 0;
-
-    int64_t nTime1 = GetTimeMicros();
-
-    const auto opt_cbTx = GetTxPayload<CCbTx>(*block.vtx[0]);
-    if (!opt_cbTx) {
-        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload");
-    }
-    auto cbTx = *opt_cbTx;
-
-    int64_t nTime2 = GetTimeMicros(); nTimePayload += nTime2 - nTime1;
-    LogPrint(BCLog::BENCHMARK, "          - GetTxPayload: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1), nTimePayload * 0.000001);
-
     if (pindex) {
         static int64_t nTimeMerkleMNL = 0;
         static int64_t nTimeMerkleQuorum = 0;
 
+        int64_t nTime1 = GetTimeMicros();
         uint256 calculatedMerkleRoot;
         if (!CalcCbTxMerkleRootMNList(calculatedMerkleRoot, std::move(sml), state)) {
             // pass the state returned by the function above
@@ -93,8 +63,10 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex,
             return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-mnmerkleroot");
         }
 
-        int64_t nTime3 = GetTimeMicros(); nTimeMerkleMNL += nTime3 - nTime2;
-        LogPrint(BCLog::BENCHMARK, "          - CalcCbTxMerkleRootMNList: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2), nTimeMerkleMNL * 0.000001);
+        int64_t nTime2 = GetTimeMicros();
+        nTimeMerkleMNL += nTime2 - nTime1;
+        LogPrint(BCLog::BENCHMARK, "          - CalcCbTxMerkleRootMNList: %.2fms [%.2fs]\n", 0.001 * (nTime2 - nTime1),
+                 nTimeMerkleMNL * 0.000001);
 
         if (cbTx.nVersion >= CCbTx::Version::MERKLE_ROOT_QUORUMS) {
             if (!CalcCbTxMerkleRootQuorums(block, pindex->pprev, quorum_block_processor, calculatedMerkleRoot, state)) {
@@ -106,9 +78,10 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex,
             }
         }
 
-        int64_t nTime4 = GetTimeMicros(); nTimeMerkleQuorum += nTime4 - nTime3;
-        LogPrint(BCLog::BENCHMARK, "          - CalcCbTxMerkleRootQuorums: %.2fms [%.2fs]\n", 0.001 * (nTime4 - nTime3), nTimeMerkleQuorum * 0.000001);
-
+        int64_t nTime3 = GetTimeMicros();
+        nTimeMerkleQuorum += nTime3 - nTime2;
+        LogPrint(BCLog::BENCHMARK, "          - CalcCbTxMerkleRootQuorums: %.2fms [%.2fs]\n", 0.001 * (nTime3 - nTime2),
+                 nTimeMerkleQuorum * 0.000001);
     }
 
     return true;
@@ -314,19 +287,9 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
     return true;
 }
 
-bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex,
+bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex,
                             const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state)
 {
-    if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
-        return true;
-    }
-
-    const auto opt_cbTx = GetTxPayload<CCbTx>(*block.vtx[0]);
-    if (!opt_cbTx) {
-        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload");
-    }
-    const auto& cbTx = *opt_cbTx;
-
     if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
         return true;
     }

src/evo/cbtx.h

@@ -64,17 +64,17 @@ class CCbTx
 };
 template<> struct is_serializable_enum<CCbTx::Version> : std::true_type {};
 
-bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state);
+bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationState& state);
 
-bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex,
+bool CheckCbTxMerkleRoots(const CBlock& block, const CCbTx& cbTx, 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);
 
-bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev,
+bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindexPrev,
                             const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state);
 bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
                            uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature);