Skip to content

refactor: remove dependencies of CChainState on CMasternodeSync #7212

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

Open
knst wants to merge 7 commits into
base: develop
Choose a base branch
from
+94 −62
Open

refactor: remove dependencies of CChainState on CMasternodeSync #7212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
91824b7
fix: make regression miner_tests works correctly with InstantSend ena…
knst Mar 12, 2026
6b2f318
fix: make regression miner_tests works correctly with InstantSend ena…
knst Mar 12, 2026
cabf5e4
fix: make even more miner_tests friendly to InstantSend enabled (Test…
knst Mar 12, 2026
c06dd4f
refactor: drop dependency of InstantSend on CMasternodeSync
knst Mar 12, 2026
d81e9f9
refactor: drop dependency of CChainState on CMasternodeSync
knst Mar 11, 2026
57fb6ee
refactor: remove usages of CMasternodeSync from chainstate
knst Mar 12, 2026
94eeabd
refactor: new stub implementation (NullNodeSyncNotifier) of NodeSyncN…
knst Mar 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
2 changes: 1 addition & 1 deletion src/chainlock/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void ChainLockSigner::TrySignChainTip()
// considered safe when it is islocked or at least known since 10 minutes (from mempool or block). These checks are
// performed for the tip (which we try to sign) and the previous 5 blocks. If a ChainLocked block is found on the
// way down, we consider all TXs to be safe.
if (m_isman.IsInstantSendEnabled() && m_isman.RejectConflictingBlocks()) {
if (m_isman.IsInstantSendEnabled()) {
const auto* pindexWalk = pindex;
while (pindexWalk != nullptr) {
if (pindex->nHeight - pindexWalk->nHeight > TX_CONFIRM_THRESHOLD) {
Expand Down
8 changes: 3 additions & 5 deletions src/evo/chainhelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, CGovernanceManager& govman,
llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync,
const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) :
const Consensus::Params& consensus_params, 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)},
mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, govman, chainman, consensus_params)},
special_tx{std::make_unique<CSpecialTxProcessor>(*credit_pool_manager, dmnman, *ehf_manager, qblockman, qsnapman,
chainman, consensus_params, chainlocks, qman)}
{}
Expand Down Expand Up @@ -68,8 +68,6 @@ bool CChainstateHelper::RemoveConflictingISLockByTx(const CTransaction& tx)
return true;
}

bool CChainstateHelper::ShouldInstantSendRejectConflicts() const { return isman.RejectConflictingBlocks(); }

std::unordered_map<uint8_t, int> CChainstateHelper::GetSignalsStage(const CBlockIndex* const pindexPrev)
{
return ehf_manager->GetSignalsStage(pindexPrev);
Expand Down
6 changes: 2 additions & 4 deletions src/evo/chainhelper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class CDeterministicMNManager;
class CEvoDB;
class CGovernanceManager;
class ChainstateManager;
class CMasternodeSync;
class CMNHFManager;
class CMNPaymentsProcessor;
class CSpecialTxProcessor;
Expand Down Expand Up @@ -53,8 +52,8 @@ class CChainstateHelper
explicit CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, CGovernanceManager& govman,
llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync,
const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman);
const Consensus::Params& consensus_params, const chainlock::Chainlocks& chainlocks,
const llmq::CQuorumManager& qman);
~CChainstateHelper();

/** Passthrough functions to chainlock::Chainlocks */
Expand All @@ -69,7 +68,6 @@ class CChainstateHelper
std::optional<std::pair</*islock_hash=*/uint256, /*txid=*/uint256>> ConflictingISLockIfAny(const CTransaction& tx) const;
bool IsInstantSendWaitingForTx(const uint256& hash) const;
bool RemoveConflictingISLockByTx(const CTransaction& tx);
bool ShouldInstantSendRejectConflicts() const;

std::unordered_map<uint8_t, int> GetSignalsStage(const CBlockIndex* const pindexPrev);
};
Expand Down
2 changes: 2 additions & 0 deletions src/governance/governance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ bool CGovernanceManager::LoadCache(bool load_cache)
return is_valid;
}

