Merge #7278: feat: remove SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBLOCKS_ENABLED

18b990b refactor: move superblock sync check inside !IsTestChain() guard (UdjinM6)
b2cde07 test, doc: address review suggestions for spork removal (UdjinM6)
3d9e80e refactor: cleanup unused members, includes and forward declaration after sporks removal (Konstantin Akimov)
ee90879 doc: add release notes for SPORK_3/SPORK_9 removal (UdjinM6)
afcf3d1 spork: remove SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBLOCKS_ENABLED (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Both sporks have been effectively permanent fixtures of the network and their gating logic is no longer needed.

  ## What was done?
  Drop the enum entries, spork definitions, the `AreSuperblocksEnabled()` helper, and all associated branches in governance, InstantSend, masternode payments and mining RPC. `getblocktemplate` now always reports `superblocks_enabled` as `true`.

  Functional tests are updated to stop toggling the removed sporks; `feature_multikeysporks.py` now uses `SPORK_19_CHAINLOCKS_ENABLED` as its second spork under test.

  ## How Has This Been Tested?
  Run tests

  ## Breaking Changes
  n/a

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 18b990b

Tree-SHA512: 2b84b1473ef1b81418fd29a0d14039ed98cda0cb7608bbf6798db62c72e76e11685ed1092cc5768c5ea95e6558b6909454fd6f5a3e2b46eb42a691294573e397

Author: PastaPastaPasta

doc/release-notes-7278.md

@@ -0,0 +1,19 @@
+Removed Sporks
+--------------
+
+* `SPORK_3_INSTANTSEND_BLOCK_FILTERING` and `SPORK_9_SUPERBLOCKS_ENABLED` have
+  been removed. These sporks were already effectively always enabled on mainnet
+  but continued to gate behavior on test networks (testnet, regtest, and devnet).
+  The associated functionality (InstantSend conflicting-block rejection and
+  superblock payments) is now permanently enabled across all networks, and the
+  sporks will no longer appear in the `spork` RPC output.
+
+Updated RPCs
+------------
+
+* `getblocktemplate` now always reports `superblocks_enabled` as `true`. The
+  field is retained for backwards compatibility.
+
+* `getblocktemplate` now requires a fully synced node at superblock heights.
+  This check is skipped on test networks. Previously it was gated by
+  `SPORK_9_SUPERBLOCKS_ENABLED` which was already always active on mainnet.

src/evo/chainhelper.cpp

@@ -18,13 +18,12 @@ CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmn
                                      llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
                                      llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
                                      const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync,
-                                     const CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks,
-                                     const llmq::CQuorumManager& qman) :
+                                     const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) :
     isman{isman},
     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, mn_sync, sporkman)},
+    mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, govman, chainman, consensus_params, mn_sync)},
     special_tx{std::make_unique<CSpecialTxProcessor>(*credit_pool_manager, dmnman, *ehf_manager, qblockman, qsnapman,
                                                      chainman, consensus_params, chainlocks, qman)}
 {}

src/evo/chainhelper.h

@@ -19,7 +19,6 @@ class CMasternodeSync;
 class CMNHFManager;
 class CMNPaymentsProcessor;
 class CSpecialTxProcessor;
-class CSporkManager;
 class CTransaction;
 class uint256;
 struct CCreditPool;
@@ -61,8 +60,7 @@ class CChainstateHelper
                                llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
                                llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
                                const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync,
-                               const CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks,
-                               const llmq::CQuorumManager& qman);
+                               const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman);
     ~CChainstateHelper();
 
     /** Passthrough functions to chainlock::Chainlocks */

src/governance/governance.cpp

@@ -11,16 +11,17 @@
 #include <governance/validators.h>
 #include <masternode/meta.h>
 #include <masternode/sync.h>
-#include <spork.h>
 
 #include <chain.h>
 #include <chainparams.h>
 #include <common/bloom.h>
 #include <deploymentstatus.h>
+#include <net.h>
 #include <node/interface_ui.h>
 #include <protocol.h>
 #include <shutdown.h>
 #include <timedata.h>
+#include <util/check.h>
 #include <util/thread.h>
 #include <util/time.h>
 #include <validationinterface.h>
