refactor: move ownerships of mnhfman, cpoolman inside ChainState where it belongs to #7104
Conversation
…from detemrinisticmns to specialtxman
|
WalkthroughThis PR refactors the dependency management and initialization of masternode-related managers in the Dash codebase. The primary changes involve removing Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/node/miner.cpp (2)
295-313: Avoid potential null-deref onm_chain_helper.credit_pool_managerin V20 coinbase path.
Ifcredit_pool_manageris ever optional (e.g., certain test setups / partial init),*m_chain_helper.credit_pool_managerwill crash. Consider making the invariant explicit before deref.Proposed fix
if (CalcCbTxBestChainlock(m_clhandler, pindexPrev, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); } else { // not an error LogPrintf("CreateNewBlock() h[%d] CbTx failed to find best CL. Inserting null CL\n", nHeight); } BlockValidationState state; + CHECK_NONFATAL(m_chain_helper.credit_pool_manager); const auto creditPoolDiff = GetCreditPoolDiffForBlock(*m_chain_helper.credit_pool_manager, m_blockman, m_qman, *pblock, pindexPrev, chainparams.GetConsensus(), blockSubsidy, state); if (creditPoolDiff == std::nullopt) { throw std::runtime_error(strprintf("%s: GetCreditPoolDiffForBlock failed: %s", __func__, state.ToString())); }
461-475: Documentcs_mainlocking requirement withAssertLockHeld(::cs_main), but remove redundant null checks.The concern about hidden locking assumptions is valid—
addPackageTxs()calls manager methods (credit_pool_manager->GetCreditPool()andehf_manager->GetSignalsStage()) but only documentsmempool.csas required. Currently invoked undercs_mainfromCreateNewBlock(), but a future refactor could accidentally drop this lock.However, the managers are initialized as
const std::unique_ptrfields inCChainstateHelperand cannot be null, makingCHECK_NONFATALredundant. Add only:void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSelected, int& nDescendantsUpdated, const CBlockIndex* const pindexPrev) { AssertLockHeld(mempool.cs); + AssertLockHeld(::cs_main);This documents the implicit assumption without false null-safety checks.
🤖 Fix all issues with AI agents
In `@src/evo/specialtxman.cpp`:
- Around line 761-785: CheckService currently asserts on
NetInfoStatus::BadInput, ::Duplicate and ::MaxLimit leaving no return in
non-debug builds; change those assert branches to return
state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "<appropriate-error>") with
distinct error strings (e.g. "bad-protx-netinfo-badinput",
"bad-protx-netinfo-duplicate", "bad-protx-netinfo-maxlimit") so every
NetInfoStatus path returns a deterministic validation failure; ensure the final
assert(false) is removed and all enum cases map to a return to avoid undefined
behavior for peer-supplied data in the CheckService template.
In `@test/lint/lint-circular-dependencies.py`:
- Around line 38-46: The EXPECTED_CIRCULAR_DEPENDENCIES allowlist contains six
stale evo/* entries that the lint tool reports as no longer present; remove the
following strings from the list: "evo/chainhelper -> evo/creditpool ->
validation -> evo/chainhelper", "evo/deterministicmns -> evo/providertx ->
validation -> evo/deterministicmns", "evo/deterministicmns -> evo/providertx ->
validation -> txmempool -> evo/deterministicmns", "evo/netinfo -> evo/providertx
-> evo/netinfo", "evo/netinfo -> evo/providertx -> validation -> txmempool ->
evo/netinfo", and "evo/providertx -> validation -> txmempool -> evo/providertx"
from the EXPECTED_CIRCULAR_DEPENDENCIES array so the allowlist matches the lint
output.
🧹 Nitpick comments (3)
src/evo/specialtxman.cpp (1)
857-912: Minor: tighten semantics aroundgsl::not_null+ fix misleading comment.
GetValidatedPayload(..., gsl::not_null<const CBlockIndex*> ...)implies non-null, but some later code paths still treatpindexPrevas optional (e.g.,if (pindexPrev)in callers). Consider removing the redundant null-check branches to reduce confusion.- In
IsVersionChangeValid(), the ExtAddr comment mentions ProRegTx, but this helper is only used by update txs here; consider adjusting the comment to reflect actual call sites.src/test/util/setup_common.cpp (1)
144-147: UseAssert(node.evodb.get())before dereferencing.This wrapper dereferences
*node.evodbwithout the usualAssert(...)guard used for other NodeContext members. If any test harness constructs NodeContext without evodb, this becomes a crash.Proposed fix
- DashChainstateSetup(chainman, *Assert(node.govman.get()), *Assert(node.mn_metaman.get()), *Assert(node.mn_sync.get()), - *Assert(node.sporkman.get()), node.chain_helper, node.dmnman, *node.evodb, + DashChainstateSetup(chainman, *Assert(node.govman.get()), *Assert(node.mn_metaman.get()), *Assert(node.mn_sync.get()), + *Assert(node.sporkman.get()), node.chain_helper, node.dmnman, *Assert(node.evodb.get()), node.llmq_ctx, Assert(node.mempool.get()), node.args->GetDataDirNet(), llmq_dbs_in_memory, llmq_dbs_wipe, llmq::DEFAULT_BLSCHECK_THREADS, llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE, consensus_params);src/evo/specialtxman.h (1)
93-104: NewCheckPro*Txdeclarations are OK; consider scoping to reduce global namespace pollution.These declarations match the implementations and help decouple deterministicmns from validation plumbing. If feasible later, consider scoping them (e.g., a dedicated namespace or making them
static/member utilities) to keep the header’s global surface smaller.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/active/context.cppsrc/active/context.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/evo/deterministicmns.cppsrc/evo/deterministicmns.hsrc/evo/mnhftx.cppsrc/evo/mnhftx.hsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/init.cppsrc/llmq/context.hsrc/llmq/ehf_signals.cppsrc/llmq/ehf_signals.hsrc/node/chainstate.cppsrc/node/chainstate.hsrc/node/context.hsrc/node/miner.cppsrc/node/miner.hsrc/rpc/blockchain.cppsrc/rpc/rawtransaction.cppsrc/test/evo_deterministicmns_tests.cppsrc/test/util/setup_common.cpptest/lint/lint-circular-dependencies.py
💤 Files with no reviewable changes (5)
- src/node/miner.h
- src/node/context.h
- src/llmq/context.h
- src/evo/deterministicmns.cpp
- src/evo/deterministicmns.h
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/llmq/ehf_signals.hsrc/active/context.hsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/llmq/ehf_signals.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/evo/mnhftx.cppsrc/node/chainstate.cppsrc/evo/mnhftx.hsrc/node/chainstate.hsrc/active/context.cpp
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/evo_deterministicmns_tests.cppsrc/test/util/setup_common.cpp
src/node/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Files:
src/node/miner.cppsrc/node/chainstate.cppsrc/node/chainstate.h
src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Files:
src/llmq/ehf_signals.hsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/llmq/ehf_signals.cppsrc/evo/mnhftx.cppsrc/evo/mnhftx.h
src/{masternode,llmq}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
BLS integration must be used for cryptographic foundation of advanced masternode features
Files:
src/llmq/ehf_signals.hsrc/llmq/ehf_signals.cpp
src/llmq/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Files:
src/llmq/ehf_signals.hsrc/llmq/ehf_signals.cpp
src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Files:
src/llmq/ehf_signals.hsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/llmq/ehf_signals.cppsrc/evo/mnhftx.cppsrc/evo/mnhftx.h
src/{masternode,evo}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/evo/mnhftx.cppsrc/evo/mnhftx.h
src/evo/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Files:
src/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/evo/mnhftx.cppsrc/evo/mnhftx.h
src/node/chainstate.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Files:
src/node/chainstate.cppsrc/node/chainstate.h
🧠 Learnings (36)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request `#6543` is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR `#6761`, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (`#6718`), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
📚 Learning: 2025-01-07T18:50:44.838Z
Learnt from: knst
Repo: dashpay/dash PR: 6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions `MigrateDBIfNeeded` and `MigrateDBIfNeeded2` in `src/evo/deterministicmns.cpp` are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/test/util/setup_common.cppsrc/init.cpp
📚 Learning: 2025-11-13T20:02:55.480Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6969
File: src/evo/deterministicmns.h:441-479
Timestamp: 2025-11-13T20:02:55.480Z
Learning: In `src/evo/deterministicmns.h`, the `internalId` field in `CDeterministicMN` and the `mnInternalIdMap` in `CDeterministicMNList` are non-deterministic and used only for internal bookkeeping and efficient lookups. Different nodes can assign different internalIds to the same masternode depending on their sync history. Methods like `IsEqual()` intentionally ignore internalId mappings and only compare consensus-critical deterministic fields (proTxHash, collateral, state, etc.).
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/evo/chainhelper.hsrc/init.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/**/*.{cpp,h} : Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/node/chainstate.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/evodb/**/*.{cpp,h} : Evolution Database (CEvoDb) must handle masternode snapshots, quorum state, governance objects with efficient differential updates for masternode lists
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/evo/mnhftx.cppsrc/node/chainstate.cppsrc/evo/mnhftx.hsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/evo/chainhelper.hsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/node/chainstate.cppsrc/evo/mnhftx.hsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/llmq/ehf_signals.hsrc/evo/specialtxman.cppsrc/llmq/ehf_signals.cppsrc/evo/mnhftx.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/test/evo_deterministicmns_tests.cppsrc/node/miner.cppsrc/llmq/ehf_signals.hsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/node/chainstate.cppsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
src/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/node/chainstate.cppsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/node/miner.cppsrc/test/util/setup_common.cppsrc/node/chainstate.cppsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
src/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/chainhelper.cppsrc/evo/chainhelper.hsrc/test/util/setup_common.cppsrc/init.cppsrc/rpc/rawtransaction.cppsrc/node/chainstate.cppsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing
Applied to files:
src/node/miner.cppsrc/rpc/blockchain.cppsrc/evo/specialtxman.cppsrc/evo/specialtxman.hsrc/llmq/ehf_signals.cpptest/lint/lint-circular-dependencies.py
📚 Learning: 2025-11-04T18:23:28.175Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.
Applied to files:
src/node/miner.cppsrc/rpc/rawtransaction.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Applied to files:
src/llmq/ehf_signals.hsrc/llmq/ehf_signals.cppsrc/evo/mnhftx.cppsrc/evo/mnhftx.hsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/llmq/ehf_signals.hsrc/evo/specialtxman.cppsrc/llmq/ehf_signals.cppsrc/rpc/rawtransaction.cppsrc/evo/mnhftx.hsrc/node/chainstate.h
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Applied to files:
src/llmq/ehf_signals.hsrc/evo/mnhftx.cppsrc/evo/mnhftx.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Applied to files:
src/llmq/ehf_signals.hsrc/active/context.hsrc/test/util/setup_common.cppsrc/init.cppsrc/evo/mnhftx.cppsrc/evo/mnhftx.hsrc/active/context.cpp
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/llmq/ehf_signals.hsrc/evo/mnhftx.h
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/llmq/ehf_signals.hsrc/active/context.hsrc/evo/specialtxman.hsrc/evo/chainhelper.hsrc/evo/mnhftx.hsrc/node/chainstate.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: In src/net_processing.cpp, if ActiveContext (m_active_ctx) is non-null, its members (including cj_server) are guaranteed to be initialized; call sites can safely dereference m_active_ctx->cj_server without an additional null-check.
Applied to files:
src/active/context.hsrc/active/context.cpp
📚 Learning: 2025-10-03T11:20:37.475Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
Applied to files:
src/active/context.hsrc/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.cppsrc/node/chainstate.hsrc/active/context.cpp
📚 Learning: 2025-12-02T12:59:28.247Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7026
File: src/rpc/evo.cpp:1713-1799
Timestamp: 2025-12-02T12:59:28.247Z
Learning: In the Dash codebase, CChain::operator[] has built-in safe bounds checking. Accessing chainman.ActiveChain()[height] is safe even if height exceeds the current chain height, as the operator will handle bounds checking internally. There is no need to manually check chain height before using the [] operator on CChain.
Applied to files:
src/rpc/blockchain.cppsrc/rpc/rawtransaction.cpp
📚 Learning: 2025-06-06T11:53:09.094Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/*is_basic_scheme_active=*/true, /*is_extended_addr=*/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
Applied to files:
src/evo/specialtxman.cppsrc/evo/specialtxman.h
📚 Learning: 2025-07-09T15:02:26.899Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.
Applied to files:
src/evo/specialtxman.cppsrc/evo/specialtxman.h
📚 Learning: 2025-06-16T17:59:55.669Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6665
File: src/evo/deterministicmns.cpp:1284-1287
Timestamp: 2025-06-16T17:59:55.669Z
Learning: In Dash masternode ProRegTx validation, platform ports (platformHTTPPort and platformP2PPort) are mandatory and must be non-zero, while netInfo (ipAndPort) is optional. This means that even if an empty netInfo returns 0 from GetPrimary().GetPort(), it won't cause false positives in port duplication checks since platform ports cannot be 0.
Applied to files:
src/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
src/evo/chainhelper.hsrc/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.cppsrc/node/chainstate.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety
Applied to files:
src/evo/chainhelper.hsrc/init.cppsrc/node/chainstate.h
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (`#6718`), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Applied to files:
src/test/util/setup_common.cpp
📚 Learning: 2025-10-28T18:36:40.263Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6923
File: src/test/util/setup_common.cpp:235-251
Timestamp: 2025-10-28T18:36:40.263Z
Learning: In `src/test/util/setup_common.cpp`, the `CEvoDB` instance in `BasicTestingSetup` is constructed with `.memory = true` flag (memory-only mode), so it does not create file handles on disk. This makes the destructor teardown order safe even if `fs::remove_all(m_path_root)` is called before `m_node.evodb.reset()`.
Applied to files:
src/test/util/setup_common.cppsrc/node/chainstate.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Applied to files:
src/test/util/setup_common.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite
Applied to files:
src/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
src/test/util/setup_common.cppsrc/init.cpp
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/rpc/rawtransaction.cpp
📚 Learning: 2025-05-05T12:45:44.781Z
Learnt from: knst
Repo: dashpay/dash PR: 6658
File: src/evo/creditpool.cpp:177-185
Timestamp: 2025-05-05T12:45:44.781Z
Learning: The GetAncestor() function in CBlockIndex safely handles negative heights by returning nullptr rather than asserting, making it safe to call with potentially negative values.
Applied to files:
src/rpc/rawtransaction.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR `#6692`, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR `#6789` to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/evo/mnhftx.cpp
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Applied to files:
src/node/chainstate.h
🧬 Code graph analysis (10)
src/node/miner.cpp (2)
src/evo/creditpool.cpp (3)
creditPoolDiff(332-332)GetCreditPoolDiffForBlock(325-349)GetCreditPoolDiffForBlock(325-327)src/evo/creditpool.h (1)
pindexPrev(81-109)
src/llmq/ehf_signals.h (1)
src/llmq/ehf_signals.cpp (2)
CEHFSignalsHandler(21-29)CEHFSignalsHandler(31-34)
src/evo/specialtxman.cpp (3)
src/evo/netinfo.cpp (2)
IsNodeOnMainnet(71-71)IsNodeOnMainnet(71-71)src/deploymentstatus.h (3)
DeploymentActiveAfter(14-18)DeploymentActiveAfter(20-24)DeploymentDIP0003Enforced(53-56)src/chainparams.cpp (2)
Params(1345-1348)Params(1345-1345)
src/evo/specialtxman.h (1)
src/evo/specialtxman.cpp (8)
CheckProRegTx(914-1048)CheckProRegTx(914-916)CheckProUpServTx(1050-1127)CheckProUpServTx(1050-1051)CheckProUpRegTx(1129-1201)CheckProUpRegTx(1129-1131)CheckProUpRevTx(1203-1233)CheckProUpRevTx(1203-1204)
src/evo/chainhelper.cpp (2)
src/evo/chainhelper.h (1)
CChainstateHelper(35-71)src/evo/mnhftx.cpp (2)
GetSignalsStage(61-92)GetSignalsStage(61-61)
src/evo/chainhelper.h (3)
src/evo/creditpool.cpp (2)
CCreditPoolManager(241-245)CCreditPoolManager(247-247)src/evo/chainhelper.cpp (4)
CChainstateHelper(17-30)CChainstateHelper(32-32)GetSignalsStage(68-71)GetSignalsStage(68-68)src/evo/mnhftx.cpp (4)
GetSignalsStage(61-92)GetSignalsStage(61-61)CMNHFManager(46-53)CMNHFManager(55-59)
src/llmq/ehf_signals.cpp (1)
src/llmq/ehf_signals.h (1)
CEHFSignalsHandler(21-50)
src/test/util/setup_common.cpp (1)
src/node/chainstate.cpp (2)
DashChainstateSetupClose(241-251)DashChainstateSetupClose(241-244)
src/node/chainstate.cpp (1)
src/test/util/setup_common.cpp (2)
DashChainstateSetup(138-148)DashChainstateSetup(138-142)
src/evo/mnhftx.h (1)
src/evo/mnhftx.cpp (2)
CMNHFManager(46-53)CMNHFManager(55-59)
🪛 Cppcheck (2.19.0)
src/evo/specialtxman.cpp
[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/evo/chainhelper.cpp
[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/llmq/ehf_signals.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (32)
src/test/evo_deterministicmns_tests.cpp (1)
10-16: Looks good: test now includes the new ProTx validation home header.
This aligns with the ProTx validation move toevo/specialtxman.*and helps keep the unit tests compiling against the refactor.src/node/miner.cpp (1)
28-35: Include move looks fine.src/active/context.h (1)
64-71: Constructor signature is consistent across header, definition, and call site—no issues found.Header declaration (lines 64–71), definition in
src/active/context.cpp(lines 26–33), and the construction site insrc/init.cpp(lines 2199–2202) all match exactly. All 21 parameters are passed in the correct order, and the removal ofCMNHFManageris properly reflected everywhere (only a forward declaration remains, no orphaned member references).src/rpc/blockchain.cpp (1)
19-20: No actionable issues. TheGetSignalsStage()call is safe and performant:
- Lock-safe: Annotated with
EXCLUSIVE_LOCKS_REQUIRED(!cs_cache), ensuring proper lock design. Calling undercs_maindoes not conflict with the lock contract.- Performance: Implementation is lightweight (block traversal and consensus checks via
GetForBlock()andGetAncestor()calls), with no file I/O or expensive operations.src/evo/specialtxman.cpp (3)
7-29: New includes look reasonable; watch for avoidable coupling.No issues spotted with the added headers in this file; they appear required for the moved validation code.
787-827: ConfirmMainParams()usage is intended for non-mainnet validation.
CheckPlatformFields()usesMainParams()for some “port must not equal core port” checks even when not on mainnet. That may be intentional, but it’s easy to accidentally bake in mainnet defaults for test/dev networks.Suggested verification:
- confirm which networks these rules are consensus-enforced on (mainnet-only vs all nets)
- confirm
NetInfoInterface::GetPrimary().GetPort()behavior whennetInfois empty and how that interacts with these checks
914-1233: Per-type ProTx validation entry points look cleanly separated.The
CheckPro*Tx()split + shared helpers is a good direction for breaking circular deps and centralizing validation entry points. OnceCheckService()is hardened (above), the rest of these flows look consistent with the existing structure (payload parse → trivial validity → semantic checks → inputsHash → sig).src/init.cpp (2)
403-405: Verify shutdown ordering insideDashChainstateSetupClose()vsCTxMemPool::DisconnectManagers().Given
DashChainstateSetupClose()now resetsllmq_ctxinternally (per snippet), please confirmmempool->DisconnectManagers()doesn’t touch llmq-owned objects afterllmq_ctx.reset(). If it does, reorder to disconnect before resetting llmq.
2199-2202: ActiveContext construction update looks consistent with the refactor intent.Call-site update aligns with removing direct mnhf_manager plumbing from init.
src/test/util/setup_common.cpp (1)
152-154: Wrapper update looks fine; keep the teardown invariants explicit.Given tests often mutate init/teardown sequences, keeping this wrapper minimal is good.
src/evo/mnhftx.h (1)
92-99: Fix typo in TODO comment on line 92: "functionallity" → "functionality"This is a simple spelling error in the TODO block.
Likely an incorrect or invalid review comment.
src/evo/chainhelper.cpp (3)
10-11: LGTM: Required includes added for manager ownership.The includes for
creditpool.handmnhftx.hare necessary sinceCChainstateHelpernow owns and constructs these manager instances directly.
17-30: LGTM: Constructor correctly initializes owned managers.The constructor properly:
- Accepts
CEvoDB& evodbto construct the managers internally- Initializes
ehf_managerandcredit_pool_managerviastd::make_uniquebeforespecial_txwhich depends on them- Passes dereferenced managers to
CSpecialTxProcessorThe initializer list order matches the member declaration order in the header, which is correct C++ practice.
68-71: Remove this suggestion—the method cannot be const.The
GetSignalsStagemethods in bothCMNHFManagerandCChainstateHelperare intentionally non-const. The abstract base class inversionbits.hexplicitly documents: "This member function is not const because it calls non-constGetFromCache()." The implementation callsGetForBlock(), which requires access to non-const cache operations. Making these methods const would violate the virtual interface contract and break the implementation.Likely an incorrect or invalid review comment.
src/evo/chainhelper.h (3)
10-15: LGTM: Forward declarations and includes properly added.The additions support the new public API:
<unordered_map>forGetSignalsStagereturn typeCBlockIndexforward declaration for the method parameterCEvoDBforward declaration for the constructor parameter
45-50: LGTM: Constructor signature simplified by internalizing manager dependencies.The constructor now takes
CEvoDB& evodband constructsehf_manager/credit_pool_managerinternally, reducing the external dependency surface. This aligns with the PR objective of moving ownership intoCChainstateHelper.
64-68: Publicconst unique_ptrmembers expose managers appropriately.The
constqualifier on theunique_ptrmembers prevents reassignment while still allowing read access and method calls on the managed objects. This is a reasonable pattern for exposing owned resources.Minor: Consider whether
GetSignalsStageshould beconst(see related comment in chainhelper.cpp).src/active/context.cpp (1)
26-43: LGTM:CMNHFManagerdependency removed fromActiveContext.The constructor signature and
CEHFSignalsHandlerinitialization correctly remove themnhfmanparameter. This aligns with the refactor moving EHF manager access through the chain helper pathway. TheCEHFSignalsHandlerwill now obtain EHF signals viaChainstateManagerrather than a direct reference.src/node/chainstate.cpp (4)
84-86: LGTM:LoadChainstatecorrectly passesevodbto setup.The call to
DashChainstateSetupnow passes*evodb(dereferenced) instead of separate manager pointers. This reflects the API change where managers are now constructed internally byCChainstateHelper.
194-196: LGTM: Signal DB update accessed through chain helper.
ForceSignalDBUpdateis now correctly accessed viachain_helper->ehf_manager, reflecting the new ownership model whereCMNHFManageris owned byCChainstateHelper.
206-238: LGTM:DashChainstateSetupsignature and implementation simplified.The function now:
- Takes
CEvoDB&by reference (appropriate since caller owns the evodb lifetime)- Constructs
dmnman,llmq_ctx, andchain_helperusing the evodb reference- No longer manages
cpoolmanormnhf_managerseparately since they're owned bychain_helperThe initialization order (
dmnman→llmq_ctx→chain_helper) is correct sincechain_helperdepends on components fromllmq_ctx.
241-251: Destruction order is correct and safe.The current order (chain_helper → llmq_ctx → mempool → dmnman) correctly handles the dependency: chain_helper holds raw references to llmq_ctx-owned components (isman and clhandler), so destroying chain_helper first ensures it no longer accesses these references before their owners are destroyed. The ehf_manager and credit_pool_manager owned by chain_helper are not accessed by llmq_ctx.
src/node/chainstate.h (2)
14-23: LGTM: Forward declarations cleaned up.The removed forward declarations for
CCreditPoolManagerandCMNHFManagercorrectly reflect that these types are no longer part of the public API signatures. They're now internal toCChainstateHelper.
108-128: LGTM: API signatures simplified.The header changes correctly reflect:
DashChainstateSetupnow takesCEvoDB&by reference instead ofunique_ptrDashChainstateSetupCloseno longer requirescpoolmanormnhf_managerparametersThis API simplification aligns with the PR objective of internalizing manager ownership within
CChainstateHelper. As per coding guidelines, chainstate initialization is properly separated in these dedicated files.src/rpc/rawtransaction.cpp (1)
49-56: Include looks fine; just watch dependency creep.
Adding<evo/chainhelper.h>is consistent with the refactor, but it does broaden RPC compile-time dependencies. Given this PR already updatedlint-circular-dependencies.py, this is probably intended.src/llmq/ehf_signals.h (1)
34-37: Ctor signature simplification is consistent with the new ownership model.
This aligns the handler with chainstate-owned EHF manager access and reduces plumbing through callers.src/evo/mnhftx.cpp (3)
204-212: ProcessBlock wiring tom_qmanlooks correct.
This keeps the verification path explicit and avoids pointer/atomic indirection.
253-260: UndoBlock wiring tom_qmanlooks correct.
Same comment as ProcessBlock—cleaner and easier to reason about.
46-53: Good refactor. Storingconst llmq::CQuorumManager&as a member eliminates the need forconnect/disconnecthandlers. The lifetime guarantee is sound:LLMQContextis initialized beforeCChainstateHelper(which ownsCMNHFManager), and destroyed after it, ensuring the referencedCQuorumManageroutlivesCMNHFManagerthroughout its lifetime.src/llmq/ehf_signals.cpp (3)
7-29: Ctor + include updates are consistent with the new access path.
Signature matchessrc/llmq/ehf_signals.hand removes the old direct manager dependency.
84-100: ehf_manager access is safe and consistent across the codebase.The
ehf_manageris aconst std::unique_ptr<CMNHFManager>member initialized in the constructor and cannot be null. The access pattern used inHandleNewRecoveredSigmatches the pattern throughout the codebase (miner.cpp, chainstate.cpp) without null checks, indicating this is intentional design. No preprocessor or cppcheck warnings were identified for this file in the current configuration.
36-49: No blocking issue identified; ehf_manager lifecycle is properly managed.The
ehf_manageris initialized viastd::make_uniquein the ChainHelper constructor and cannot be null. The ChainHelper() accessor includes an assert guard, and the CEHFSignalsHandler properly manages listener registration/unregistration through RAII (constructor registers, destructor unregisters). The code is safe as-is and poses no crashing risk.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| template <typename ProTx> | ||
| static bool CheckService(const ProTx& proTx, TxValidationState& state) | ||
| { | ||
| switch (proTx.netInfo->Validate()) { | ||
| case NetInfoStatus::BadAddress: | ||
| return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr"); | ||
| case NetInfoStatus::BadPort: | ||
| return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-port"); | ||
| case NetInfoStatus::BadType: | ||
| return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-type"); | ||
| case NetInfoStatus::NotRoutable: | ||
| return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-unroutable"); | ||
| case NetInfoStatus::Malformed: | ||
| return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad"); | ||
| case NetInfoStatus::Success: | ||
| return true; | ||
| // Shouldn't be possible during self-checks | ||
| case NetInfoStatus::BadInput: | ||
| case NetInfoStatus::Duplicate: | ||
| case NetInfoStatus::MaxLimit: | ||
| assert(false); | ||
| } // no default case, so the compiler can warn about missing cases | ||
| assert(false); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the CheckService function and understand its context
fd "specialtxman" --type fRepository: dashpay/dash
Length of output: 103
🏁 Script executed:
# Look for the NetInfoStatus enum and Validate() method
rg "enum class NetInfoStatus|enum NetInfoStatus" -A 20 --type cpp --type hRepository: dashpay/dash
Length of output: 804
🏁 Script executed:
# Find where CheckService is called
rg "CheckService" --type cpp --type h -B 3 -A 3Repository: dashpay/dash
Length of output: 1265
🏁 Script executed:
# Examine the netInfo->Validate() implementation
rg "Validate()" src/evo --type cpp --type h -B 2 -A 8 | head -100Repository: dashpay/dash
Length of output: 6173
🏁 Script executed:
# Find the NetInfo class and Validate() method implementation
rg "class NetInfo|class NetInfoEntry" -A 50 src/evo/netinfo.h | head -100Repository: dashpay/dash
Length of output: 2746
🏁 Script executed:
# Search for Validate() implementation in NetInfo
rg "NetInfoEntry::Validate\(\)|NetInfo::Validate\(\)" -A 30 src/evo --type cpp --type hRepository: dashpay/dash
Length of output: 3514
🏁 Script executed:
# Look at the full netinfo.h file to understand the structure
wc -l src/evo/netinfo.hRepository: dashpay/dash
Length of output: 77
🏁 Script executed:
# Read the netinfo.h file
cat -n src/evo/netinfo.h | head -150Repository: dashpay/dash
Length of output: 5331
🏁 Script executed:
# Search for where BadInput and MaxLimit are returned
rg "BadInput|MaxLimit" src/evo/netinfo.cpp -B 3 -A 3Repository: dashpay/dash
Length of output: 3475
🏁 Script executed:
# Get the full ExtNetInfo::Validate() implementation
rg "ExtNetInfo::Validate" -A 50 src/evo/netinfo.cppRepository: dashpay/dash
Length of output: 5031
🏁 Script executed:
# Look for MnNetInfo::Validate and any Add() methods that might return these
rg "MnNetInfo::Add|ExtNetInfo::Add" -A 20 src/evo/netinfo.cpp | head -80Repository: dashpay/dash
Length of output: 1715
🏁 Script executed:
# Verify there's no return statement after the switch in release builds
sed -n '761,785p' src/evo/specialtxman.cpp | tail -10Repository: dashpay/dash
Length of output: 375
🏁 Script executed:
# Confirm MnNetInfo::Validate() also can return these values
rg "class MnNetInfo" -A 100 src/evo/netinfo.h | grep -A 5 "Validate"Repository: dashpay/dash
Length of output: 958
🏁 Script executed:
# Check what types proTx.netInfo can be (MnNetInfo or ExtNetInfo)
rg "netInfo.*std::variant|variant.*netInfo|netInfo.*unique_ptr" src/evo --type h -B 2 -A 2Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
# Find where netInfo is declared in ProTx types
rg "netInfo|NetInfo" src/evo/providertx.h -B 2 -A 2 | head -60Repository: dashpay/dash
Length of output: 1085
🏁 Script executed:
# Check which classes have netInfo and what type it is
rg "class CProRegTx|class CProUpServTx|class CProUpRegTx" -A 30 src/evo/providertx.h | grep -A 30 "CProRegTx"Repository: dashpay/dash
Length of output: 1749
🏁 Script executed:
# Final verification: check if there's any guard that prevents BadInput/Duplicate before CheckService call
rg -B 10 "CheckService\(\*opt_ptx" src/evo/specialtxman.cpp | head -40Repository: dashpay/dash
Length of output: 1124
Handle all NetInfoStatus values gracefully without assertions on untrusted peer data.
CheckService() asserts on BadInput, Duplicate, and MaxLimit enum values, then has no return statement in release builds. Since ExtNetInfo::Validate() can return BadInput (from domain validation failures) and Duplicate (from duplicate address detection), these assertions are reachable with peer-supplied transaction data, causing undefined behavior and potential remote DoS. Replace assertions with proper validation rejections using state.Invalid() for all enum cases to guarantee a return value in all build configurations.
Proposed fix
template <typename ProTx>
static bool CheckService(const ProTx& proTx, TxValidationState& state)
{
switch (proTx.netInfo->Validate()) {
case NetInfoStatus::BadAddress:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr");
case NetInfoStatus::BadPort:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-port");
case NetInfoStatus::BadType:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-type");
case NetInfoStatus::NotRoutable:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-addr-unroutable");
case NetInfoStatus::Malformed:
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-bad");
case NetInfoStatus::Success:
return true;
- // Shouldn't be possible during self-checks
case NetInfoStatus::BadInput:
- case NetInfoStatus::Duplicate:
- case NetInfoStatus::MaxLimit:
- assert(false);
+ return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-input");
+ case NetInfoStatus::Duplicate:
+ return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-dup");
+ case NetInfoStatus::MaxLimit:
+ return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-max");
} // no default case, so the compiler can warn about missing cases
- assert(false);
+ return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-protx-netinfo-unknown");
}🤖 Prompt for AI Agents
In `@src/evo/specialtxman.cpp` around lines 761 - 785, CheckService currently
asserts on NetInfoStatus::BadInput, ::Duplicate and ::MaxLimit leaving no return
in non-debug builds; change those assert branches to return
state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "<appropriate-error>") with
distinct error strings (e.g. "bad-protx-netinfo-badinput",
"bad-protx-netinfo-duplicate", "bad-protx-netinfo-maxlimit") so every
NetInfoStatus path returns a deterministic validation failure; ensure the final
assert(false) is removed and all enum cases map to a return to avoid undefined
behavior for peer-supplied data in the CheckService template.
| "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify", | ||
| "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> consensus/tx_verify", | ||
| "evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> evo/assetlocktx", | ||
| "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", | ||
| "evo/deterministicmns -> validation -> evo/deterministicmns", | ||
| "evo/deterministicmns -> validation -> txmempool -> evo/deterministicmns", | ||
| "evo/chainhelper -> evo/creditpool -> validation -> evo/chainhelper", | ||
| "evo/deterministicmns -> evo/providertx -> validation -> evo/deterministicmns", | ||
| "evo/deterministicmns -> evo/providertx -> validation -> txmempool -> evo/deterministicmns", | ||
| "evo/netinfo -> evo/providertx -> evo/netinfo", | ||
| "evo/netinfo -> evo/providertx -> validation -> txmempool -> evo/netinfo", | ||
| "evo/providertx -> validation -> txmempool -> evo/providertx", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the file structure and see what tool outputs
if [ -f "test/lint/lint-circular-dependencies.py" ]; then
echo "=== File exists, checking head/tail ==="
wc -l test/lint/lint-circular-dependencies.py
echo ""
echo "=== Lines around 38-46 ==="
sed -n '35,50p' test/lint/lint-circular-dependencies.py
fiRepository: dashpay/dash
Length of output: 1653
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to understand the structure - check if EXPECTED_CIRCULAR_DEPENDENCIES is defined
echo "=== Searching for EXPECTED_CIRCULAR_DEPENDENCIES definition ==="
rg "EXPECTED_CIRCULAR_DEPENDENCIES" test/lint/lint-circular-dependencies.py -A 20 -B 5 | head -100Repository: dashpay/dash
Length of output: 2865
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check recent git history to understand what changed in this file
echo "=== Recent changes to lint-circular-dependencies.py ==="
git log --oneline -n 20 -- test/lint/lint-circular-dependencies.py 2>/dev/null || echo "Git not available"Repository: dashpay/dash
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get git diff to see what actually changed in this file
echo "=== Git diff for lint-circular-dependencies.py ==="
git diff HEAD -- test/lint/lint-circular-dependencies.py | head -150Repository: dashpay/dash
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if we can determine the PR context - look at what branch or commits we're on
echo "=== Git status ==="
git status --short | head -20
echo ""
echo "=== Most recent commits ==="
git log --oneline -n 5
echo ""
echo "=== Check if any changes in evo/creditpool or evo/providertx ==="
git diff HEAD --name-only | grep -E "(creditpool|providertx)" || echo "No direct changes to those files in diff"Repository: dashpay/dash
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to run the lint tool to see if it passes/fails
echo "=== Running lint-circular-dependencies.py ==="
python3 test/lint/lint-circular-dependencies.py 2>&1 || echo "Exit code: $?"Repository: dashpay/dash
Length of output: 16586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the actual content of the file currently - specifically count how many entries
echo "=== Counting EXPECTED_CIRCULAR_DEPENDENCIES entries ==="
rg "evo/chainhelper -> evo/creditpool -> validation -> evo/chainhelper" test/lint/lint-circular-dependencies.py
echo ""
echo "=== Check what the full EXPECTED_CIRCULAR_DEPENDENCIES looks like ==="
sed -n '6,120p' test/lint/lint-circular-dependencies.py | tail -50Repository: dashpay/dash
Length of output: 1882
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let's understand the current state - check if there's uncommitted changes or if this is a detached state
echo "=== Checking for any staged/uncommitted changes ==="
git diff --cached --name-only 2>/dev/null || echo "No staged changes"
echo ""
echo "=== Check HEAD ref ==="
git rev-parse HEAD
echo ""
echo "=== Look at context around where the code would be ==="
echo "The tool output showed that ALL expected circular dependencies are now MISSING"
echo "This means either:"
echo "1. The code was refactored to fix all circular dependencies"
echo "2. OR the entries in EXPECTED_CIRCULAR_DEPENDENCIES should all be removed"Repository: dashpay/dash
Length of output: 460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what the actual current repo state looks like - has the refactoring been done?
echo "=== Search for include relationships in evo/creditpool and validation ==="
find . -path ./test -prune -o -name "*.h" -o -name "*.cpp" | head -1
ls -la src/evo/ 2>/dev/null | grep -E "(creditpool|providertx|chainhelper)" || echo "Checking file existence"
echo ""
echo "=== Check if files referenced in the circular deps actually exist ==="
for file in creditpool chainhelper providertx; do
found=$(find src -name "*${file}*" 2>/dev/null | head -1)
if [ -n "$found" ]; then
echo "Found: $found"
else
echo "NOT found: *${file}*"
fi
doneRepository: dashpay/dash
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me check the file size and get the complete EXPECTED_CIRCULAR_DEPENDENCIES
echo "=== Full EXPECTED_CIRCULAR_DEPENDENCIES content ==="
sed -n '6,80p' test/lint/lint-circular-dependencies.pyRepository: dashpay/dash
Length of output: 4656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if the entries mentioned in the review are actually present
echo "=== Verify specific entries from review comment are in the file ==="
rg 'evo/chainhelper -> evo/creditpool|evo/deterministicmns -> evo/providertx|evo/netinfo -> evo/providertx' test/lint/lint-circular-dependencies.pyRepository: dashpay/dash
Length of output: 509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me verify - are these entries actually part of the PR changes or were they already there?
echo "=== Check git log to see when these entries were added ==="
git log --all --oneline --follow -- test/lint/lint-circular-dependencies.py | head -10
echo ""
echo "=== Check if there's a PR being reviewed by looking at recent commits ==="
git log --oneline -n 20Repository: dashpay/dash
Length of output: 871
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let's check what the lint tool would output if we had the dependencies
echo "=== Run the devtools circular-dependencies.py to understand what deps exist ==="
python3 contrib/devtools/circular-dependencies.py src/**/*.{h,cpp} 2>&1 | head -30 || echo "Tool needs multiprocess package or other dependencies"
echo ""
echo "=== Summary: The EXPECTED_CIRCULAR_DEPENDENCIES entries exist in the file but tool says they're not found ==="
echo "This means either:"
echo "1. These circular dependencies have been resolved by code changes (not in EXPECTED_CIRCULAR_DEPENDENCIES file)"
echo "2. The entries in the file are stale and should be removed"
</parameter>Repository: dashpay/dash
Length of output: 844
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the specific commit that updated the lint allowlist
echo "=== Details of the recent lint allowlist update ==="
git show --stat de4d0ea8f3 2>/dev/null | head -50
echo ""
echo "=== Check what that commit changed ==="
git show de4d0ea8f3:test/lint/lint-circular-dependencies.py 2>/dev/null | grep -A 5 "evo/chainhelper\|evo/providertx" | head -20Repository: dashpay/dash
Length of output: 2052
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me check the exact diff to see what changed between the old and current version
echo "=== Full git diff for that commit ==="
git show de4d0ea8f3 -- test/lint/lint-circular-dependencies.py 2>/dev/nullRepository: dashpay/dash
Length of output: 3696
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Now let's check what the previous version had
echo "=== Previous version before the allowlist update commit ==="
git show de4d0ea8f3~1:test/lint/lint-circular-dependencies.py 2>/dev/null | grep -E "evo/chainhelper|evo/creditpool|evo/providertx|evo/deterministic" | head -20Repository: dashpay/dash
Length of output: 751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me check the exact current state compared to the commit we examined
echo "=== Current state in HEAD ==="
git show HEAD:test/lint/lint-circular-dependencies.py 2>/dev/null | grep -E "evo/chainhelper|evo/creditpool|evo/providertx|evo/deterministicmns" | head -15
echo ""
echo "=== What was in de4d0ea8f3 (the allowlist update commit) ==="
git show de4d0ea8f3:test/lint/lint-circular-dependencies.py 2>/dev/null | grep -E "evo/chainhelper|evo/creditpool|evo/providertx|evo/deterministicmns" | head -15Repository: dashpay/dash
Length of output: 1453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check what commits are between de4d0ea8f3 and HEAD
echo "=== Commits between the allowlist update and HEAD ==="
git log --oneline de4d0ea8f3..HEAD
echo ""
echo "=== Check if HEAD is further along than de4d0ea8f3 ==="
git log --oneline --graph de4d0ea8f3..HEAD | head -20Repository: dashpay/dash
Length of output: 313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# The current state has entries that weren't in de4d0ea8f3, so this must be a PR in progress
# Let's see what the uncommitted or staged changes actually are
echo "=== Checking for any untracked or modified files ==="
git status
echo ""
echo "=== Detailed view of changes in test/lint/lint-circular-dependencies.py ==="
if [ -f test/lint/lint-circular-dependencies.py ]; then
echo "File exists and is being reviewed"
echo "Comparing current file with de4d0ea8f3 version:"
git diff de4d0ea8f3 test/lint/lint-circular-dependencies.py
fiRepository: dashpay/dash
Length of output: 1733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Perfect! Now I can see the actual PR changes. Let me verify the semantics
# The changes are replacing one dependency path with another
# Let me check if evo/creditpool, evo/providertx are real modules
echo "=== Check if the new modules in the updated paths exist ==="
ls -la src/evo/ 2>/dev/null | grep -E "(creditpool|providertx|chainhelper|specialtxman)"
echo ""
echo "=== Check if specialtxman is being removed from the dependency chain ==="
rg "specialtxman" src/evo/ --type cpp --type h -c | head -5Repository: dashpay/dash
Length of output: 787
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Now let me understand the refactoring - check what imports changed in evo/chainhelper
echo "=== Check evo/chainhelper includes ==="
grep -E "^#include|^#pragma" src/evo/chainhelper.h
echo ""
echo "=== Check what creditpool is and its relationship ==="
grep -E "^#include|^#pragma" src/evo/creditpool.h | head -10Repository: dashpay/dash
Length of output: 476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me understand the context better - is this a refactoring PR that resolves circular deps?
# Check the PR description or commit messages for context
echo "=== Check the commit that made these changes ==="
git log --all --pretty=format:"%H %s" | grep -i "creditpool\|providertx\|circular" | head -10
echo ""
echo "=== Check if there are multiple branches/refs to understand PR context ==="
git for-each-ref --format='%(refname:short) %(objectname:short) %(subject)' | head -20Repository: dashpay/dash
Length of output: 1181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Let me check the most recent actual commit that touches circular dependencies
echo "=== Looking at recent circular dependency changes ==="
git show 2dc0a25a4a:test/lint/lint-circular-dependencies.py 2>/dev/null | grep -E "evo/chainhelper|evo/creditpool|evo/providertx|evo/deterministicmns" | head -10Repository: dashpay/dash
Length of output: 745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Now I understand the full picture. Let me verify the final assessment:
# The PR is refactoring the dependency paths, BUT when we run the tool with the new entries,
# it still reports them all as "no longer present"
echo "=== Summary of findings ==="
echo "1. The PR changes these dependency paths:"
echo " OLD: evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper"
echo " NEW: evo/chainhelper -> evo/creditpool -> validation -> evo/chainhelper"
echo ""
echo "2. Tool output when run with current code:"
echo " Reports ALL expected entries (old AND new) as 'no longer present'"
echo ""
echo "3. This means:"
echo " - Even though paths were updated, the circular deps they describe don't actually exist"
echo " - All six entries should be REMOVED per tool output"
echo ""
# Verify this is a PR being reviewed (detached HEAD at FETCH_HEAD is typical for PRs)
echo "4. This is a PR in review (detached HEAD at FETCH_HEAD)"Repository: dashpay/dash
Length of output: 608
Remove all six evo/ entries—the lint tool reports them as no longer present and the allowlist should not contain them.*
The PR updates circular dependency paths (replacing specialtxman with creditpool, and validation with providertx), but when the lint tool executes against this code, it reports all six entries listed as "no longer present" and explicitly requests their removal from EXPECTED_CIRCULAR_DEPENDENCIES. Approving these entries contradicts the tool output and will cause CI to fail.
Remove these lines entirely:
- Line 41:
"evo/chainhelper -> evo/creditpool -> validation -> evo/chainhelper" - Line 42:
"evo/deterministicmns -> evo/providertx -> validation -> evo/deterministicmns" - Line 43:
"evo/deterministicmns -> evo/providertx -> validation -> txmempool -> evo/deterministicmns" - Lines 34-36 (unchanged but reported as missing):
"evo/netinfo -> evo/providertx"entries and"evo/providertx -> validation -> txmempool -> evo/providertx"
🤖 Prompt for AI Agents
In `@test/lint/lint-circular-dependencies.py` around lines 38 - 46, The
EXPECTED_CIRCULAR_DEPENDENCIES allowlist contains six stale evo/* entries that
the lint tool reports as no longer present; remove the following strings from
the list: "evo/chainhelper -> evo/creditpool -> validation -> evo/chainhelper",
"evo/deterministicmns -> evo/providertx -> validation -> evo/deterministicmns",
"evo/deterministicmns -> evo/providertx -> validation -> txmempool ->
evo/deterministicmns", "evo/netinfo -> evo/providertx -> evo/netinfo",
"evo/netinfo -> evo/providertx -> validation -> txmempool -> evo/netinfo", and
"evo/providertx -> validation -> txmempool -> evo/providertx" from the
EXPECTED_CIRCULAR_DEPENDENCIES array so the allowlist matches the lint output.
1 participant
Issue being fixed or feature implemented
LoadChainState has too many arguments that constructed outside, even if ownership and life time is controlled by chain state.
What was done?
CMNHFManager), Credit Pool Manager (CCreditPoolManagerCMNHFManager::ConnectManagersm_qmaninCMNHFManagerHow Has This Been Tested?
Run unit and functional tests, run lint-circular-dependencies.py
Breaking Changes
N/A
Checklist: