perf: optimize dmnlist construction and payye validation

src/evo/deterministicmns.cpp

@@ -22,6 +22,7 @@
 #include <uint256.h>
 #include <util/helpers.h>
 
+#include <immer/algorithm.hpp>
 #include <univalue.h>
 
 #include <functional>
@@ -337,27 +338,18 @@ CDeterministicMNListDiff CDeterministicMNList::BuildDiff(const CDeterministicMNL
 {
     CDeterministicMNListDiff diffRet;
 
-    for (const auto& p : to.mnMap) {
-        const auto& toPtr = p.second;
-        auto fromPtr = GetMN(toPtr->proTxHash);
-        if (fromPtr == nullptr) {
-            diffRet.addedMNs.emplace_back(toPtr);
-        } else if (fromPtr != toPtr || fromPtr->pdmnState != toPtr->pdmnState) {
-            CDeterministicMNStateDiff stateDiff(*fromPtr->pdmnState, *toPtr->pdmnState);
-            if (stateDiff.fields) {
-                diffRet.updatedMNs.emplace(toPtr->GetInternalId(), std::move(stateDiff));
-            }
-        }
-    }
-    if (mnMap.size() + diffRet.addedMNs.size() != to.mnMap.size()) {
-        for (auto& fromPtr : mnMap) {
-            const auto toPtr = to.GetMN(fromPtr.second->proTxHash);
-            if (toPtr == nullptr) {
-                diffRet.removedMns.emplace(fromPtr.second->GetInternalId());
-                if (mnMap.size() + diffRet.addedMNs.size() - diffRet.removedMns.size() == to.mnMap.size()) break;
-            }
-        };
-    }
+    // Visits only the subtrees the two maps do not share, so a list derived from this one by a
+    // few updates is diffed in time proportional to those updates.
+    immer::diff(mnMap, to.mnMap,
+                immer::make_differ(
+                    [&diffRet](const auto& added) { diffRet.addedMNs.emplace_back(added.second); },
+                    [&diffRet](const auto& removed) { diffRet.removedMns.emplace(removed.second->GetInternalId()); },
+                    [&diffRet](const auto& from, const auto& to_entry) {
+                        CDeterministicMNStateDiff stateDiff(*from.second->pdmnState, *to_entry.second->pdmnState);
+                        if (stateDiff.fields) {
+                            diffRet.updatedMNs.emplace(to_entry.second->GetInternalId(), std::move(stateDiff));
+                        }
+                    }));
 
     // added MNs need to be sorted by internalId so that these are added in correct order when the diff is applied later
     // otherwise internalIds will not match with the original list
@@ -687,8 +679,8 @@ CDeterministicMNManager::CDeterministicMNManager(CEvoDB& evoDb, CMasternodeMetaM
 CDeterministicMNManager::~CDeterministicMNManager() = default;
 
 bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex,
-                                           BlockValidationState& state, const CDeterministicMNList& newList,
-                                           MNListUpdates& updatesRet)
+                                           BlockValidationState& state, const CDeterministicMNList& oldList,
+                                           const CDeterministicMNList& newList, MNListUpdates& updatesRet)
 {
     AssertLockHeld(::cs_main);
 
@@ -697,7 +689,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
         return true;
     }
 
-    CDeterministicMNList oldList;
     CDeterministicMNListDiff diff;
 
     int nHeight = pindex->nHeight;
@@ -707,7 +698,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
 
         LOCK(cs);
 
-        oldList = GetListForBlockInternal(pindex->pprev);
         diff = oldList.BuildDiff(newList);
 
         // A WriteDerived mismatch is local EvoDB corruption, not a statement
@@ -756,7 +746,9 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_null<co
         return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-dmn-block");
     }
 