@@ -1490,8 +1491,3 @@ std::vector<std::shared_ptr<const CGovernanceObject>> CGovernanceManager::GetApp
 
     return ret;
 }
-
-bool AreSuperblocksEnabled(const CSporkManager& sporkman)
-{
-    return sporkman.IsSporkActive(SPORK_9_SUPERBLOCKS_ENABLED);
-}

src/governance/governance.h

@@ -40,7 +40,6 @@ class CGovernanceObject;
 class CGovernanceVote;
 class CMasternodeMetaMan;
 class CMasternodeSync;
-class CSporkManager;
 class CSuperblock;
 
 class UniValue;
@@ -444,6 +443,4 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
         EXCLUSIVE_LOCKS_REQUIRED(cs_store);
 };
 
-bool AreSuperblocksEnabled(const CSporkManager& sporkman);
-
 #endif // BITCOIN_GOVERNANCE_GOVERNANCE_H

src/instantsend/instantsend.cpp

@@ -474,10 +474,6 @@ bool CInstantSendManager::RejectConflictingBlocks() const
     if (!m_mn_sync.IsBlockchainSynced()) {
         return false;
     }
-    if (!spork_manager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING)) {
-        LogPrint(BCLog::INSTANTSEND, "%s: spork3 is off, skipping transaction locking checks\n", __func__);
-        return false;
-    }
     return true;
 }
 

src/masternode/payments.cpp

@@ -241,17 +241,6 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo
 
     // we are synced and possibly on a superblock now
 
-    if (!AreSuperblocksEnabled(m_sporkman)) {
-        // should NOT allow superblocks at all, when superblocks are disabled
-        // revert to block reward limits in this case
-        LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Superblocks are disabled, no superblocks allowed\n", __func__);
-        if(!isBlockRewardValueMet) {
-            strErrorRet = strprintf("coinbase pays too much at height %d (actual=%d vs limit=%d), exceeded block reward, superblocks are disabled",
-                                    nBlockHeight, block.vtx[0]->GetValueOut(), blockReward);
-        }
-        return isBlockRewardValueMet;
-    }
-
     if (!check_superblock) return true;
 
     const auto tip_mn_list = m_dmnman.GetListAtChainTip();
@@ -308,26 +297,23 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB
     }
 
     // superblocks started
+    if (!check_superblock) return true;
 
-    if (AreSuperblocksEnabled(m_sporkman)) {
-        if (!check_superblock) return true;
-        const auto tip_mn_list = m_dmnman.GetListAtChainTip();
-        if (m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) {
-            if (m_govman.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight,
-                                           blockSubsidy + feeReward)) {
-                LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */
-                // continue validation, should also pay MN
-            } else {
-                LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */
-                // should NOT allow such superblocks, when superblocks are enabled
-                return false;
-            }
+    const auto tip_mn_list = m_dmnman.GetListAtChainTip();
+    if (m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) {
+        if (m_govman.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight,
+                                       blockSubsidy + feeReward)) {
+            LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", /* Continued */
+                     __func__, nBlockHeight, txNew.ToString());
+            // continue validation, should also pay MN
         } else {
-            LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- No triggered superblock detected at height %d\n", __func__, nBlockHeight);
+            LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Invalid superblock detected at height %d: %s", /* Continued */
+                      __func__, nBlockHeight, txNew.ToString());
+            return false;
         }
     } else {
-        // should NOT allow superblocks at all, when superblocks are disabled
-        LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Superblocks are disabled, no superblocks allowed\n", __func__);
+        LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- No triggered superblock detected at height %d\n",
+                 __func__, nBlockHeight);
     }
 
     return true;
