Merge #6376: refactor: a bunch of trivial-ish headers improvements

e9387eef69 refactor: remove unused includes and forward declarations from headers (Konstantin Akimov)
f2a186f3f3 refactor: remove definition IsQuorumDKGEnabled from header (Konstantin Akimov)
8a7941bd0d refactor: merge helper GetCoinbaseTx which is used only once into GetNonNullCoinbaseChainlock (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Split from https://github.com/dashpay/dash/pull/6375 and added some extra

  ## What was done?
  Tidy up headers of evo/ and llmq/:
   - removed unused includes and added missing
   - removed unused forward declarations and added missing
   - removed helper GetCoinbaseTx, moved helper IsQuorumDKGEnabled from header to implementation

  ## How Has This Been Tested?
  Run unit and functional tests

  Compilation time _in theory_ should be improved, in really it shaved just 2-5 seconds from 5 minutes by my benchmark.

  ## 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
  - [x] 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 e9387eef69
  PastaPastaPasta:
    utACK e9387eef69

Tree-SHA512: 7b4795fe0bcea23b9f368a962b888d610cac94b42dd6419cfe06977c1a28bbe27a7a2ae2e4cd730ec0ca4f8b333f656a601ebb90bc271f4117dec7a424a08b45

src/evo/assetlocktx.h

@@ -6,9 +6,9 @@
 #define BITCOIN_EVO_ASSETLOCKTX_H
 
 #include <bls/bls_ies.h>
-#include <primitives/transaction.h>
+#include <consensus/amount.h>
 #include <gsl/pointers.h>
-
+#include <primitives/transaction.h>
 #include <serialize.h>
 #include <univalue.h>
 

src/evo/cbtx.cpp

@@ -20,6 +20,7 @@
 #include <deploymentstatus.h>
 #include <validation.h>
 
+
 bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state)
 {
     if (tx.nType != TRANSACTION_COINBASE) {
@@ -448,7 +449,7 @@ std::string CCbTx::ToString() const
         creditPoolBalance / COIN, creditPoolBalance % COIN);
 }
 
-std::optional<CCbTx> GetCoinbaseTx(const CBlockIndex* pindex)
+std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex)
 {
     if (pindex == nullptr) {
         return std::nullopt;
@@ -464,20 +465,14 @@ std::optional<CCbTx> GetCoinbaseTx(const CBlockIndex* pindex)
         return std::nullopt;
     }
 
-    CTransactionRef cbTx = block.vtx[0];
+    const CTransactionRef cbTx = block.vtx[0];
-    return GetTxPayload<CCbTx>(*cbTx);
+    const auto opt_cbtx = GetTxPayload<CCbTx>(*cbTx);
-}
-
-std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex)
-{
-    auto opt_cbtx = GetCoinbaseTx(pindex);
 
     if (!opt_cbtx.has_value()) {
         return std::nullopt;
     }
 
-    CCbTx& cbtx = opt_cbtx.value();
+    const CCbTx& cbtx = opt_cbtx.value();
-
     if (cbtx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
         return std::nullopt;
     }

src/evo/cbtx.h

@@ -100,7 +100,6 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev,
 bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
                            uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature);
 
-std::optional<CCbTx> GetCoinbaseTx(const CBlockIndex* pindex);
 std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex);
 
 #endif // BITCOIN_EVO_CBTX_H

src/evo/creditpool.h

@@ -5,10 +5,7 @@
 #ifndef BITCOIN_EVO_CREDITPOOL_H
 #define BITCOIN_EVO_CREDITPOOL_H
 
-#include <coins.h>
+#include <consensus/amount.h>
-
-#include <evo/assetlocktx.h>
-
 #include <saltedhasher.h>
 #include <serialize.h>
 #include <sync.h>
@@ -16,10 +13,13 @@
 #include <unordered_lru_cache.h>
 #include <util/ranges_set.h>
 
+#include <evo/assetlocktx.h>
+
 #include <optional>
 #include <unordered_set>
 
 class BlockManager;
+class CBlock;
 class CBlockIndex;
 class BlockValidationState;
 class CEvoDB;

src/evo/deterministicmns.h

@@ -29,6 +29,7 @@
 class CBlock;
 class CBlockIndex;
 class CChainState;
+class CCoinsViewCache;
 class CConnman;
 class CEvoDB;
 class TxValidationState;

