Merge #7330: refactor: separate governance from consensus code

58ab8b3 fix: register NetGovernance unconditionally and tie SuperblockManager lifetime to govman (UdjinM6)
28f339e refactor: move validators to governance/object.h; remove lonely empty header and cpp (Konstantin Akimov)
8d4989f refactor: replace class CProposalValidator with a simple function (Konstantin Akimov)
7b08205 refactor: drop friend CGovernanceObject from CGovernanceManager (Konstantin Akimov)
853c81d refactor: pass reference to CDeterministicMNManager instead of reference to unique_ptr (Konstantin Akimov)
abced0f refactor: simplify after governance refactoring: drop unused forward declarations & includes, move more network code to NetGovernance (Konstantin Akimov)
c08fcae refactor: merge governance/exceptions into governance/object where it is used (Konstantin Akimov)
2366761 refactor: move code from governance/classes to governance/superblock where it belongs (Konstantin Akimov)
8579a1a refactor: cleanup governance/governance includes and forward declarations (Konstantin Akimov)
b7baf0b fmt: clang-apply (Konstantin Akimov)
9237c62 refactor: break circular dependency between governance/object <-> governance/governance (Konstantin Akimov)
95d5dcc refactor: extract SuperblockManager from CGovernanceManager (Konstantin Akimov)
57ba90f refactor: drop useless GovernanceSignerParent (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  The `dash-chainstate` (see #7234) requires as dependency full governance implementation, even if it is not functional or relevant to dash-chainstate (the governance uses off-chain objects and they can't be retrieved without p2p communication).

  It brings extra files to compilation unit of kernel / dash-chainstate:

      governance/classes.cpp \
      governance/common.cpp \
      governance/exceptions.cpp \
      governance/governance.cpp \
      governance/object.cpp \
      governance/validators.cpp \
      governance/vote.cpp \
      governance/votedb.cpp \

  Also, Governance blocks backporting https://github.com/bitcoin/bitcoin/pull/25064/files because it uses timedata.cpp

  Also, Governance requires extra dependency of node/txindex (and recursively index/base which is out-of-scope for kernel)

  ## What was done?
  The only usage of governance in coinsensus code is a validation of superblocks (CMNPaymentsProcessor); superblock validator has been extracted from CGovernanceManager to own file and class `governance::SuperblockManager`.
  It can be simply replaced to stub NullSuperblockmanager, same as `class NullNodeSyncNotifier final : public NodeSyncNotifier` for kernel project.

  There are also several relevant refactorings:
   - governance/exception files are removed; `CGovernanceException` moved to `governance/object.h` where it's used
   - governance/validators files are removed; the class `CProposalValidator` is replaced by a simple helper `governance::ValidateProposal` and moved to `governance/object.h'
   - the dependency CGovermentManager -> GovernanceSigner is reverted; `CGovermentManager` should not know anything about signer if it's not a masternode. Abstract interface `GovernanceSignerParent` is removed.

  ## How Has This Been Tested?
  2 circular dependencies disappeared:
  -    "governance/classes -> governance/object -> governance/governance -> governance/classes",
  -    "governance/governance -> governance/signing -> governance/object -> governance/governance",

  Unit & functional tests succeed.
  Mock proposal is created on testnet with Dash-Qt.

  ## 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
  - [x] 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:
    utACK 58ab8b3

Tree-SHA512: 1090981c656d4ba98e8676bf38a8e22d837b7830837406f58532208021d212662028899348e474436a06262be4548bb48e6433ec10f2a6e105e41434fdeae406

Author: PastaPastaPasta

src/Makefile.am

@@ -234,14 +234,12 @@ BITCOIN_CORE_H = \
   evo/types.h \
   external_signer.h \
   dsnotificationinterface.h \
-  governance/classes.h \
   governance/common.h \
-  governance/exceptions.h \
   governance/governance.h \
   governance/net_governance.h \
   governance/object.h \
   governance/signing.h \
-  governance/validators.h \
+  governance/superblock.h \
   governance/vote.h \
   governance/votedb.h \
   gsl/assert.h \
@@ -525,13 +523,11 @@ libbitcoin_node_a_SOURCES = \
   evo/specialtx_filter.cpp \
   evo/specialtxman.cpp \
   flatfile.cpp \
-  governance/classes.cpp \
-  governance/exceptions.cpp \
   governance/governance.cpp \
   governance/net_governance.cpp \
   governance/object.cpp \
   governance/signing.cpp \
-  governance/validators.cpp \
+  governance/superblock.cpp \
   governance/vote.cpp \
   governance/votedb.cpp \
   gsl/assert.cpp \

src/active/context.cpp

@@ -12,6 +12,7 @@
 #include <evo/deterministicmns.h>
 #include <governance/governance.h>
 #include <governance/signing.h>
+#include <governance/superblock.h>
 #include <instantsend/instantsend.h>
 #include <instantsend/signing.h>
 #include <llmq/debug.h>
@@ -26,7 +27,8 @@
 #include <validationinterface.h>
 
 ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman,
-                             CDeterministicMNManager& dmnman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman,
+                             CDeterministicMNManager& dmnman, CGovernanceManager& govman,
+                             governance::SuperblockManager& superblocks, CMasternodeMetaMan& mn_metaman,
                              CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks, CTxMemPool& mempool,
                              chainlock::ChainlockHandler& clhandler, llmq::CInstantSendManager& isman,
                              llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
@@ -40,7 +42,7 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
     dkgdbgman{std::make_unique<llmq::CDKGDebugManager>(dmnman, qsnapman, chainman)},
     qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)},
     shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, *nodeman, qman, sporkman)},
-    gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)},
+    gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, superblocks, *nodeman, chainman, mn_sync)},
     ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, sigman, *shareman, qman)},
     cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), chainlocks, clhandler, isman,
                                                            qman, sigman, *shareman, mn_sync)},

src/active/context.h

@@ -31,6 +31,9 @@ class Chainlocks;
 class ChainlockHandler;
 class ChainLockSigner;
 } // namespace chainlock
+namespace governance {
+class SuperblockManager;
+} // namespace governance
 namespace instantsend {
 class InstantSendSigner;
 } // namespace instantsend
@@ -55,7 +58,8 @@ struct ActiveContext final : public llmq::QuorumRole, public CValidationInterfac
     ActiveContext(const ActiveContext&) = delete;
     ActiveContext& operator=(const ActiveContext&) = delete;
     explicit ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman,
-                           CDeterministicMNManager& dmnman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman,
+                           CDeterministicMNManager& dmnman, CGovernanceManager& govman,
+                           governance::SuperblockManager& superblocks, CMasternodeMetaMan& mn_metaman,
                            CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks, CTxMemPool& mempool,
                            chainlock::ChainlockHandler& clhandler, llmq::CInstantSendManager& isman,
                            llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,

src/evo/chainhelper.cpp

@@ -4,32 +4,49 @@
 
 #include <evo/chainhelper.h>
 
-#include <chainparams.h>
-
 #include <chainlock/chainlock.h>
+#include <chainparams.h>
 #include <evo/creditpool.h>
 #include <evo/mnhftx.h>
 #include <evo/specialtxman.h>
+#include <governance/superblock.h>
 #include <instantsend/instantsend.h>
 #include <instantsend/lock.h>
+#include <logging.h>
 #include <masternode/payments.h>
+#include <masternode/sync.h>
 
-CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, CGovernanceManager& govman,
+CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, const CMasternodeSync& mn_sync,
                                      llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
                                      llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
                                      const Consensus::Params& consensus_params, const chainlock::Chainlocks& chainlocks,
                                      const llmq::CQuorumManager& qman) :
     isman{isman},
+    mn_sync{mn_sync},
     credit_pool_manager{std::make_unique<CCreditPoolManager>(evodb, chainman)},
     m_chainlocks{chainlocks},
     ehf_manager{std::make_unique<CMNHFManager>(evodb, chainman, qman)},
-    mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, govman, chainman, consensus_params)},
+    superblocks{std::make_unique<governance::SuperblockManager>()},
+    mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, *superblocks, chainman, consensus_params)},
     special_tx{std::make_unique<CSpecialTxProcessor>(*credit_pool_manager, dmnman, *ehf_manager, qblockman, qsnapman,
                                                      chainman, consensus_params, chainlocks, qman)}
 {}
 
 CChainstateHelper::~CChainstateHelper() = default;
 
+bool CChainstateHelper::IsSuperblockValidationRequired(const CBlockIndex* const pindex)
+{
+    if (m_chainlocks.GetBestChainLockHeight() >= pindex->nHeight) {
+        LogPrint(BCLog::MNPAYMENTS, "%s -- validation of chainlocked block=%s is skipped\n", __func__, pindex->GetBlockHash().ToString());
+        return false;
+    }
+    if (!mn_sync.IsSynced()) {
+        LogPrint(BCLog::MNPAYMENTS, "%s -- WARNING! Node is not fully synced, checked superblock for block=%s max bounds only\n", __func__, pindex->GetBlockHash().ToString());
+        return false;
+    }
+    return true;
+}
+
 /** Passthrough functions to chainlock::Chainlocks */
 bool CChainstateHelper::HasConflictingChainLock(int nHeight, const uint256& blockHash) const
 {

src/evo/chainhelper.h

@@ -13,8 +13,8 @@ class CBlockIndex;
 class CCreditPoolManager;
 class CDeterministicMNManager;
 class CEvoDB;
-class CGovernanceManager;
 class ChainstateManager;
+class CMasternodeSync;
 class CMNHFManager;
 class CMNPaymentsProcessor;
 class CSpecialTxProcessor;
@@ -24,6 +24,9 @@ struct CCreditPool;
 namespace chainlock {
 class Chainlocks;
 } // namespace chainlock
+namespace governance {
+class SuperblockManager;
+} // namespace governance
 namespace Consensus {
 struct Params;
 } // namespace Consensus
@@ -37,25 +40,29 @@ class CChainstateHelper
 {
 private:
     llmq::CInstantSendManager& isman;
+    const CMasternodeSync& mn_sync;
 
 public:
     const std::unique_ptr<CCreditPoolManager> credit_pool_manager;
     const chainlock::Chainlocks& m_chainlocks;
     const std::unique_ptr<CMNHFManager> ehf_manager;
+    const std::unique_ptr<governance::SuperblockManager> superblocks;
     const std::unique_ptr<CMNPaymentsProcessor> mn_payments;
     const std::unique_ptr<CSpecialTxProcessor> special_tx;
 
 public:
     CChainstateHelper() = delete;
     CChainstateHelper(const CChainstateHelper&) = delete;
     CChainstateHelper& operator=(const CChainstateHelper&) = delete;
-    explicit CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, CGovernanceManager& govman,
+    explicit CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, const CMasternodeSync& mn_sync,
                                llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
                                llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
                                const Consensus::Params& consensus_params, const chainlock::Chainlocks& chainlocks,
                                const llmq::CQuorumManager& qman);
     ~CChainstateHelper();
 
+    bool IsSuperblockValidationRequired(const CBlockIndex* const pindex);
+
     /** Passthrough functions to chainlock::Chainlocks */
     bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const;
     bool HasChainLock(int nHeight, const uint256& blockHash) const;

src/governance/classes.h

@@ -2,11 +2,12 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <core_io.h>
 #include <governance/common.h>
 #include <governance/governance.h>
-
-#include <core_io.h>
+#include <governance/object.h>
 #include <rpc/util.h>
+#include <tinyformat.h>
 #include <util/check.h>
 
 #include <univalue.h>