refactor: drop circular dependency over validation <-> masternode/payment

[knst]

converted to draft as it conflicts with 7314 and 7330

Issue being fixed or feature implemented

There's an exception for existing circular dependency in test/lint/lint-circular-dependencies.py:

• "masternode/payments -> validation -> masternode/payments",

What was done?

• introduced new enum MnRewardEra to use in masternode/payment helpers
• removed dependency of masternode/payment on CChainManager (validation.h)
• removed dependency of masternode/payment on global Params() (chainparams.h)

How Has This Been Tested?

Run unit & functional tests; run linter test/lint/lint-circular-dependencies.py

Please note, that 2 other circular dependencies always had existed but had been hidden behind masternode/payment <-> validation:

+    "evo/deterministicmns -> evo/providertx -> validation -> masternode/payments -> evo/deterministicmns",
+    "governance/superblock -> validation -> masternode/payments -> governance/superblock",

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)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• refactor: separate governance from consensus code #7330 - refactor: separate governance from consensus code
• refactor: break circular dependency over net_processing and dkgsessionhandler #7314 - refactor: break circular dependency over net_processing and dkgsessionhandler

Please coordinate with the authors of these PRs to avoid merge conflicts.

[thepastaclaw]

✅ Review complete (commit dc2e5ea)

[coderabbitai]

Walkthrough

This PR introduces a MnRewardEra enum to abstract masternode reward computation and decouples the CMNPaymentsProcessor class from ChainstateManager dependency. The reward era (Classic, CreditPool, or EvoReward) is now computed explicitly at call sites based on deployment activation status and threaded through the entire payments validation pipeline. The era replaces the previous boolean v20 activation flag. GetMasternodePayment is moved from validation.cpp to payments.cpp with updated signature, and a new GetMnRewardEraAfter helper is added to determine era based on deployment states. All payment validation methods now accept CChain& active_chain instead of accessing m_chainman.ActiveChain() internally.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• dashpay/dash#7212: Both PRs modify the masternode payments pipeline wiring around CMNPaymentsProcessor (notably its constructor usage in src/evo/chainhelper.cpp and behavior in src/masternode/payments.{cpp,h}), so the changes are directly related at the CMNPaymentsProcessor interface/initialization level.

Suggested reviewers

• UdjinM6
• PastaPastaPasta
• kwvg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'refactor: drop circular dependency over validation <-> masternode/payment' directly and clearly summarizes the main objective of the PR: removing a circular dependency between these two modules.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description clearly describes the issue being fixed (circular dependency), the changes made (introduced MnRewardEra enum, removed dependencies), and testing approach (unit/functional tests and linter).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/validation.cpp (1)

2571-2581: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pass this chainstate’s chain into payment validation.

ConnectBlock() runs on whichever CChainState is being advanced, but these calls still thread m_chainman.ActiveChain(). IsBlockValueValid() uses that chain for CSuperblock::GetPaymentsLimit(...), and IsBlockPayeeValid() uses it for superblock/governance checks, so background/snapshot validation can evaluate a block against the wrong chain context. Pass m_chain (or a local reference to this chainstate’s chain) instead.

Suggested fix

+    const CChain& chain = m_chain;
-    if (!m_chain_helper->mn_payments->IsBlockValueValid(m_chainman.ActiveChain(), block, pindex->nHeight, blockSubsidy + feeReward, strError, check_superblock)) {
+    if (!m_chain_helper->mn_payments->IsBlockValueValid(chain, block, pindex->nHeight, blockSubsidy + feeReward, strError, check_superblock)) {
         // NOTE: Do not punish, the node might be missing governance data
         LogPrintf("ERROR: ConnectBlock(DASH): %s\n", strError);
         return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-amount");
     }
@@
-    if (!m_chain_helper->mn_payments->IsBlockPayeeValid(m_chainman.ActiveChain(), *block.vtx[0], pindex->pprev, blockSubsidy, feeReward, mn_reward_era, check_superblock)) {
+    if (!m_chain_helper->mn_payments->IsBlockPayeeValid(chain, *block.vtx[0], pindex->pprev, blockSubsidy, feeReward, mn_reward_era, check_superblock)) {
         // NOTE: Do not punish, the node might be missing governance data
         LogPrintf("ERROR: ConnectBlock(DASH): couldn't find masternode or superblock payments\n");
         return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-payee");
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/validation.cpp` around lines 2571 - 2581, ConnectBlock is calling
m_chain_helper->mn_payments->IsBlockValueValid(...) and IsBlockPayeeValid(...)
with m_chainman.ActiveChain(), which can be the wrong chain during
background/snapshot validation; change those calls to pass this CChainState's
chain (use m_chain or a local reference to m_chain) instead of
m_chainman.ActiveChain() so CSuperblock::GetPaymentsLimit and
superblock/governance checks use the correct chain context (update the two call
sites that reference IsBlockValueValid and IsBlockPayeeValid to pass m_chain).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/validation.cpp`:
- Around line 2571-2581: ConnectBlock is calling
m_chain_helper->mn_payments->IsBlockValueValid(...) and IsBlockPayeeValid(...)
with m_chainman.ActiveChain(), which can be the wrong chain during
background/snapshot validation; change those calls to pass this CChainState's
chain (use m_chain or a local reference to m_chain) instead of
m_chainman.ActiveChain() so CSuperblock::GetPaymentsLimit and
superblock/governance checks use the correct chain context (update the two call
sites that reference IsBlockValueValid and IsBlockPayeeValid to pass m_chain).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ec84e219-dc6e-4c80-954b-025e317994e1

📥 Commits

Reviewing files that changed from the base of the PR and between 0f3a2fe and dc2e5ea.

📒 Files selected for processing (10)

• src/evo/chainhelper.cpp
• src/evo/creditpool.cpp
• src/masternode/payments.cpp
• src/masternode/payments.h
• src/node/miner.cpp
• src/rpc/masternode.cpp
• src/test/block_reward_reallocation_tests.cpp
• src/validation.cpp
• src/validation.h
• test/lint/lint-circular-dependencies.py

[thepastaclaw]

Code Review

Clean refactor PR that decouples CMNPaymentsProcessor from ChainstateManager and replaces a boolean fV20Active flag with an MnRewardEra enum computed by callers via a new GetMnRewardEraAfter helper. Both verifier agents converged on no blocking findings, and the two nitpicks raised were low-signal observations dropped during verification.

[github-actions]

This pull request has conflicts, please rebase.