src/evo/dmnstate.h

@@ -5,12 +5,12 @@
 #ifndef BITCOIN_EVO_DMNSTATE_H
 #define BITCOIN_EVO_DMNSTATE_H
 
-#include <crypto/common.h>
 #include <bls/bls.h>
-#include <pubkey.h>
+#include <crypto/sha256.h>
-#include <netaddress.h>
-#include <script/script.h>
 #include <evo/providertx.h>
+#include <netaddress.h>
+#include <pubkey.h>
+#include <script/script.h>
 
 #include <memory>
 #include <utility>

src/evo/mnauth.h

@@ -14,16 +14,12 @@ class CBlockIndex;
 class CChain;
 class CConnman;
 class CDataStream;
-class CDeterministicMN;
 class CDeterministicMNList;
 class CDeterministicMNListDiff;
-class CDeterministicMNManager;
 class CMasternodeMetaMan;
 class CMasternodeSync;
 class CNode;
 
-class UniValue;
-
 enum ServiceFlags : uint64_t;
 
 /**

src/evo/mnhftx.h

@@ -7,14 +7,12 @@
 
 #include <bls/bls.h>
 #include <gsl/pointers.h>
-#include <primitives/transaction.h>
 #include <sync.h>
 #include <threadsafety.h>
 #include <univalue.h>
 
 #include <optional>
 #include <saltedhasher.h>
-#include <unordered_map>
 #include <unordered_lru_cache.h>
 #include <versionbits.h>
 
@@ -22,6 +20,7 @@ class BlockValidationState;
 class CBlock;
 class CBlockIndex;
 class CEvoDB;
+class CTransaction;
 class ChainstateManager;
 class TxValidationState;
 namespace llmq {

src/evo/providertx.h

@@ -17,8 +17,6 @@
 #include <univalue.h>
 #include <util/underlying.h>
 
-class CBlockIndex;
-class CCoinsViewCache;
 class TxValidationState;
 
 class CProRegTx

src/evo/simplifiedmns.h

@@ -6,18 +6,22 @@
 #define BITCOIN_EVO_SIMPLIFIEDMNS_H
 
 #include <bls/bls.h>
-#include <evo/deterministicmns.h>
 #include <evo/dmn_types.h>
 #include <merkleblock.h>
 #include <netaddress.h>
 #include <pubkey.h>
+#include <sync.h>
+#include <threadsafety.h>
 
 class UniValue;
 class CBlockIndex;
-class CDeterministicMNList;
 class CDeterministicMN;
+class CDeterministicMNList;
+class CDeterministicMNManager;
 class ChainstateManager;
 
+extern RecursiveMutex cs_main;
+
 namespace llmq {
 class CFinalCommitment;
 class CQuorumBlockProcessor;

src/evo/specialtx.h

@@ -11,7 +11,6 @@
 #include <uint256.h>
 #include <version.h>
 
-#include <string_view>
 #include <optional>
 #include <vector>
 

src/evo/specialtxman.h

@@ -5,7 +5,7 @@
 #ifndef BITCOIN_EVO_SPECIALTXMAN_H
 #define BITCOIN_EVO_SPECIALTXMAN_H
 
-#include <primitives/transaction.h>
+#include <consensus/amount.h>
 #include <sync.h>
 #include <threadsafety.h>
 
@@ -17,6 +17,7 @@ class CBlockIndex;
 class CCoinsViewCache;
 class CCreditPoolManager;
 class CDeterministicMNManager;
+class CTransaction;
 class ChainstateManager;
 class CMNHFManager;
 class TxValidationState;

src/llmq/blockprocessor.h

@@ -7,9 +7,7 @@
 
 #include <unordered_lru_cache.h>
 
-#include <chain.h>
+#include <llmq/params.h>
-#include <consensus/params.h>
-#include <primitives/block.h>
 #include <protocol.h>
 #include <saltedhasher.h>
 #include <sync.h>
@@ -19,9 +17,10 @@
 #include <optional>
 
 class BlockValidationState;
+class CBlock;
+class CBlockIndex;
 class CChain;
 class CChainState;
-class CConnman;
 class CDataStream;
 class CDeterministicMNManager;
 class CEvoDB;

src/llmq/chainlocks.h

@@ -7,11 +7,10 @@
 
 #include <llmq/clsig.h>
 
-#include <crypto/common.h>
+#include <crypto/common.h> // For ReadLE64
 #include <llmq/signing.h>
-#include <net.h>
+#include <net.h> // For NodeId
 #include <net_types.h>
-#include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <saltedhasher.h>
 #include <sync.h>
@@ -21,10 +20,10 @@
 #include <atomic>
 #include <map>
 #include <unordered_map>
-#include <unordered_set>
 
-class CChainState;
+class CBlock;
 class CBlockIndex;
+class CChainState;
 class CMasternodeSync;
 class CScheduler;
 class CSporkManager;

src/llmq/commitment.h

@@ -6,16 +6,20 @@
 #define BITCOIN_LLMQ_COMMITMENT_H
 
 #include <bls/bls.h>
-#include <consensus/params.h>
+#include <llmq/params.h>
 #include <primitives/transaction.h>
 #include <util/irange.h>
 #include <util/strencodings.h>
 #include <util/underlying.h>
 
 #include <gsl/pointers.h>
-
 #include <univalue.h>
 
+#include <limits>
+#include <memory>
+#include <string>
+#include <vector>
+
 class CBlockIndex;
 class CDeterministicMNManager;
 class ChainstateManager;

src/llmq/context.h

@@ -12,7 +12,6 @@ class CBLSWorker;
 class CConnman;
 class ChainstateManager;
 class CDeterministicMNManager;
-class CDBWrapper;
 class CEvoDB;
 class CMasternodeMetaMan;
 class CMasternodeSync;

src/llmq/dkgsession.h

@@ -22,7 +22,6 @@ class CConnman;
 class CDeterministicMN;
 class CMasternodeMetaMan;
 class CSporkManager;
-class UniValue;
 
 using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
 

src/llmq/dkgsessionhandler.h

@@ -5,21 +5,28 @@
 #ifndef BITCOIN_LLMQ_DKGSESSIONHANDLER_H
 #define BITCOIN_LLMQ_DKGSESSIONHANDLER_H
 
-#include <ctpl_stl.h>
+#include <net.h> // for NodeId
-#include <net.h>
 
 #include <gsl/pointers.h>
 
 #include <atomic>
+#include <list>
 #include <map>
+#include <memory>
 #include <optional>
+#include <set>
+#include <string>
+#include <thread>
+#include <vector>
 
 class CActiveMasternodeManager;
-class CBlockIndex;
 class CBLSWorker;
+class CBlockIndex;
 class CChainState;
+class CConnman;
 class CDeterministicMNManager;
 class CMasternodeMetaMan;
+class CNode;
 class CSporkManager;
 class PeerManager;
 

src/llmq/dkgsessionmgr.cpp

@@ -18,6 +18,11 @@
 #include <util/underlying.h>
 #include <validation.h>
 
+static bool IsQuorumDKGEnabled(const CSporkManager& sporkman)
+{
+    return sporkman.IsSporkActive(SPORK_17_QUORUM_DKG_ENABLED);
+}
+
 namespace llmq
 {
 static const std::string DB_VVEC = "qdkg_V";
@@ -515,9 +520,4 @@ void CDKGSessionManager::CleanupOldContributions() const
     }
 }
 
-bool IsQuorumDKGEnabled(const CSporkManager& sporkman)
-{
-    return sporkman.IsSporkActive(SPORK_17_QUORUM_DKG_ENABLED);
-}
-
 } // namespace llmq

src/llmq/dkgsessionmgr.h

@@ -104,8 +104,6 @@ private:
     void MigrateDKG();
     void CleanupCache() const;
 };
-
-bool IsQuorumDKGEnabled(const CSporkManager& sporkman);
 } // namespace llmq
 
 #endif // BITCOIN_LLMQ_DKGSESSIONMGR_H

src/llmq/instantsend.h

@@ -6,14 +6,13 @@
 #define BITCOIN_LLMQ_INSTANTSEND_H
 
 #include <llmq/signing.h>
-#include <unordered_lru_cache.h>
 
-#include <chain.h>
+#include <consensus/params.h>
-#include <coins.h>
 #include <net_types.h>
 #include <primitives/transaction.h>
 #include <threadinterrupt.h>
 #include <txmempool.h>
+#include <unordered_lru_cache.h>
 
 #include <gsl/pointers.h>
 
@@ -21,6 +20,7 @@
 #include <unordered_map>
 #include <unordered_set>
 
+class CBlockIndex;
 class CChainState;
 class CDBWrapper;
 class CMasternodeSync;
@@ -365,7 +365,7 @@ public:
     bool IsInstantSendMempoolSigningEnabled() const;
     bool RejectConflictingBlocks() const;
 };
-
+// TODO: split CInstantSendManager and  CInstantSendLock to 2 files
 extern std::unique_ptr<CInstantSendManager> quorumInstantSendManager;
 
 } // namespace llmq

src/llmq/quorums.h

@@ -5,17 +5,16 @@
 #ifndef BITCOIN_LLMQ_QUORUMS_H
 #define BITCOIN_LLMQ_QUORUMS_H
 
-#include <chain.h>
+#include <llmq/params.h>
-#include <consensus/params.h>
-#include <saltedhasher.h>
-#include <threadinterrupt.h>
-#include <unordered_lru_cache.h>
 
 #include <bls/bls.h>
 #include <bls/bls_worker.h>
-#include <protocol.h>
+#include <ctpl_stl.h>
-
 #include <gsl/pointers.h>
+#include <protocol.h>
+#include <saltedhasher.h>
+#include <threadinterrupt.h>
+#include <unordered_lru_cache.h>
 
 #include <atomic>
 #include <map>
@@ -23,6 +22,7 @@
 
 class CActiveMasternodeManager;
 class CBlockIndex;
+class CChain;
 class CChainState;
 class CConnman;
 class CDataStream;

src/llmq/signing.h

@@ -6,14 +6,13 @@
 #define BITCOIN_LLMQ_SIGNING_H
 
 #include <bls/bls.h>
-#include <consensus/params.h>
 #include <gsl/pointers.h>
+#include <llmq/params.h>
 #include <protocol.h>
 #include <random.h>
 #include <saltedhasher.h>
 #include <sync.h>
 #include <threadinterrupt.h>
-#include <univalue.h>
 #include <unordered_lru_cache.h>
 
 #include <unordered_map>
@@ -27,6 +26,7 @@ class CDBWrapper;
 class CInv;
 class CNode;
 class PeerManager;
+class UniValue;
 
 using NodeId = int64_t;
 

src/llmq/signing_shares.h

@@ -5,25 +5,29 @@
 #ifndef BITCOIN_LLMQ_SIGNING_SHARES_H
 #define BITCOIN_LLMQ_SIGNING_SHARES_H
 
-#include <bls/bls.h>
 #include <llmq/signing.h>
-#include <net.h>
+
+#include <bls/bls.h>
 #include <random.h>
 #include <saltedhasher.h>
 #include <serialize.h>
-#include <threadinterrupt.h>
 #include <sync.h>
+#include <threadinterrupt.h>
 #include <uint256.h>
 
 #include <atomic>
+#include <limits>
+#include <memory>
 #include <optional>
+#include <string>
 #include <thread>
 #include <unordered_map>
 #include <utility>
+#include <vector>
 
+class CNode;
+class CConnman;
 class CDeterministicMN;
-class CEvoDB;
-class CScheduler;
 class CSporkManager;
 class PeerManager;
 

src/llmq/snapshot.h

@@ -10,16 +10,16 @@
 #include <llmq/params.h>
 #include <saltedhasher.h>
 #include <serialize.h>
-#include <univalue.h>
+#include <sync.h>
+#include <threadsafety.h>
 #include <unordered_lru_cache.h>
 #include <util/irange.h>
 
 #include <optional>
 
 class CBlockIndex;
-class CDeterministicMN;
+class CEvoDB;
-class CDeterministicMNList;
+class UniValue;
-class CEvoDb;
 
 namespace llmq {
 class CQuorumBlockProcessor;

src/masternode/meta.cpp

@@ -5,6 +5,7 @@
 #include <masternode/meta.h>
 
 #include <flat-database.h>
+#include <univalue.h>
 #include <util/time.h>
 
 #include <sstream>

src/masternode/meta.h

@@ -7,14 +7,17 @@
 
 #include <serialize.h>
 #include <sync.h>
+#include <threadsafety.h>
 #include <uint256.h>
 
-#include <univalue.h>
-
 #include <atomic>
+#include <map>
 #include <memory>
+#include <vector>
 
 class CConnman;
+class UniValue;
+
 template<typename T>
 class CFlatDB;