bool CGovernanceManager::IsValidAndSynced() const { return is_valid && m_mn_sync.IsSynced(); }

void CGovernanceManager::RelayObject(const CGovernanceObject& obj)
{
AssertLockNotHeld(cs_relay);
Expand Down
1 change: 1 addition & 0 deletions src/governance/governance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent

// Basic initialization and querying
bool IsValid() const override { return is_valid; }
bool IsValidAndSynced() const;
bool LoadCache(bool load_cache)
EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
[[nodiscard]] static RPCResult GetJsonHelp(const std::string& key, bool optional);
Expand Down
1 change: 0 additions & 1 deletion src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1992,7 +1992,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
chainman,
*node.govman,
*node.mn_metaman,
*node.mn_sync,
*node.sporkman,
*node.chainlocks,
node.chain_helper,
Expand Down
15 changes: 2 additions & 13 deletions src/instantsend/instantsend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <chainparams.h>
#include <consensus/validation.h>
#include <masternode/sync.h>
#include <node/blockstorage.h>
#include <spork.h>
#include <stats/client.h>
Expand All @@ -15,11 +14,9 @@ using node::fImporting;
using node::fReindex;

namespace llmq {
CInstantSendManager::CInstantSendManager(CSporkManager& sporkman, const CMasternodeSync& mn_sync,
const util::DbWrapperParams& db_params) :
CInstantSendManager::CInstantSendManager(CSporkManager& sporkman, const util::DbWrapperParams& db_params) :
db{db_params},
spork_manager{sporkman},
m_mn_sync{mn_sync}
spork_manager{sporkman}
{
}

Expand Down Expand Up @@ -469,14 +466,6 @@ bool CInstantSendManager::IsInstantSendEnabled() const
return !fReindex && !fImporting && spork_manager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED);
}

bool CInstantSendManager::RejectConflictingBlocks() const
{
if (!m_mn_sync.IsBlockchainSynced()) {
return false;
}
return true;
}

Uint256HashMap<instantsend::InstantSendLockPtr> CInstantSendManager::RemoveConfirmedInstantSendLocks(const CBlockIndex* pindex)
{
int nUntilHeight = pindex->nHeight;
Expand Down
11 changes: 1 addition & 10 deletions src/instantsend/instantsend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

class CBlockIndex;
class CDataStream;
class CMasternodeSync;
class CSporkManager;
namespace Consensus {
struct LLMQParams;
Expand Down Expand Up @@ -54,7 +53,6 @@ class CInstantSendManager
private:
instantsend::CInstantSendDb db;
CSporkManager& spork_manager;
const CMasternodeSync& m_mn_sync;

mutable Mutex cs_pendingLocks;
// Incoming and not verified yet
Expand Down Expand Up @@ -90,8 +88,7 @@ class CInstantSendManager
CInstantSendManager() = delete;
CInstantSendManager(const CInstantSendManager&) = delete;
CInstantSendManager& operator=(const CInstantSendManager&) = delete;
explicit CInstantSendManager(CSporkManager& sporkman, const CMasternodeSync& mn_sync,
const util::DbWrapperParams& db_params);
explicit CInstantSendManager(CSporkManager& sporkman, const util::DbWrapperParams& db_params);
~CInstantSendManager();

void AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined)
Expand Down Expand Up @@ -154,12 +151,6 @@ class CInstantSendManager
int GetTipHeight() const EXCLUSIVE_LOCKS_REQUIRED(!cs_height_cache);

bool IsInstantSendEnabled() const;
/**
* If true, MN should sign all transactions, if false, MN should not sign
* transactions in mempool, but should sign txes included in a block. This
* allows ChainLocks to continue even while this spork is disabled.
*/
bool RejectConflictingBlocks() const;
Uint256HashMap<instantsend::InstantSendLockPtr> RemoveConfirmedInstantSendLocks(const CBlockIndex* pindex)
EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
};
Expand Down
7 changes: 3 additions & 4 deletions src/llmq/context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,16 @@
#include <validation.h>

