Skip to content

feat: remove SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBLOCKS_ENABLED #7278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 5 commits into from
Apr 12, 2026
Merged

feat: remove SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBLOCKS_ENABLED #7278

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
afcf3d1
spork: remove SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBL…
UdjinM6 Apr 9, 2026
ee90879
doc: add release notes for SPORK_3/SPORK_9 removal
UdjinM6 Apr 9, 2026
3d9e80e
refactor: cleanup unused members, includes and forward declaration af…
knst Apr 10, 2026
b2cde07
test, doc: address review suggestions for spork removal
UdjinM6 Apr 10, 2026
18b990b
refactor: move superblock sync check inside !IsTestChain() guard
UdjinM6 Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions doc/release-notes-7278.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
Removed Sporks
--------------

* `SPORK_3_INSTANTSEND_BLOCK_FILTERING` and `SPORK_9_SUPERBLOCKS_ENABLED` have
been removed. The behaviours they gated (InstantSend conflicting-block
rejection and superblock payments) are now always enabled. These sporks will
Comment thread
UdjinM6 marked this conversation as resolved.
Outdated
no longer appear in the output of the `spork` RPC.

Updated RPCs
------------

* `getblocktemplate` now always reports `superblocks_enabled` as `true`. The
field is retained for backwards compatibility.
Comment on lines +12 to +15
Copy link
Copy Markdown

@thepastaclaw thepastaclaw Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Release notes should mention getblocktemplate sync requirement on test chains

The release notes document that getblocktemplate now always reports superblocks_enabled as true, and that the removed functionality is "permanently enabled across all networks." However, the practical consequence — that getblocktemplate now throws RPC_CLIENT_IN_INITIAL_DOWNLOAD at superblock heights on regtest/testnet/devnet when the node hasn't completed masternode sync — is not called out. This is a subtle but potentially breaking change for test harnesses.

💡 Suggested change
Suggested change
------------
* `getblocktemplate` now always reports `superblocks_enabled` as `true`. The
field is retained for backwards compatibility.
* `getblocktemplate` now always reports `superblocks_enabled` as `true`. The
field is retained for backwards compatibility. On test networks (regtest,
testnet, devnet), `getblocktemplate` now requires masternode sync to be
complete at superblock heights; previously this check was gated by the
now-removed `SPORK_9`.

source: ['claude']

5 changes: 0 additions & 5 deletions src/governance/governance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,8 +1490,3 @@ std::vector<std::shared_ptr<const CGovernanceObject>> CGovernanceManager::GetApp

return ret;
}

bool AreSuperblocksEnabled(const CSporkManager& sporkman)
{
return sporkman.IsSporkActive(SPORK_9_SUPERBLOCKS_ENABLED);
}
2 changes: 0 additions & 2 deletions src/governance/governance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,4 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
EXCLUSIVE_LOCKS_REQUIRED(cs_store);
};

bool AreSuperblocksEnabled(const CSporkManager& sporkman);

#endif // BITCOIN_GOVERNANCE_GOVERNANCE_H
4 changes: 0 additions & 4 deletions src/instantsend/instantsend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
45 changes: 15 additions & 30 deletions src/masternode/payments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
12 changes: 5 additions & 7 deletions src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,12 +731,10 @@ static RPCHelpMan getblocktemplate()
}
}

// 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))
throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is syncing with network...");
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...");
}
Copy link
Copy Markdown

@thepastaclaw thepastaclaw Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Unconditional superblock sync check changes regtest getblocktemplate behavior

SPORK_9's default value was 4070908800 (OFF). On regtest, AreSuperblocksEnabled() previously returned false unless the spork was explicitly activated, so this sync check was skipped. Now it runs unconditionally. On regtest, superblocks start at height 1500 with a 20-block cycle, and CMasternodeSync::IsSynced() is false unless the full sync state machine completes. This means getblocktemplate will start returning RPC_CLIENT_IN_INITIAL_DOWNLOAD every 20 blocks for plain regtest nodes that don't run masternode sync — a silent behavioral regression. Note: generate() (used by BitcoinTestFramework) is unaffected since it calls BlockAssembler directly, so functional tests won't catch this.

💡 Suggested change
Suggested change
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...");
}
if (!Params().MineBlocksOnDemand()
&& !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...");
}