@@ -338,10 +324,9 @@ void CMNPaymentsProcessor::FillBlockPayments(CMutableTransaction& txNew, const C
 {
     int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1;
 
-    // only create superblocks if spork is enabled AND if superblock is actually triggered
-    // (height should be validated inside)
+    // Only create superblocks when one is actually triggered.
     const auto tip_mn_list = m_dmnman.GetListAtChainTip();
-    if (AreSuperblocksEnabled(m_sporkman) && m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) {
+    if (m_govman.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) {
         LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Triggered superblock creation at height %d\n", __func__, nBlockHeight);
         m_govman.GetSuperblockPayments(tip_mn_list, nBlockHeight, voutSuperblockPaymentsRet);
     }

src/masternode/payments.h

@@ -17,7 +17,6 @@ class CGovernanceManager;
 class ChainstateManager;
 class CMasternodeSync;
 class CTransaction;
-class CSporkManager;
 class CTxOut;
 
 struct CMutableTransaction;
@@ -38,7 +37,6 @@ class CMNPaymentsProcessor
     const ChainstateManager& m_chainman;
     const Consensus::Params& m_consensus_params;
     const CMasternodeSync& m_mn_sync;
-    const CSporkManager& m_sporkman;
 
 private:
     [[nodiscard]] bool GetBlockTxOuts(const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward,
@@ -51,9 +49,8 @@ class CMNPaymentsProcessor
 
 public:
     explicit CMNPaymentsProcessor(CDeterministicMNManager& dmnman, CGovernanceManager& govman, const ChainstateManager& chainman,
-                                  const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync, const CSporkManager& sporkman) :
-        m_dmnman{dmnman}, m_govman{govman}, m_chainman{chainman}, m_consensus_params{consensus_params}, m_mn_sync{mn_sync},
-        m_sporkman{sporkman} {}
+                                  const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync) :
+        m_dmnman{dmnman}, m_govman{govman}, m_chainman{chainman}, m_consensus_params{consensus_params}, m_mn_sync{mn_sync} {}
 
     bool IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock);
     bool IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, const bool check_superblock);

src/node/chainstate.cpp

@@ -236,7 +236,7 @@ void DashChainstateSetup(ChainstateManager& chainman,
     mempool->ConnectManagers(dmnman.get(), llmq_ctx->isman.get());
     chain_helper.reset();
     chain_helper = std::make_unique<CChainstateHelper>(evodb, *dmnman, govman, *(llmq_ctx->isman), *(llmq_ctx->quorum_block_processor),
-                                                       *(llmq_ctx->qsnapman), chainman, consensus_params, mn_sync, sporkman, chainlocks,
+                                                       *(llmq_ctx->qsnapman), chainman, consensus_params, mn_sync, chainlocks,
                                                        *(llmq_ctx->qman));
 }
 

src/rpc/mining.cpp

@@ -14,10 +14,12 @@
 #include <core_io.h>
 #include <deploymentinfo.h>
 #include <deploymentstatus.h>
+#include <governance/classes.h>
 #include <key_io.h>
 #include <llmq/blockprocessor.h>
 #include <llmq/context.h>
 #include <evo/evodb.h>
+#include <masternode/sync.h>
 #include <net.h>
 #include <node/context.h>
 #include <node/miner.h>
@@ -42,10 +44,6 @@
 #include <validationinterface.h>
 #include <warnings.h>
 
-#include <governance/classes.h>
-#include <governance/governance.h>
-#include <masternode/sync.h>
-
 #include <memory>
 #include <stdint.h>
 
@@ -729,14 +727,12 @@ static RPCHelpMan getblocktemplate()
         if (active_chainstate.IsInitialBlockDownload()) {
             throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is in initial sync and waiting for blocks...");
         }
-    }
 
-    // next bock is a superblock and we need governance info to correctly construct it
-    CHECK_NONFATAL(node.sporkman);
-    if (AreSuperblocksEnabled(*node.sporkman)
-        && !node.mn_sync->IsSynced()
-        && CSuperblock::IsValidBlockHeight(active_chain.Height() + 1))
+        if (!node.mn_sync->IsSynced() && CSuperblock::IsValidBlockHeight(active_chain.Height() + 1)) {
+            // Next block is a superblock but we need governance info to correctly construct it.
             throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is syncing with network...");
+        }
+    }
 
     static unsigned int nTransactionsUpdatedLast;
     const CTxMemPool& mempool = EnsureMemPool(node);
@@ -962,7 +958,7 @@ static RPCHelpMan getblocktemplate()
     }
     result.pushKV("superblock", superblockObjArray);
     result.pushKV("superblocks_started", pindexPrev->nHeight + 1 > consensusParams.nSuperblockStartBlock);
-    result.pushKV("superblocks_enabled", AreSuperblocksEnabled(*node.sporkman));
+    result.pushKV("superblocks_enabled", true);
 
     result.pushKV("coinbase_payload", HexStr(pblock->vtx[0]->vExtraPayload));