Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions doc/release-notes-7278.md
Original file line number Diff line number Diff line change
@@ -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.
Comment on lines +12 to +15
Copy link
Copy Markdown

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']


* `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.
Comment on lines +4 to +19
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Suggestion: Release notes still omit that sporkupdate now rejects the removed spork names

This note covers the removed sporks disappearing from spork output, but it still misses the other user-visible RPC change from dropping these entries out of sporkDefs: sporkupdate now resolves SPORK_3_INSTANTSEND_BLOCK_FILTERING and SPORK_9_SUPERBLOCKS_ENABLED to SPORK_INVALID and returns RPC_INVALID_PARAMETER (src/rpc/node.cpp:218-220). That is a compatibility change for any automation that still tries to toggle those sporks, so it should be called out alongside the existing RPC notes.

source: ['codex-general']

🤖 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 `doc/release-notes-7278.md`:
- [SUGGESTION] lines 4-19: Release notes still omit that `sporkupdate` now rejects the removed spork names
  This note covers the removed sporks disappearing from `spork` output, but it still misses the other user-visible RPC change from dropping these entries out of `sporkDefs`: `sporkupdate` now resolves `SPORK_3_INSTANTSEND_BLOCK_FILTERING` and `SPORK_9_SUPERBLOCKS_ENABLED` to `SPORK_INVALID` and returns `RPC_INVALID_PARAMETER` (`src/rpc/node.cpp:218-220`). That is a compatibility change for any automation that still tries to toggle those sporks, so it should be called out alongside the existing RPC notes.

5 changes: 2 additions & 3 deletions src/evo/chainhelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
{}
Expand Down
4 changes: 1 addition & 3 deletions src/evo/chainhelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ class CMasternodeSync;
class CMNHFManager;
class CMNPaymentsProcessor;
class CSpecialTxProcessor;
class CSporkManager;
class CTransaction;
class uint256;
struct CCreditPool;
Expand Down Expand Up @@ -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 */
Expand Down
8 changes: 2 additions & 6 deletions src/governance/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
}
3 changes: 0 additions & 3 deletions src/governance/governance.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class CGovernanceObject;
class CGovernanceVote;
class CMasternodeMetaMan;
class CMasternodeSync;
class CSporkManager;
class CSuperblock;

class UniValue;
Expand Down Expand Up @@ -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
4 changes: 0 additions & 4 deletions src/instantsend/instantsend.cpp
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
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
7 changes: 2 additions & 5 deletions src/masternode/payments.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class CGovernanceManager;
class ChainstateManager;
class CMasternodeSync;
class CTransaction;
class CSporkManager;
class CTxOut;

struct CMutableTransaction;
Expand All @@ -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,
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/node/chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down
18 changes: 7 additions & 11 deletions src/rpc/mining.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));

Expand Down
6 changes: 1 addition & 5 deletions src/spork.h
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: 1 addition & 0 deletions test/functional/feature_dip3_deterministicmns.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ def run_test(self):

multisig = self.nodes[0].createmultisig(1, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
self.update_mn_payee(mns[0], multisig)
self.wait_until(lambda: self.nodes[0].mnsync("status")['IsSynced'])
found_multisig_payee = False
for _ in range(len(mns)):
bt = self.nodes[0].getblocktemplate()
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_governance.py
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
3 changes: 0 additions & 3 deletions test/functional/feature_governance_cl.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +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()

# Move to the superblock cycle start block
n = sb_cycle - self.nodes[0].getblockcount() % sb_cycle
if n > 0:
Expand Down
1 change: 0 additions & 1 deletion test/functional/feature_mnehf.py
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
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
3 changes: 1 addition & 2 deletions test/functional/test_framework/test_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -1844,9 +1844,8 @@ def setup_network(self):
for i in range(1, num_simple_nodes):
force_finish_mnsync(self.nodes[i])

# Enable InstantSend (including block filtering) and ChainLocks by default
# Enable InstantSend and ChainLocks by default
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