LLMQContext::LLMQContext(CDeterministicMNManager& dmnman, CEvoDB& evo_db, CSporkManager& sporkman,
ChainstateManager& chainman, const CMasternodeSync& mn_sync,
const util::DbWrapperParams& db_params, int8_t bls_threads, int16_t worker_count,
int64_t max_recsigs_age) :
ChainstateManager& chainman, const util::DbWrapperParams& db_params, int8_t bls_threads,
int16_t worker_count, int64_t max_recsigs_age) :
bls_worker{std::make_shared<CBLSWorker>()},
qsnapman{std::make_unique<llmq::CQuorumSnapshotManager>(evo_db)},
quorum_block_processor{std::make_unique<llmq::CQuorumBlockProcessor>(chainman.ActiveChainstate(), dmnman, evo_db,
*qsnapman, bls_threads)},
qman{std::make_unique<llmq::CQuorumManager>(*bls_worker, dmnman, evo_db, *quorum_block_processor, *qsnapman,
chainman, db_params)},
sigman{std::make_unique<llmq::CSigningManager>(*qman, db_params, max_recsigs_age)},
isman{std::make_unique<llmq::CInstantSendManager>(sporkman, mn_sync, db_params)}
isman{std::make_unique<llmq::CInstantSendManager>(sporkman, db_params)}
{
// Have to start it early to let VerifyDB check ChainLock signatures in coinbase
bls_worker->Start(worker_count);
Expand Down
6 changes: 2 additions & 4 deletions src/llmq/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class CBLSWorker;
class ChainstateManager;
class CDeterministicMNManager;
class CEvoDB;
class CMasternodeSync;
class CSporkManager;
class PeerManager;

Expand All @@ -34,9 +33,8 @@ struct LLMQContext {
LLMQContext(const LLMQContext&) = delete;
LLMQContext& operator=(const LLMQContext&) = delete;
explicit LLMQContext(CDeterministicMNManager& dmnman, CEvoDB& evo_db, CSporkManager& sporkman,
ChainstateManager& chainman, const CMasternodeSync& mn_sync,
const util::DbWrapperParams& db_params, int8_t bls_threads, int16_t worker_count,
int64_t max_recsigs_age);
ChainstateManager& chainman, const util::DbWrapperParams& db_params, int8_t bls_threads,
int16_t worker_count, int64_t max_recsigs_age);
~LLMQContext();

/** Guaranteed if LLMQContext is initialized then all members are valid too
Expand Down
6 changes: 3 additions & 3 deletions src/masternode/payments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ CAmount PlatformShare(const CAmount reward)
int nOffset = nBlockHeight % m_consensus_params.nBudgetPaymentsCycleBlocks;
if (nOffset < m_consensus_params.nBudgetPaymentsWindowBlocks) {
// NOTE: old budget system is disabled since 12.1
if(m_mn_sync.IsSynced()) {
if (m_govman.IsValidAndSynced()) {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 13, 2026

Choose a reason for hiding this comment

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

P2 Badge Keep old-budget reward checks independent of gov cache

This branch now depends on m_govman.IsValidAndSynced(), but the previous logic only required masternode sync; as a result, when governance is disabled (e.g. -disablegovernance) or governance state is unavailable, a fully synced node takes the "accepting block" path and skips the blockReward cap for old-budget-window heights. That can allow overpaying coinbase amounts to be accepted between nBudgetPaymentsStartBlock and nSuperblockStartBlock, creating validation differences versus nodes that still enforce the cap.

Useful? React with 👍 / 👎.

// no old budget blocks should be accepted here on mainnet,
// testnet/devnet/regtest should produce regular blocks only
LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- WARNING! Client synced but old budget system is disabled, checking block value against block reward\n", __func__);
Expand Down Expand Up @@ -232,7 +232,7 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo
return false;
}

if (!m_mn_sync.IsSynced() || !m_govman.IsValid()) {
if (!m_govman.IsValidAndSynced()) {
LogPrint(BCLog::MNPAYMENTS, "CMNPaymentsProcessor::%s -- WARNING! Not enough data, checked superblock max bounds only\n", __func__);
// not enough data for full checks but at least we know that the superblock limits were honored.
// We rely on the network to have followed the correct chain in this case
Expand Down Expand Up @@ -280,7 +280,7 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB
return false;
}

if (!m_mn_sync.IsSynced() || !m_govman.IsValid()) {
if (!m_govman.IsValidAndSynced()) {
// governance data is either incomplete or non-existent
LogPrint(BCLog::MNPAYMENTS, "CMNPaymentsProcessor::%s -- WARNING! Not enough data, skipping superblock payee checks\n", __func__);
return true; // not an error
Expand Down
13 changes: 8 additions & 5 deletions src/masternode/payments.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class CBlockIndex;
class CDeterministicMNManager;
class CGovernanceManager;
class ChainstateManager;
class CMasternodeSync;
class CTransaction;
class CTxOut;

Expand All @@ -36,7 +35,6 @@ class CMNPaymentsProcessor
CGovernanceManager& m_govman;
const ChainstateManager& m_chainman;
const Consensus::Params& m_consensus_params;
const CMasternodeSync& m_mn_sync;

private:
[[nodiscard]] bool GetBlockTxOuts(const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward,
Expand All @@ -48,9 +46,14 @@ class CMNPaymentsProcessor
[[nodiscard]] bool IsOldBudgetBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet);

public:
explicit CMNPaymentsProcessor(CDeterministicMNManager& dmnman, CGovernanceManager& govman, const ChainstateManager& chainman,
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} {}
explicit CMNPaymentsProcessor(CDeterministicMNManager& dmnman, CGovernanceManager& govman,
const ChainstateManager& chainman, const Consensus::Params& consensus_params) :
m_dmnman{dmnman},
m_govman{govman},
m_chainman{chainman},
m_consensus_params{consensus_params}
{
}

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
5 changes: 5 additions & 0 deletions src/masternode/sync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
#include <util/time.h>
#include <util/translation.h>

#include <cassert>

void NullNodeSyncNotifier::SyncReset() { assert(false); }
void NullNodeSyncNotifier::SyncFinished() { assert(false); }
Comment thread
knst marked this conversation as resolved.
Comment on lines +14 to +15
Copy link
Copy Markdown

@thepastaclaw thepastaclaw May 2, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: NullNodeSyncNotifier uses lowercase assert(false) — compiled out under NDEBUG

The header at src/masternode/sync.h:35-41 documents that NullNodeSyncNotifier 'asserts on any call — if sync state is being advanced, a real notifier (NodeSyncNotifierImpl) must be used instead.' The implementation uses the C assert() macro from , which expands to nothing when NDEBUG is defined. Release builds of dashd define NDEBUG (only --enable-debug clears it), so in release builds these methods silently return rather than abort. If a chainstate-only context wired with NullNodeSyncNotifier ever routes a call into CMasternodeSync::SwitchToNextAsset() or Reset(true, /fNotifyReset=/true), the stub will silently no-op and the documented invariant will be violated. Use Bitcoin Core's Assert() from util/check.h, which survives NDEBUG.

💡 Suggested change
Suggested change
void NullNodeSyncNotifier::SyncReset() { assert(false); }
void NullNodeSyncNotifier::SyncFinished() { assert(false); }
void NullNodeSyncNotifier::SyncReset() { Assert(false); }
void NullNodeSyncNotifier::SyncFinished() { Assert(false); }

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/masternode/sync.cpp`:
- [SUGGESTION] lines 14-15: NullNodeSyncNotifier uses lowercase assert(false) — compiled out under NDEBUG
  The header at src/masternode/sync.h:35-41 documents that NullNodeSyncNotifier 'asserts on any call — if sync state is being advanced, a real notifier (NodeSyncNotifierImpl) must be used instead.' The implementation uses the C `assert()` macro from <cassert>, which expands to nothing when NDEBUG is defined. Release builds of dashd define NDEBUG (only `--enable-debug` clears it), so in release builds these methods silently return rather than abort. If a chainstate-only context wired with NullNodeSyncNotifier ever routes a call into CMasternodeSync::SwitchToNextAsset() or Reset(true, /*fNotifyReset=*/true), the stub will silently no-op and the documented invariant will be violated. Use Bitcoin Core's `Assert()` from util/check.h, which survives NDEBUG.