source: ['codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/rpc/mining.cpp`:
- [SUGGESTION] lines 734-737: Unconditional superblock sync check changes regtest getblocktemplate behavior
  SPORK_9's default value was `4070908800` (OFF). On regtest, `AreSuperblocksEnabled()` previously returned false unless the spork was explicitly activated, so this sync check was skipped. Now it runs unconditionally. On regtest, superblocks start at height 1500 with a 20-block cycle, and `CMasternodeSync::IsSynced()` is false unless the full sync state machine completes. This means `getblocktemplate` will start returning `RPC_CLIENT_IN_INITIAL_DOWNLOAD` every 20 blocks for plain regtest nodes that don't run masternode sync — a silent behavioral regression. Note: `generate()` (used by `BitcoinTestFramework`) is unaffected since it calls `BlockAssembler` directly, so functional tests won't catch this.

Copy link
Copy Markdown

@thepastaclaw thepastaclaw Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Unconditional superblock sync check changes getblocktemplate behavior on test chains

Previously, this check was guarded by AreSuperblocksEnabled(*node.sporkman), which checked SPORK_9. On mainnet, SPORK_9 was hardened to active (value 0), so the check always ran — no behavioral change there. But on regtest/testnet/devnet, SPORK_9 defaulted to 4070908800 (OFF), so this sync check never executed. Now it runs unconditionally.

On regtest (nSuperblockCycle = 20), any node that hasn't completed masternode sync will get RPC_CLIENT_IN_INITIAL_DOWNLOAD from getblocktemplate every 20 blocks. The fix in feature_dip3_deterministicmns.py (adding wait_until(... IsSynced)) confirms the author is aware, but this patches one test caller rather than addressing the API-level change.

The generate() RPC bypasses getblocktemplate (calls BlockAssembler directly), so most functional tests won't surface this, but external tooling or custom test setups using getblocktemplate on regtest/devnet will silently break. Consider either:

  • Guarding with !Params().IsTestChain() to restore previous test-chain behavior, or
  • Documenting this as an intentional behavioral change in the release notes

source: ['claude', 'codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/rpc/mining.cpp`:
- [SUGGESTION] lines 732-735: Unconditional superblock sync check changes `getblocktemplate` behavior on test chains
  Previously, this check was guarded by `AreSuperblocksEnabled(*node.sporkman)`, which checked SPORK_9. On mainnet, SPORK_9 was hardened to active (value `0`), so the check always ran — no behavioral change there. But on regtest/testnet/devnet, SPORK_9 defaulted to `4070908800` (OFF), so this sync check **never executed**. Now it runs unconditionally.

On regtest (`nSuperblockCycle = 20`), any node that hasn't completed masternode sync will get `RPC_CLIENT_IN_INITIAL_DOWNLOAD` from `getblocktemplate` every 20 blocks. The fix in `feature_dip3_deterministicmns.py` (adding `wait_until(... IsSynced)`) confirms the author is aware, but this patches one test caller rather than addressing the API-level change.

The `generate()` RPC bypasses `getblocktemplate` (calls `BlockAssembler` directly), so most functional tests won't surface this, but external tooling or custom test setups using `getblocktemplate` on regtest/devnet will silently break. Consider either:
- Guarding with `!Params().IsTestChain()` to restore previous test-chain behavior, or
- Documenting this as an intentional behavioral change in the release notes

Copy link
Copy Markdown

@thepastaclaw thepastaclaw Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Unconditional superblock sync check changes getblocktemplate behavior on test chains

Before this PR, the sync check at superblock heights was gated by AreSuperblocksEnabled() (SPORK_9), which was OFF on test chains — so this check never executed on regtest/testnet/devnet. Now it runs unconditionally.

On regtest (nSuperblockCycle = 20), any node that hasn't completed masternode sync will get RPC_CLIENT_IN_INITIAL_DOWNLOAD every 20 blocks. The test fix in feature_dip3_deterministicmns.py (adding wait_until(... IsSynced)) confirms this is a real behavioral change.

Notably, the IBD check at lines 722–730 is already guarded by !Params().IsTestChain(), establishing a pattern of leniency on test chains that this new check doesn't follow. If this is fully intentional (as the release notes suggest), no code change is needed — but external test harnesses or mining tooling using getblocktemplate on regtest/devnet may silently break.

source: ['claude']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/rpc/mining.cpp`:
- [SUGGESTION] lines 732-735: Unconditional superblock sync check changes `getblocktemplate` behavior on test chains
  Before this PR, the sync check at superblock heights was gated by `AreSuperblocksEnabled()` (SPORK_9), which was OFF on test chains — so this check never executed on regtest/testnet/devnet. Now it runs unconditionally.

On regtest (`nSuperblockCycle = 20`), any node that hasn't completed masternode sync will get `RPC_CLIENT_IN_INITIAL_DOWNLOAD` every 20 blocks. The test fix in `feature_dip3_deterministicmns.py` (adding `wait_until(... IsSynced)`) confirms this is a real behavioral change.

Notably, the IBD check at lines 722–730 is already guarded by `!Params().IsTestChain()`, establishing a pattern of leniency on test chains that this new check doesn't follow. If this is fully intentional (as the release notes suggest), no code change is needed — but external test harnesses or mining tooling using `getblocktemplate` on regtest/devnet may silently break.

Copy link
Copy Markdown
Author

@UdjinM6 UdjinM6 Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably makes sense... added 18b990b


static unsigned int nTransactionsUpdatedLast;
const CTxMemPool& mempool = EnsureMemPool(node);
Expand Down Expand Up @@ -962,7 +960,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));

Expand Down
6 changes: 1 addition & 5 deletions src/spork.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ class CSporkManager;
*/
enum SporkId : int32_t {
SPORK_2_INSTANTSEND_ENABLED = 10001,
SPORK_3_INSTANTSEND_BLOCK_FILTERING = 10002,
SPORK_9_SUPERBLOCKS_ENABLED = 10008,
SPORK_17_QUORUM_DKG_ENABLED = 10016,
SPORK_19_CHAINLOCKS_ENABLED = 10018,
SPORK_21_QUORUM_ALL_CONNECTED = 10020,
Expand Down Expand Up @@ -69,10 +67,8 @@ struct CSporkDef
};

#define MAKE_SPORK_DEF(name, defaultValue) CSporkDef{name, defaultValue, #name}
[[maybe_unused]] static constexpr std::array<CSporkDef, 7> sporkDefs = {
[[maybe_unused]] static constexpr std::array<CSporkDef, 5> sporkDefs = {
MAKE_SPORK_DEF(SPORK_2_INSTANTSEND_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_3_INSTANTSEND_BLOCK_FILTERING, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_9_SUPERBLOCKS_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_17_QUORUM_DKG_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_19_CHAINLOCKS_ENABLED, 4070908800ULL), // OFF
MAKE_SPORK_DEF(SPORK_21_QUORUM_ALL_CONNECTED, 4070908800ULL), // OFF
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_governance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def run_test(self):
self.expected_v20_budget = satoshi_round("18.57142860")

self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 4070908800)
self.nodes[0].sporkupdate("SPORK_9_SUPERBLOCKS_ENABLED", 0)
self.wait_for_sporks_same()

assert_equal(len(self.nodes[0].gobject("list-prepared")), 0)
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_governance_cl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ def run_test(self):
self.sync_blocks()
self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash())

self.nodes[0].sporkupdate("SPORK_9_SUPERBLOCKS_ENABLED", 0)
self.wait_for_sporks_same()
Comment thread
UdjinM6 marked this conversation as resolved.
Outdated

# Move to the superblock cycle start block
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_mnehf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def set_sporks(self):

self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", spork_enabled)
self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", spork_disabled)
self.nodes[0].sporkupdate("SPORK_3_INSTANTSEND_BLOCK_FILTERING", spork_disabled)
self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", spork_disabled)
self.wait_for_sporks_same()

Expand Down
4 changes: 2 additions & 2 deletions test/functional/feature_multikeysporks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ def test_spork(self, spork_name, final_value):

def run_test(self):
self.test_spork('SPORK_2_INSTANTSEND_ENABLED', 2)
self.test_spork('SPORK_3_INSTANTSEND_BLOCK_FILTERING', 3)
self.test_spork('SPORK_19_CHAINLOCKS_ENABLED', 3)
for node in self.nodes:
assert self.get_test_spork_value(node, 'SPORK_2_INSTANTSEND_ENABLED') == 2
assert self.get_test_spork_value(node, 'SPORK_3_INSTANTSEND_BLOCK_FILTERING') == 3
assert self.get_test_spork_value(node, 'SPORK_19_CHAINLOCKS_ENABLED') == 3


if __name__ == '__main__':
Expand Down
1 change: 0 additions & 1 deletion test/functional/test_framework/test_framework.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,6 @@ def setup_network(self):

# Enable InstantSend (including block filtering) and ChainLocks by default
Comment thread
UdjinM6 marked this conversation as resolved.
Outdated
Comment thread
UdjinM6 marked this conversation as resolved.
Outdated
self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 0)
self.nodes[0].sporkupdate("SPORK_3_INSTANTSEND_BLOCK_FILTERING", 0)
self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 0)
self.wait_for_sporks_same()
self.bump_mocktime(1)
Expand Down
Loading