-    updatesRet = {.old_list = oldList, .new_list = newList, .diff = diff};
+    updatesRet.old_list = oldList;
+    updatesRet.new_list = newList;
+    updatesRet.diff = diff;
 
     if (::g_stats_client->active()) {
         const auto counts{newList.GetCounts()};
@@ -805,7 +797,9 @@ bool CDeterministicMNManager::UndoBlock(gsl::not_null<const CBlockIndex*> pindex
         curList.ApplyDiff(pindex, diff);
 
         auto inversedDiff{curList.BuildDiff(prevList)};
-        updatesRet = {.old_list = curList, .new_list = prevList, .diff = inversedDiff};
+        updatesRet.old_list = curList;
+        updatesRet.new_list = prevList;
+        updatesRet.diff = inversedDiff;
     }
 
     const auto& consensusParams = Params().GetConsensus();

src/evo/deterministicmns.h

@@ -263,6 +263,8 @@ class CDeterministicMNList
         InvalidateSMLCache();
     }
 
+    [[nodiscard]] size_t GetAllMNsCount() const { return mnMap.size(); }
+
     [[nodiscard]] Counts GetCounts() const
     {
         Counts ret;
@@ -295,7 +297,8 @@ class CDeterministicMNList
      * @param onlyValid Run on all masternodes, or only "valid" (not banned) masternodes
      * @param cb callback to execute
      */
-    void ForEachMN(bool onlyValid, std::function<void(const CDeterministicMN&)> cb) const
+    template <typename Callback>
+    void ForEachMN(bool onlyValid, Callback&& cb) const
     {
         for (const auto& p : mnMap) {
             if (!onlyValid || !p.second->pdmnState->IsBanned()) {
@@ -311,7 +314,8 @@ class CDeterministicMNList
      * @param onlyValid Run on all masternodes, or only "valid" (not banned) masternodes
      * @param cb callback to execute
      */
-    void ForEachMNShared(bool onlyValid, std::function<void(const CDeterministicMNCPtr&)> cb) const
+    template <typename Callback>
+    void ForEachMNShared(bool onlyValid, Callback&& cb) const
     {
         for (const auto& p : mnMap) {
             if (!onlyValid || !p.second->pdmnState->IsBanned()) {
@@ -728,6 +732,8 @@ struct MNListUpdates
     CDeterministicMNList old_list;
     CDeterministicMNList new_list;
     CDeterministicMNListDiff diff;
+    //! The masternode the block pays, as selected from old_list
+    CDeterministicMNCPtr payee;
 };
 
 /** Sentinel suffix carried by serving-failure messages that report locally
@@ -783,7 +789,7 @@ class CDeterministicMNManager
     ~CDeterministicMNManager();
 
     bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,
-                      const CDeterministicMNList& newList, MNListUpdates& updatesRet)
+                      const CDeterministicMNList& oldList, const CDeterministicMNList& newList, MNListUpdates& updatesRet)
         EXCLUSIVE_LOCKS_REQUIRED(!cs, ::cs_main);
     bool UndoBlock(gsl::not_null<const CBlockIndex*> pindex, MNListUpdates& updatesRet) EXCLUSIVE_LOCKS_REQUIRED(!cs);
 

src/evo/specialtxman.cpp

@@ -750,12 +750,11 @@ bool CSpecialTxProcessor::ApplyTxToList(const CTransaction& tx, gsl::not_null<co
 }
 
 bool CSpecialTxProcessor::FinishListForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev,
-                                             const CDeterministicMNList& prevList, bool debugLogs,
-                                             CDeterministicMNList newList, BlockValidationState& state,
+                                             const CDeterministicMNList& prevList, const CDeterministicMNCPtr& payee,
+                                             bool debugLogs, CDeterministicMNList newList, BlockValidationState& state,
                                              CDeterministicMNList& mnListRet) const
 {
     const int nHeight = pindexPrev->nHeight + 1;
-    auto payee = prevList.GetMNPayee(pindexPrev);
     const bool isMNRewardReallocation{DeploymentActiveAfter(pindexPrev, m_consensus_params, Consensus::DEPLOYMENT_MN_RR)};
 
     // we skip the coinbase
@@ -827,9 +826,13 @@ bool CSpecialTxProcessor::FinishListForBlock(const CBlock& block, gsl::not_null<
 bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev,
                                                bool is_v24_active, const CDeterministicMNList& prevList,
                                                const CCoinsViewCache& view, bool debugLogs, BlockValidationState& state,
-                                               CDeterministicMNList& mnListRet)
+                                               CDeterministicMNList& mnListRet, CDeterministicMNCPtr* payeeRet)
 {
     CDeterministicMNList newList = BeginListForBlock(pindexPrev, prevList);
+    const CDeterministicMNCPtr payee{prevList.GetMNPayee(pindexPrev)};
+    if (payeeRet != nullptr) {
+        *payeeRet = payee;
+    }
 
     // we skip the coinbase
     for (int i = 1; i < static_cast<int>(block.vtx.size()); i++) {
@@ -838,7 +841,7 @@ bool CSpecialTxProcessor::RebuildListFromBlock(const CBlock& block, gsl::not_nul
         }
     }
 
-    return FinishListForBlock(block, pindexPrev, prevList, debugLogs, newList, state, mnListRet);
+    return FinishListForBlock(block, pindexPrev, prevList, payee, debugLogs, newList, state, mnListRet);
 }
 
 CMNListBuilder::CMNListBuilder(const CSpecialTxProcessor& processor, const CDeterministicMNList& prevList,
@@ -850,6 +853,7 @@ CMNListBuilder::CMNListBuilder(const CSpecialTxProcessor& processor, const CDete
     m_is_v24_active{is_v24_active},
     m_view{&view},
     m_debug_logs{debugLogs},
+    m_payee{prevList.GetMNPayee(pindexPrev)},
     m_list{processor.BeginListForBlock(pindexPrev, prevList)}
 {
 }
@@ -869,7 +873,8 @@ bool CMNListBuilder::ApplyTx(const CTransaction& tx, BlockValidationState& state
 
 bool CMNListBuilder::Finish(const CBlock& block, BlockValidationState& state, CDeterministicMNList& mnListRet) const
 {
-    return m_processor->FinishListForBlock(block, m_pindex_prev, m_prev_list, m_debug_logs, m_list, state, mnListRet);
+    return m_processor->FinishListForBlock(block, m_pindex_prev, m_prev_list, m_payee, m_debug_logs, m_list, state,
+                                           mnListRet);
 }
 
 uint256 CSpecialTxProcessor::GetDeterministicMNListHash(gsl::not_null<const CBlockIndex*> pindex) const
@@ -992,13 +997,14 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const
             // ConnectBlock validates masternode payments against the list at the
             // previous block, also under fJustCheck where ProcessBlock is skipped.
             updatesRet.old_list = m_dmnman.GetListForBlock(pindex->pprev);
-            if (!BuildNewListFromBlock(block, pindex->pprev, rules.v24, view, true, state, mn_list)) {
+            if (!RebuildListFromBlock(block, pindex->pprev, rules.v24, updatesRet.old_list, view, true, state, mn_list,
+                                      &updatesRet.payee)) {
                 // pass the state returned by the function above
                 return false;
             }
             mn_list.SetBlockHash(pindex->GetBlockHash());
 
-            if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, mn_list, updatesRet)) {
+            if (!fJustCheck && !m_dmnman.ProcessBlock(block, pindex, state, updatesRet.old_list, mn_list, updatesRet)) {
                 // pass the state returned by the function above
                 return false;
             }

src/evo/specialtxman.h

@@ -100,9 +100,11 @@ class CSpecialTxProcessor
 
     // Variant that takes an explicit starting list instead of loading from GetListForBlock
     // Used for rebuilding diffs from trusted snapshots
+    // payeeRet, when given, receives the masternode the block pays as selected from prevList
     bool RebuildListFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev, bool is_v24_active,
                               const CDeterministicMNList& prevList, const CCoinsViewCache& view, bool debugLogs,
-                              BlockValidationState& state, CDeterministicMNList& mnListRet);
+                              BlockValidationState& state, CDeterministicMNList& mnListRet,
+                              CDeterministicMNCPtr* payeeRet = nullptr);
 
     /** Return a canonical hash of the deterministic MN list derived at a block. */
     uint256 GetDeterministicMNListHash(gsl::not_null<const CBlockIndex*> pindex) const;
@@ -121,8 +123,9 @@ class CSpecialTxProcessor
                        const CCoinsViewCache& view, bool debugLogs, CDeterministicMNList& newList,
                        BlockValidationState& state) const;
     bool FinishListForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindexPrev,
-                            const CDeterministicMNList& prevList, bool debugLogs, CDeterministicMNList newList,
-                            BlockValidationState& state, CDeterministicMNList& mnListRet) const;
+                            const CDeterministicMNList& prevList, const CDeterministicMNCPtr& payee, bool debugLogs,
+                            CDeterministicMNList newList, BlockValidationState& state,
+                            CDeterministicMNList& mnListRet) const;
 
     bool CheckSpecialTxInner(const CChain* chain, const CTransaction& tx, const CBlockIndex* pindexPrev,
                              SpecialTxRules rules, const CCoinsViewCache& view, const std::optional<CRangesSet>& indexes,
@@ -160,6 +163,7 @@ class CMNListBuilder
     bool m_is_v24_active;
     const CCoinsViewCache* m_view;
     bool m_debug_logs;
+    CDeterministicMNCPtr m_payee;
     CDeterministicMNList m_list;
 };
 

src/masternode/payments.cpp

@@ -208,7 +208,8 @@ bool IsSuperblockValid(const SuperblockStatus& superblock, const CTransaction& t
 }
 
 bool GetMasternodePayments(const CDeterministicMNList& mn_list, const CBlockIndex* pindexPrev,
-                           const CAmount blockSubsidy, const CAmount feeReward, MnRewardEra era,
+                           const CDeterministicMNCPtr& dmnPayee, const CAmount blockSubsidy, const CAmount feeReward,
+                           MnRewardEra era,
                            const Consensus::Params& consensus_params,
                            std::vector<CTxOut>& voutMasternodePaymentsRet)
 {
@@ -230,11 +231,10 @@ bool GetMasternodePayments(const CDeterministicMNList& mn_list, const CBlockInde
         LogPrint(BCLog::MNPAYMENTS, "%s -- MN reward %lld reallocated to credit pool\n", __func__, platformReward);
         voutMasternodePaymentsRet.emplace_back(platformReward, CScript() << OP_RETURN);
     }
-    if (mn_list.GetCounts().total() == 0) {
+    if (mn_list.GetAllMNsCount() == 0) {
         LogPrint(BCLog::MNPAYMENTS, "%s -- no masternode registered to receive a payment\n", __func__);
         return true;
     }
-    const auto dmnPayee = mn_list.GetMNPayee(pindexPrev);
     if (!dmnPayee) {
         return false;
     }

src/masternode/payments.h

@@ -6,6 +6,7 @@
 #define BITCOIN_MASTERNODE_PAYMENTS_H
 
 #include <consensus/amount.h>
+#include <evo/types.h>
 #include <primitives/transaction.h>
 
 #include <vector>
@@ -65,7 +66,7 @@ CAmount GetMasternodePayment(int nHeight, CAmount blockValue, const Consensus::P
  * output in that case.
  */
 bool GetMasternodePayments(const CDeterministicMNList& mn_list, const CBlockIndex* pindexPrev,
-                           CAmount blockSubsidy, CAmount feeReward, MnRewardEra era,
+                           const CDeterministicMNCPtr& dmnPayee, CAmount blockSubsidy, CAmount feeReward, MnRewardEra era,
                            const Consensus::Params& consensus_params,
                            std::vector<CTxOut>& voutMasternodePaymentsRet);
 

src/node/miner.cpp

@@ -190,7 +190,7 @@ void BlockAssembler::FillBlockPayments(CMutableTransaction& txNew, const CBlockI
     }
 
     const MnRewardEra era{GetMnRewardEraAfter(pindexPrev, m_chainstate.m_chainman)};
-    if (!GetMasternodePayments(mn_list, pindexPrev, blockSubsidy, feeReward, era, chainparams.GetConsensus(), voutMasternodePaymentsRet)) {
+    if (!GetMasternodePayments(mn_list, pindexPrev, mn_list.GetMNPayee(pindexPrev), blockSubsidy, feeReward, era, chainparams.GetConsensus(), voutMasternodePaymentsRet)) {
         LogPrintf("%s -- ERROR Failed to get payee\n", __func__);
         LogPrint(BCLog::MNPAYMENTS, "%s -- No masternode to pay (MN list probably empty)\n", __func__);
     } else {
@@ -267,7 +267,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     if (fDIP0003Active_context) {
         // No list-transition logs: a package's trial may still be discarded, and block connection
         // logs the transitions that actually happen.
-        mn_list_builder.emplace(m_chainstate.m_special_tx, pindexPrev, fV24Active_context, m_chainstate.CoinsTip(),
+        mn_list_builder.emplace(m_chainstate.m_special_tx, gsl::not_null<const CBlockIndex*>{pindexPrev}, fV24Active_context, m_chainstate.CoinsTip(),
                                 /*debugLogs=*/false);
         for (const Consensus::LLMQParams& params : llmq::GetEnabledQuorumParams(m_chainstate.m_chainman, pindexPrev)) {
             std::vector<CTransactionRef> vqcTx;

src/rpc/masternode.cpp

@@ -456,7 +456,8 @@ static RPCHelpMan masternode_payments()
         std::vector<CTxOut> voutMasternodePayments;
         CAmount blockSubsidy = GetBlockSubsidy(pindex, Params().GetConsensus());
         const MnRewardEra mn_reward_era{GetMnRewardEraAfter(pindex->pprev, chainman)};
-        if (!GetMasternodePayments(node.dmnman->GetListForBlock(pindex->pprev), pindex->pprev, blockSubsidy, nBlockFees,
+        const CDeterministicMNList mn_list{node.dmnman->GetListForBlock(pindex->pprev)};
+        if (!GetMasternodePayments(mn_list, pindex->pprev, mn_list.GetMNPayee(pindex->pprev), blockSubsidy, nBlockFees,
                                    mn_reward_era, Params().GetConsensus(), voutMasternodePayments)) {
             LogPrint(BCLog::MNPAYMENTS, "%s -- No masternode to pay (MN list probably empty)\n", self.m_name);
         }

src/validation.cpp

@@ -2645,7 +2645,7 @@ bool Chainstate::IsBlockValueValid(const CBlock& block, const CBlockIndex* pinde
     return true;
 }
 
-bool Chainstate::IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pindex, const CDeterministicMNList& mn_list, const CAmount blockSubsidy, const CAmount feeReward) const
+bool Chainstate::IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pindex, const CDeterministicMNList& mn_list, const CDeterministicMNCPtr& payee, const CAmount blockSubsidy, const CAmount feeReward) const
 {
     const Consensus::Params& consensus_params{m_chainman.GetConsensus()};
     const CTransaction& txNew{*block.vtx[0]};
@@ -2657,7 +2657,7 @@ bool Chainstate::IsBlockPayeeValid(const CBlock& block, const CBlockIndex* pinde
 
     std::vector<CTxOut> voutMasternodePayments;
     const MnRewardEra era{GetMnRewardEraAfter(pindex->pprev, m_chainman)};
-    if (!GetMasternodePayments(mn_list, pindex->pprev, blockSubsidy, feeReward, era, consensus_params, voutMasternodePayments)) {
+    if (!GetMasternodePayments(mn_list, pindex->pprev, payee, blockSubsidy, feeReward, era, consensus_params, voutMasternodePayments)) {
         LogPrintf("%s -- ERROR! Failed to get payees for block at height %s\n", __func__, nBlockHeight);
         return true;
     }
@@ -3071,7 +3071,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
              Ticks<SecondsDouble>(time_value_valid),
              Ticks<MillisecondsDouble>(time_value_valid) / num_blocks_total);
 
-    if (!IsBlockPayeeValid(block, pindex, mnlist_updates.old_list, blockSubsidy, feeReward)) {
+    if (!IsBlockPayeeValid(block, pindex, mnlist_updates.old_list, mnlist_updates.payee, blockSubsidy, feeReward)) {
         // NOTE: Do not punish, the node might be missing governance data
         LogPrintf("ERROR: ConnectBlock(DASH): couldn't find masternode or superblock payments\n");
         return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-payee");