CMasternodeSync::CMasternodeSync(std::unique_ptr<NodeSyncNotifier>&& sync_notifier) :
nTimeAssetSyncStarted{GetTime()},
nTimeLastBumped{GetTime()},
Expand Down
14 changes: 14 additions & 0 deletions src/masternode/sync.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ class NodeSyncNotifier
virtual ~NodeSyncNotifier() = default;
};

/** Stub implementation for use in chainstate-only (non-network) contexts.
* CMasternodeSync constructed with this notifier permanently returns
* IsBlockchainSynced()=false and IsSynced()=false, which correctly disables
* network-dependent validation paths.
*
* Asserts on any call — if sync state is being advanced, a real notifier
* (NodeSyncNotifierImpl) must be used instead. */
Comment on lines +35 to +41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Doc comment overstates the notifier guarantee

Line 36–38 says this setup permanently keeps IsBlockchainSynced() and IsSynced() false, but CMasternodeSync::SwitchToNextAsset() can move from MASTERNODE_SYNC_BLOCKCHAIN to MASTERNODE_SYNC_GOVERNANCE before any notifier assert, which makes IsBlockchainSynced() true. Please soften this to “expected in chainstate-only usage” to avoid a misleading contract.

Suggested wording tweak 
-/** Stub implementation for use in chainstate-only (non-network) contexts.
- *  CMasternodeSync constructed with this notifier permanently returns
- *  IsBlockchainSynced()=false and IsSynced()=false, which correctly disables
- *  network-dependent validation paths.
+/** Stub implementation for use in chainstate-only (non-network) contexts.
+ *  In expected chainstate-only usage, sync state is not advanced, so
+ *  IsBlockchainSynced() and IsSynced() remain false, disabling
+ *  network-dependent validation paths.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Stub implementation for use in chainstate-only (non-network) contexts.
* CMasternodeSync constructed with this notifier permanently returns
* IsBlockchainSynced()=false and IsSynced()=false, which correctly disables
* network-dependent validation paths.
*
* Asserts on any call — if sync state is being advanced, a real notifier
* (NodeSyncNotifierImpl) must be used instead. */
/** Stub implementation for use in chainstate-only (non-network) contexts.
* In expected chainstate-only usage, sync state is not advanced, so
* IsBlockchainSynced() and IsSynced() remain false, disabling
* network-dependent validation paths.
*
* Asserts on any call — if sync state is being advanced, a real notifier
* (NodeSyncNotifierImpl) must be used instead. */
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/masternode/sync.h` around lines 35 - 41, The doc comment for the
chainstate-only stub overstates its guarantee by saying CMasternodeSync
"permanently" returns IsBlockchainSynced()=false and IsSynced()=false; change
the wording to a softer statement such as "expected in chainstate-only usage"
and note that SwitchToNextAsset() can advance sync state (e.g., to
MASTERNODE_SYNC_GOVERNANCE) before notifier asserts, so
IsBlockchainSynced()/IsSynced() are not strictly guaranteed to be permanently
false; keep the note that any actual sync advancement should use a real notifier
(NodeSyncNotifierImpl) and that the stub asserts on calls.

class NullNodeSyncNotifier final : public NodeSyncNotifier
{
public:
void SyncReset() override;
void SyncFinished() override;
};

//
// CMasternodeSync : Sync masternode assets in stages
Comment on lines +38 to 50
Copy link
Copy Markdown

@thepastaclaw thepastaclaw May 2, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: NullNodeSyncNotifier is added but never instantiated in this PR

The commit message for 94eeabd says the stub is introduced 'to use it for CChainState', but a grep across src/ finds only the declaration in src/masternode/sync.h:42 and the asserting bodies in src/masternode/sync.cpp:14-15 — no construction site. The class is dead code as merged. Either move the commit into the follow-up that actually consumes it, or note in the PR description that this is staging for a follow-up so reviewers don't expect the integration here. (The assert(false) bodies are safe — src/util/check.h makes building with NDEBUG a hard error in this codebase, so the asserts cannot be elided in release builds.)

source: ['claude']

//
Expand Down
8 changes: 3 additions & 5 deletions src/node/chainstate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
ChainstateManager& chainman,
CGovernanceManager& govman,
CMasternodeMetaMan& mn_metaman,
CMasternodeSync& mn_sync,
CSporkManager& sporkman,
chainlock::Chainlocks& chainlocks,
std::unique_ptr<CChainstateHelper>& chain_helper,
Expand Down Expand Up @@ -83,7 +82,7 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
pblocktree.reset();
pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, block_tree_db_in_memory, fReset));

DashChainstateSetup(chainman, govman, mn_metaman, mn_sync, sporkman, chainlocks, chain_helper,
DashChainstateSetup(chainman, govman, mn_metaman, sporkman, chainlocks, chain_helper,
dmnman, *evodb, llmq_ctx, mempool, data_dir, dash_dbs_in_memory,
/*llmq_dbs_wipe=*/fReset || fReindexChainState, bls_threads, worker_count,
max_recsigs_age, consensus_params);
Expand Down Expand Up @@ -209,7 +208,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
void DashChainstateSetup(ChainstateManager& chainman,
CGovernanceManager& govman,
CMasternodeMetaMan& mn_metaman,
CMasternodeSync& mn_sync,
CSporkManager& sporkman,
chainlock::Chainlocks& chainlocks,
std::unique_ptr<CChainstateHelper>& chain_helper,
Expand All @@ -230,13 +228,13 @@ void DashChainstateSetup(ChainstateManager& chainman,
dmnman = std::make_unique<CDeterministicMNManager>(evodb, mn_metaman);

llmq_ctx.reset();
llmq_ctx = std::make_unique<LLMQContext>(*dmnman, evodb, sporkman, chainman, mn_sync,
llmq_ctx = std::make_unique<LLMQContext>(*dmnman, evodb, sporkman, chainman,
util::DbWrapperParams{.path = data_dir, .memory = llmq_dbs_in_memory, .wipe = llmq_dbs_wipe},
bls_threads, worker_count, max_recsigs_age);
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, chainlocks,
*(llmq_ctx->qsnapman), chainman, consensus_params, chainlocks,
*(llmq_ctx->qman));
}

Expand Down
3 changes: 0 additions & 3 deletions src/node/chainstate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class CEvoDB;
class CGovernanceManager;
class ChainstateManager;
class CMasternodeMetaMan;
class CMasternodeSync;
class CSporkManager;
class CTxMemPool;
struct LLMQContext;
Expand Down Expand Up @@ -80,7 +79,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
ChainstateManager& chainman,
CGovernanceManager& govman,
CMasternodeMetaMan& mn_metaman,
CMasternodeSync& mn_sync,
CSporkManager& sporkman,
chainlock::Chainlocks& chainlocks,
std::unique_ptr<CChainstateHelper>& chain_helper,
Expand Down Expand Up @@ -111,7 +109,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
void DashChainstateSetup(ChainstateManager& chainman,
CGovernanceManager& govman,
CMasternodeMetaMan& mn_metaman,
CMasternodeSync& mn_sync,
CSporkManager& sporkman,
chainlock::Chainlocks& chainlocks,
std::unique_ptr<CChainstateHelper>& chain_helper,
Expand Down
2 changes: 1 addition & 1 deletion src/node/miner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& packa
}

const auto& txid = it->GetTx().GetHash();
if (!m_isman.RejectConflictingBlocks() || !m_isman.IsInstantSendEnabled() || m_isman.IsLocked(txid)) {
if (!m_isman.IsInstantSendEnabled() || m_isman.IsLocked(txid)) {
continue;
}

Expand Down
Loading
Loading