-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: extract CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation)
#7065
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.
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
base: develop
Are you sure you want to change the base?
Conversation
|
CActiveMasternodeManager from LLMQContext (3/3, DKG session isolation, ActiveContext consolidation)CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation)
|
This pull request has conflicts, please rebase. |
…ontext` (2/n, `CQuorumManager` handler separation) d41e5bd chore: apply review suggestions (Kittywhiskers Van Gogh) 8a3ad09 refactor: abstract away parent implementation from handler (Kittywhiskers Van Gogh) 718ee50 refactor: streamline quorum cache logic (Kittywhiskers Van Gogh) 1360d9d refactor: drop unnecessary `CConnman` argument in handlers (Kittywhiskers Van Gogh) 2a57a44 refactor: move quorum manager to separate source file (Kittywhiskers Van Gogh) 50a5f26 refactor: pull handler initialization out of `CQuorumManager` (Kittywhiskers Van Gogh) 61da16d refactor: separate observer/common routines into dedicated class (Kittywhiskers Van Gogh) e8b771e move-only: move observer/common routines to `llmq/observer` (Kittywhiskers Van Gogh) 4078de0 refactor: disentangle watch-only and masternode mode conn checking (Kittywhiskers Van Gogh) a2d909b refactor: pull deletable quorums routine to dedicated function (Kittywhiskers Van Gogh) eaee1a8 refactor: disentangle watch-only and masternode mode thread triggers (Kittywhiskers Van Gogh) e6d8e69 refactor: pull data recovery lambda to a dedicated function (Kittywhiskers Van Gogh) 1fe85be refactor: separate observer/participant logic into dedicated class (Kittywhiskers Van Gogh) ba24b4e move-only: move observer/participant logic to `active/quorums.cpp` (Kittywhiskers Van Gogh) de0ce4e refactor: extract masternode-mode specific `SetSecretKeyShare()` (Kittywhiskers Van Gogh) 201ccfc refactor: extract `ENCRYPTED_CONTRIBUTIONS` processing case (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #7062 * Dependency for #7065 * To enforce the split between masternode mode (which can participate in quorums and seek quorum data) and watch-only mode (which can **only** seek quorum data), threading logic is split between `StartDataRecoveryThread()` and `StartVvecSyncThread()`, they both call the same underlying `DataRecoveryThread()` but one has access to masternode-specific parameters and the other does not. This becomes relevant as the entities are split out and the access to specific parameters are enforced by the relevant class members outright not existing. * It is recommended to use `TryStartVvecSyncThread()` as it will not start threads if no data is actually needed, calling `StartVvecSyncThread()` directly bypasses this check. * `CQuorumManager` exposes both `IsWatching()` and `IsMasternode()` to allow P2P code and interfaces to query the node's state (this is most relevant in `PeerManager` which can trivially detect masternode mode but not watch-only status). * Watch-only nodes cannot be masternodes but masternodes can **also** be watch-only (the term "observer" has been used in the codebase where possible as watch-only becomes a bit of a misnomer in this case but is the established term) nodes. * The `CQuorumManager` cache warmer was one of the tasks allocated to the common worker pool, as the rest of the activities are managed by the observer context, rather than keeping the worker pool in the quorum manager and then exposing it through the interface for the sake of one task, the worker pool has been moved to the observer context and we have a regular thread for the quorum manager instead. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK d41e5bd Tree-SHA512: 9d4f0c67149dcec10d50a74dafe18b165590b8977203f82f3104645bf5a6e1e109fa901c1aaebf543ebc215aa910bd47b8b88dc993a66363655fb0c6ea57ed73
Required for upcoming commits.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
WalkthroughThis PR moves masternode-active code from src/masternode/active/ into src/active/, refactors ActiveContext to inherit from CValidationInterface and manage masternode/LLMQ subsystems, introduces ActiveDKGSession and ActiveDKGSessionHandler for DKG lifecycle, reworks CDKGSession/CDKGSessionHandler and CDKGSessionManager APIs (handler factory), reorders several constructor parameter lists (LLMQContext, LLMQ components), changes CActiveMasternodeManager construction/fields, replaces node.mn_activeman with node.active_ctx throughout, and updates many includes and Makefile target exports. Sequence Diagram(s)sequenceDiagram
%% Styling
participant Node
participant ActiveContext
participant DKGMgr as CDKGSessionManager
participant DKGHandler as ActiveDKGSessionHandler
participant PeerMgr as PeerManager
participant Chain as ChainstateManager
Note over Node,ActiveContext: Node constructs ActiveContext during startup
Node->>ActiveContext: new ActiveContext(...deps...)
ActiveContext->>DKGMgr: InitializeHandlers(factory)
DKGMgr-->>DKGHandler: factory(llmq_params, quorum_idx) -> create handler
ActiveContext->>PeerMgr: SetCJServer / register managers
Note over ActiveContext,DKGHandler: DKGHandler runs per-quorum loop (phases)
loop DKG phases
DKGHandler->>PeerMgr: RelayInvToParticipants(invs)
PeerMgr->>DKGHandler: Receive messages / CConnman relay
DKGHandler->>DKGMgr: Update contributions / verify / commit
DKGMgr->>Chain: Query block/proof state
DKGMgr->>ActiveContext: NotifyRecoveredSig / UpdatedBlockTip
ActiveContext->>DKGHandler: Forward block-tip updates
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/llmq/dkgsessionmgr.h (1)
59-72: Consider consolidating multipleprivate:sections.The class has three separate
private:sections (lines 59, 62, 69). While functionally correct, consolidating them would improve readability.🔎 Suggested consolidation
private: static constexpr int64_t MAX_CONTRIBUTION_CACHE_TIME = 60 * 1000; -private: CDeterministicMNManager& m_dmnman; CQuorumSnapshotManager& m_qsnapman; const ChainstateManager& m_chainman; const CSporkManager& m_sporkman; const bool m_quorums_watch{false}; -private: std::unique_ptr<CDBWrapper> db{nullptr}; SessionHandlerMap dkgSessionHandlers;src/net_processing.cpp (1)
1595-1613: Usingm_active_ctx != nullptras theis_masternodesignal is coherent across request schedulingDeriving
is_masternodefromm_active_ctx != nullptrin bothProcessMessageandSendMessages, and threading that intoCalculateObjectGetDataTime(inRequestObjectand in the requeue path) keeps the previous behaviour of “masternode nodes skip inbound TX delay” while now keying off the newActiveContextas the single source of truth. The requeue logic still correctly biases towards outbound peers via!state->fPreferredDownload, and the is_masternode flag is only used to disable the inbound delay in theinv.IsMsgTx()case, so non-TX objects and non-masternode nodes remain unaffected. As a minor readability tweak, you could factor a tiny helper likebool IsMasternode() const { return static_cast<bool>(m_active_ctx); }to avoid repeating the comparison in multiple sites, but that’s purely optional.Also applies to: 1619-1637, 3715-3719, 5979-5982, 6528-6563
src/llmq/dkgsession.cpp (1)
68-85: CDKGSession ctor/dtor wiring and MaybeDecrypt hook look soundThe new constructor parameter ordering (worker, dmnman, debug, manager, qsnapman, chainman, base index, params) and reference members in
CDKGSessionare coherent with the call sites, and the defaulted virtual destructor is appropriate for subclassing.In
ReceiveMessage(const CDKGContribution& qc), theAreWeMember()guard before callingMaybeDecrypt(*qc.contributions, *myIdx, …)matches the Init logic that only setsmyProTxHashandmyIdxwhen our proTxHash is present in the quorum member list, so the optional dereference is safe given the documented “one CDKGSession per DKG” lifecycle. If you want to make that invariant explicit for future refactors, a debug-only assert thatmyIdx.has_value()in theAreWeMember()path would be a low-risk clarity improvement.Also applies to: 86-88, 252-261
src/active/dkgsession.cpp (1)
22-37: ActiveDKGSession wiring and phase logic mirror the legacy DKG flow correctlyThe new
ActiveDKGSessioncleanly specializesCDKGSessionfor the active-masternode case:
- The ctor threads through all required managers (BLSS worker, DKG managers, snapshot manager, chainman, sporkman, ActiveMasternodeManager) and derives
m_use_legacy_blsonce fromDeploymentActiveAfter(m_quorum_base_block_index, …, DEPLOYMENT_V19), which is then used consistently for all BLS signing/verification.- Phase 1–4 implementations (
Contribute,SendContributions,VerifyPendingContributions,VerifyAndComplain,SendComplaint,VerifyAndJustify,SendJustification,VerifyAndCommit,SendCommitment) closely follow the existing logic from the llmq DKG path, but now route encryption/decryption and operator signing viam_mn_activeman, which fits the new ActiveContext ownership model.VerifyPendingContributions’s use ofcs_pending,pendingContributionVerifications, and the variousreceived*arrays is internally consistent and writes encrypted/verified contributions throughdkgManagerin the same places as before.FinalizeCommitmentsandFinalizeSingleCommitmentconstructCFinalCommitmentobjects using the same quorum membership, vvec, and signature aggregation rules, with verification performed against{m_dmnman, m_qsnapman, m_chainman, m_quorum_base_block_index}as in the legacy code.- The
MaybeDecryptoverride correctly delegates toCActiveMasternodeManager::Decrypt, keeping CDKGSession agnostic of how operator keys are managed.Given how many components are involved, you might consider a debug assert in the ctor that
m_quorum_base_block_index != nullptrfor active sessions, to document the expectation that ActiveDKGSession is only constructed once the base block index is known, but behavior as written is consistent with the existing DKG design.Also applies to: 38-112, 118-170, 213-255, 389-544, 546-653, 655-711, 713-717
src/active/dkgsessionhandler.h (1)
8-15: ActiveDKGSessionHandler API and state are well-structured for phase controlThe new
ActiveDKGSessionHandlercleanly extendsCDKGSessionHandlerwith everything needed to drive an active-masternode DKG:
- It holds references to the same core components ActiveDKGSession depends on (BLS worker, deterministic MN manager, meta manager, DKG/debug managers, quorum block/snapshot managers, ActiveMasternodeManager, chainman, sporkman) plus flags for
quorums_watch.- The thread-control members (
stopRequested,currentHeight,phaseHandlerThread) andcs_phase_qhash-guardedphase/quorumHashgive a clear separation between external notifications (UpdatedBlockTip) and the internal phase loop (PhaseHandlerThread/HandleDKGRound).- Overridden
GetPhaseusesWITH_LOCK(cs_phase_qhash, …)and is annotated withEXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash), matching the intended locking model.StartPhaseFunc/WhileWaitFuncprovide a flexible callback mechanism for the variousWait*/Handle*helpers.If
<functional>is not already pulled in by the includedllmq/dkgsessionhandler.h, it would be slightly safer to include it directly here to avoid relying on transitive includes, but functionally the design looks solid.Also applies to: 16-28, 30-38, 40-49, 51-60, 62-83
src/active/dkgsessionhandler.cpp (3)
41-46: Consider avoiding the reset-then-recreate pattern in constructor.The constructor resets
curSession(initialized by the parent) and creates a newActiveDKGSession. While functionally correct, this creates and immediately destroys aCDKGSessionobject.🔎 Alternative: Defer parent session creation
An alternative design would have the parent class accept the session via parameter or use a factory pattern. However, given this is a refactoring PR focused on extraction, this optimization can be deferred. The current approach works correctly and the allocation overhead is negligible (once per handler, not per DKG round).
52-52: Minor: Commented-out assertion.Line 52 has a commented-out
//AssertLockNotHeld(cs_main);. If this assertion is no longer needed, consider removing the comment entirely. If it's temporarily disabled for debugging, consider adding a TODO explaining why.
517-531: Thread loop handles abort gracefully; consider broader exception handling for resilience.The
PhaseHandlerThreadcorrectly catchesAbortPhaseExceptionfor normal flow control (phase changes, shutdown requests) and continues looping. This is the intended behavior.However, any other exception (e.g.,
std::runtime_errorfrom an unexpected failure) would silently terminate the thread. While the codebase uses broader exception handling patterns elsewhere (e.g.,catch(std::exception&)in src/llmq/signing.cpp andcatch(...)in serialization code), this specific thread handler does not. Consider whether adding a catch-all with logging would improve resilience, or if letting unexpected exceptions terminate the thread is intentional to surface bugs early.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/Makefile.amsrc/active/context.cppsrc/active/context.hsrc/active/dkgsession.cppsrc/active/dkgsession.hsrc/active/dkgsessionhandler.cppsrc/active/dkgsessionhandler.hsrc/active/masternode.cppsrc/active/masternode.hsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/init.cppsrc/llmq/context.cppsrc/llmq/context.hsrc/llmq/dkgsession.cppsrc/llmq/dkgsession.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/observer/context.cppsrc/llmq/signing_shares.cppsrc/masternode/active/context.cppsrc/masternode/active/notificationinterface.cppsrc/masternode/active/notificationinterface.hsrc/net_processing.cppsrc/net_processing.hsrc/node/chainstate.cppsrc/node/chainstate.hsrc/node/context.cppsrc/node/context.hsrc/node/interfaces.cppsrc/rpc/coinjoin.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/coinjoin_queue_tests.cppsrc/test/denialofservice_tests.cppsrc/test/net_peer_connection_tests.cppsrc/test/util/setup_common.cppsrc/test/util/setup_common.htest/lint/lint-circular-dependencies.pytest/util/data/non-backported.txt
💤 Files with no reviewable changes (8)
- test/util/data/non-backported.txt
- src/node/chainstate.h
- src/net_processing.h
- src/test/util/setup_common.h
- src/masternode/active/context.cpp
- src/masternode/active/notificationinterface.h
- src/masternode/active/notificationinterface.cpp
- src/node/context.h
🧰 Additional context used
📓 Path-based instructions (12)
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/node/interfaces.cppsrc/test/net_peer_connection_tests.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/rpc/coinjoin.cppsrc/test/denialofservice_tests.cppsrc/active/dkgsession.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/active/dkgsessionhandler.hsrc/node/chainstate.cppsrc/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/active/dkgsession.hsrc/active/masternode.cppsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/test/coinjoin_queue_tests.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/active/masternode.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.hsrc/active/dkgsessionhandler.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/interfaces.cppsrc/node/context.cppsrc/node/chainstate.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/net_peer_connection_tests.cppsrc/test/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/test/coinjoin_queue_tests.cpp
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/mnauth.cpp
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/evo/mnauth.cppsrc/governance/signing.cppsrc/llmq/signing_shares.cppsrc/coinjoin/server.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.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/mnauth.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/evo/mnauth.cppsrc/governance/signing.cppsrc/llmq/signing_shares.cppsrc/coinjoin/server.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.h
src/governance/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Files:
src/governance/signing.cpp
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/signing_shares.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.h
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/signing_shares.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.h
src/coinjoin/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Files:
src/coinjoin/server.cpp
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.cpp
🧠 Learnings (39)
📓 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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
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: 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/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
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.
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.
📚 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/interfaces.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/masternode.cppsrc/active/masternode.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/node/interfaces.cppsrc/net_processing.cppsrc/rpc/coinjoin.cppsrc/init.cppsrc/active/context.cppsrc/active/context.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/net_peer_connection_tests.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/active/masternode.cppsrc/test/coinjoin_queue_tests.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}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/test/net_peer_connection_tests.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/test/denialofservice_tests.cppsrc/active/dkgsession.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/dkgsessionhandler.hsrc/active/dkgsession.hsrc/active/masternode.cppsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/test/coinjoin_queue_tests.cppsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/active/masternode.hsrc/llmq/dkgsessionmgr.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/test/net_peer_connection_tests.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/rpc/coinjoin.cppsrc/active/dkgsession.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/dkgsession.hsrc/active/masternode.cppsrc/llmq/dkgsession.cppsrc/test/coinjoin_queue_tests.cppsrc/llmq/dkgsession.hsrc/active/context.hsrc/active/masternode.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/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/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/masternode.cppsrc/active/context.hsrc/active/masternode.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/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/Makefile.amsrc/active/masternode.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/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/node/context.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cppsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/node/chainstate.cppsrc/active/masternode.cpptest/lint/lint-circular-dependencies.pysrc/llmq/dkgsession.hsrc/active/context.hsrc/active/masternode.h
📚 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/evo/mnauth.cppsrc/governance/signing.cppsrc/rpc/governance.cppsrc/active/quorums.cppsrc/active/masternode.cppsrc/active/masternode.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/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/evo/mnauth.cppsrc/coinjoin/server.cppsrc/rpc/quorums.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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Applied to files:
src/governance/signing.cppsrc/rpc/governance.cppsrc/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/{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/rpc/governance.cppsrc/net_processing.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/test/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/context.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/rpc/governance.cppsrc/node/context.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/node/chainstate.cppsrc/llmq/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/rpc/governance.cppsrc/node/context.cppsrc/rpc/masternode.cppsrc/rpc/coinjoin.cppsrc/test/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.cppsrc/active/context.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/active/context.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/net_processing.cppsrc/llmq/signing_shares.cppsrc/active/quorums.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/active/dkgsessionhandler.hsrc/node/chainstate.cppsrc/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/llmq/dkgsessionmgr.cppsrc/active/dkgsessionhandler.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/{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/net_processing.cppsrc/node/context.cppsrc/rpc/masternode.cppsrc/coinjoin/server.cppsrc/rpc/quorums.cppsrc/rpc/coinjoin.cppsrc/test/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/context.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/net_processing.cppsrc/llmq/signing_shares.cppsrc/active/quorums.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/active/dkgsessionhandler.hsrc/node/chainstate.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/llmq/dkgsessionmgr.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/net_processing.cppsrc/rpc/masternode.cppsrc/rpc/coinjoin.cppsrc/test/util/setup_common.cppsrc/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/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/net_processing.cppsrc/llmq/signing_shares.cppsrc/active/quorums.cppsrc/rpc/quorums.cppsrc/active/dkgsession.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/active/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/active/dkgsession.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.hsrc/active/dkgsessionhandler.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/net_processing.cppsrc/llmq/signing_shares.cppsrc/active/quorums.cppsrc/rpc/quorums.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/Makefile.amsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.h
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Applied to files:
src/net_processing.cppsrc/llmq/signing_shares.cppsrc/rpc/quorums.cppsrc/active/context.hsrc/active/dkgsessionhandler.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/net_processing.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/node/context.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/active/context.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/llmq/signing_shares.cppsrc/coinjoin/server.cppsrc/node/chainstate.cpp
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.
Applied to files:
src/rpc/masternode.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/masternode.cpp
📚 Learning: 2025-11-04T18:24:27.241Z
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.
Applied to files:
src/rpc/quorums.cpp
📚 Learning: 2025-08-19T15:08:00.835Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/commitment.cpp:54-60
Timestamp: 2025-08-19T15:08:00.835Z
Learning: In Dash Core, llmq_params.size == 1 is used only for regtest environments, not on public networks, which significantly reduces the risk profile of code paths specific to single-member quorums.
Applied to files:
src/rpc/quorums.cpp
📚 Learning: 2025-09-02T07:34:28.226Z
Learnt from: knst
Repo: dashpay/dash PR: 6834
File: test/functional/wallet_mnemonicbits.py:50-51
Timestamp: 2025-09-02T07:34:28.226Z
Learning: CJ (CoinJoin) descriptors with derivation path "9'/1" are intentionally inactive in descriptor wallets, while regular internal/external descriptors with different derivation paths remain active.
Applied to files:
src/rpc/coinjoin.cppsrc/test/util/setup_common.cpp
📚 Learning: 2025-07-23T09:30:34.631Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.
Applied to files:
src/Makefile.am
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
src/Makefile.am
📚 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/active/dkgsessionhandler.hsrc/llmq/context.hsrc/active/dkgsession.hsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsession.hsrc/active/context.hsrc/active/masternode.hsrc/llmq/dkgsessionmgr.h
📚 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/node/chainstate.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsession.hsrc/llmq/observer/context.cppsrc/active/context.hsrc/llmq/dkgsessionmgr.cpp
📚 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/node/chainstate.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/observer/context.cppsrc/active/context.h
📚 Learning: 2025-05-10T00:54:15.597Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6629
File: src/evo/deterministicmns.cpp:473-485
Timestamp: 2025-05-10T00:54:15.597Z
Learning: In the current implementation of `NetInfoEntry`, only `CService` is a valid type and `std::monostate` represents an invalid state. Entries that don't provide a valid `CService` through `GetAddrPort()` should be rejected with an exception, not silently skipped.
Applied to files:
src/active/masternode.cpp
📚 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/active/masternode.cpp
📚 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/active/masternode.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/coinjoin_queue_tests.cpp
🧬 Code graph analysis (18)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/net_processing.cpp (1)
src/evo/mnauth.cpp (2)
PushMNAUTH(19-49)PushMNAUTH(19-19)
src/rpc/masternode.cpp (1)
src/interfaces/node.h (2)
node(48-50)node(481-481)
src/rpc/quorums.cpp (1)
src/interfaces/chain.h (1)
node(40-42)
src/test/denialofservice_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/active/dkgsession.cpp (1)
src/llmq/dkgsession.cpp (4)
ShouldSimulateError(52-59)ShouldSimulateError(52-52)MarkBadMember(686-697)MarkBadMember(686-686)
src/node/chainstate.cpp (1)
src/test/util/setup_common.cpp (2)
DashChainstateSetup(140-150)DashChainstateSetup(140-144)
src/active/context.cpp (5)
src/active/context.h (1)
m_cj_server(103-103)src/llmq/signing_shares.cpp (4)
Start(199-218)Start(199-199)Stop(220-238)Stop(220-220)src/active/dkgsessionhandler.cpp (2)
UpdatedBlockTip(50-75)UpdatedBlockTip(50-50)src/active/masternode.cpp (2)
UpdatedBlockTip(175-215)UpdatedBlockTip(175-175)src/governance/signing.cpp (2)
UpdatedBlockTip(287-296)UpdatedBlockTip(287-287)
src/active/dkgsession.h (1)
src/llmq/dkgsession.h (4)
llmq(30-39)llmq(41-67)CDKGSession(276-409)CFinalCommitment(383-383)
src/active/masternode.cpp (1)
src/active/masternode.h (1)
cs(40-75)
src/llmq/context.cpp (1)
src/node/interfaces.cpp (3)
chainman(754-756)chainman(1239-1241)chainman(1309-1312)
src/llmq/dkgsession.cpp (2)
src/llmq/dkgsession.h (1)
CDKGSession(276-409)src/active/dkgsession.cpp (4)
MaybeDecrypt(713-717)MaybeDecrypt(713-714)qc(263-263)qc(442-442)
src/test/coinjoin_queue_tests.cpp (1)
src/test/util/setup_common.cpp (2)
TestingSetup(312-395)TestingSetup(397-422)
src/llmq/dkgsessionhandler.h (1)
src/llmq/dkgsessionhandler.cpp (2)
CDKGSessionHandler(22-39)CDKGSessionHandler(41-41)
src/llmq/dkgsession.h (3)
src/llmq/dkgsessionhandler.h (4)
llmq(34-163)CDKGPendingMessages(64-158)void(160-160)void(161-161)src/llmq/dkgsessionmgr.cpp (2)
CDKGSessionManager(32-42)CDKGSessionManager(44-44)src/llmq/dkgsessionhandler.cpp (2)
CDKGSessionHandler(22-39)CDKGSessionHandler(41-41)
src/active/masternode.h (1)
src/active/masternode.cpp (11)
CActiveMasternodeManager(52-63)CActiveMasternodeManager(65-65)nodiscard(266-271)nodiscard(277-281)nodiscard(283-289)nodiscard(293-297)GetPubKey(293-293)InitInternal(112-173)InitInternal(112-112)GetLocalAddress(217-255)GetLocalAddress(217-217)
src/llmq/dkgsessionmgr.cpp (1)
src/dbwrapper.h (1)
Commit(677-725)
src/llmq/dkgsessionmgr.h (1)
src/llmq/dkgsessionmgr.cpp (2)
CDKGSessionManager(32-42)CDKGSessionManager(44-44)
🪛 Cppcheck (2.19.0)
src/evo/mnauth.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/rpc/governance.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/llmq/signing_shares.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/rpc/masternode.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/coinjoin/server.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/rpc/quorums.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/active/dkgsession.cpp
[error] 10-10: 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/active/context.cpp
[error] 10-10: 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/llmq/dkgsessionhandler.cpp
[error] 10-10: 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/test/coinjoin_queue_tests.cpp
[error] 15-15: There is an unknown macro here somewhere. Configuration is required. If BOOST_FIXTURE_TEST_SUITE is a macro then please configure it.
(unknownMacro)
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/llmq/observer/context.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/active/dkgsessionhandler.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
|
|
||
| //TODO name struct instead of std::pair | ||
| std::map<std::pair<Consensus::LLMQType, int>, CDKGSessionHandler> dkgSessionHandlers; | ||
| private: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove this private:, it's already above twice: for constant and for members
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repeated usage of private: is to segment constants, ctor variables (the references), state variables (everything else) and functions.
| auto operator<=>(const SessionHandlerKey&) const = default; | ||
| }; | ||
|
|
||
| using SessionHandlerMap = std::map<SessionHandlerKey, std::unique_ptr<CDKGSessionHandler>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider replacing multiple Assert around usages of SessionHandlerMap to just gsl::not_null<std::unique_ptr<CDKGSessionHandler>> here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wouldn't be possible as std::unique_ptr is movable and therefore could violate the gsl::not_null guarantee. Plus std::map's [] operator creates an entry if none exists (source), which would again be at odds with the guarantees.
We also need to move CCoinJoinServer creation back into ActiveContext
Also: - Drop stale TODO, remove headers in code that no longer refer to masternode capabilities
Now that `ActiveNotificationInterface` exclusively calls `ActiveContext` we can now merge them together. Also: - Drop `LLMQContext` from `ActiveContext`, only import what we use
Also: - Headers cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @src/active/context.cpp:
- Around line 1-61: The ActiveContext constructor formatting in this file fails
clang-format CI; run the project's clang-format wrapper (e.g.
clang-format-diff.py or the repo's format script) on the changed hunk or on
src/active/context.cpp to normalize indentation/spacing around the ActiveContext
constructor, member initializer list (m_clhandler, m_isman, m_qman, nodeman,
dkgdbgman, qdkgsman, shareman, gov_signer, ehf_sighandler, qman_handler,
cl_signer, is_signer) and the lambda passed to qdkgsman->InitializeHandlers,
then add the resulting formatted changes and push the commit.
In @src/active/dkgsessionhandler.cpp:
- Around line 324-334: The fallback verification loop iterates messages and
calls session.GetMember(msg->proTxHash) without checking for a null member,
risking a null dereference when using member->dmn->pdmnState->pubKeyOperator in
msg->sig.VerifyInsecure; update the loop that iterates messages (the for (const
auto& [nodeId, msg] : messages) block), add a null check after obtaining member
from session.GetMember and treat a null member the same as an invalid signature
(e.g., emplace nodeId into ret and continue) before calling
msg->sig.VerifyInsecure.
In @src/llmq/dkgsessionhandler.cpp:
- Around line 22-27: CDKGSession is being constructed with
pQuorumBaseBlockIndex=nullptr which is unsafe because CDKGSession dereferences
m_quorum_base_block_index; replace the nullptr by obtaining a valid CBlockIndex*
from the available CQuorumSnapshotManager (qsnapman) and pass that into
CDKGSession in the CDKGSessionHandler constructor (or, if no such accessor
exists, add a method on CQuorumSnapshotManager to return the appropriate quorum
base CBlockIndex* and use it here); do not leave a nullptr, or alternatively
implement comprehensive nullptr checks inside CDKGSession where
m_quorum_base_block_index is dereferenced (references: CDKGSessionHandler
constructor, CDKGSession, m_quorum_base_block_index).
🧹 Nitpick comments (7)
src/net_processing.cpp (1)
1635-1636: Usingm_active_ctx != nullptras the masternode indicator is coherent across scheduling pathsYou now derive
is_masternodefromm_active_ctx != nullptrand feed that into both:
RequestObject→CalculateObjectGetDataTime(..., /*is_masternode=*/m_active_ctx != nullptr, !state->fPreferredDownload)ProcessMessage/SendMessagesvia a localconst bool is_masternode = m_active_ctx != nullptr;, which in turn drives:
- early MNAUTH handshake gating,
- DKG/session handling (
qdkgsman->ProcessMessage(..., is_masternode, ...)),- and the rescheduling of queued object requests (
CalculateObjectGetDataTime(..., is_masternode, !state.fPreferredDownload)).This keeps the “we’re a masternode node, don’t apply inbound TX delay and enable MN‑specific flows” decision centralized on the presence of
ActiveContext, which is exactly what this refactor is trying to consolidate.As long as
active_ctxis only instantiated when the node is configured to operate as an active masternode, this should be a no‑behavior‑change refactor.If you touch this again, consider a tiny helper like
bool IsMasternodeNode() const { return m_active_ctx != nullptr; }to avoid duplicating the check and make its intent explicit.Also applies to: 3715-3717, 5979-5980
src/active/masternode.h (1)
20-25: Private state split and guarded accessors are a good cleanupMoving to
m_outpoint,m_service, andm_protx_hashguarded bycswith inline READ-locked getters aligns with the mutex annotations and removes the need for a public info struct; keeping operator keys immutable viaconstmembers matches their intended invariant.Note: CI reports clang-format differences for this file; please run the project’s clang-format-diff tooling on it to clear the pipeline failure.
Also applies to: 37-43, 67-69
src/active/masternode.cpp (1)
265-271: BLS operations now correctly centralize on operator keys
Decrypt,Sign, andGetPubKeyconsistently operate onm_operator_sk/m_operator_pkunder read locks and assert the mutex is not already held at the public API, which matches the new role ofCActiveMasternodeManageras the single BLS operator-key holder.Note: clang-format diff is reported for this file as well; please re-run the formatter to satisfy CI.
Also applies to: 277-281, 293-297
src/active/dkgsession.cpp (1)
655-711: FinalizeSingleCommitment and MaybeDecrypt behavior are acceptable with minor nitsThe single-quorum final commitment uses the active masternode’s operator key as a workaround quorum pubkey and signs both quorum and members signatures with the appropriate BLS mode, then self-verifies and logs;
MaybeDecryptcleanly delegates toCActiveMasternodeManager::Decrypt. There is some minor dead/unused work (signerIds/thresholdSigsand the unusedsk1pubkey when the workaround is always enabled) that could be trimmed later without behavior change.Note: clang-format diff is reported for this file; please run the project’s clang-format tooling to fix style before merging.
src/active/dkgsessionhandler.cpp (3)
52-52: Remove or uncomment the assertion.The commented-out
//AssertLockNotHeld(cs_main);on line 52 should either be removed if no longer needed, or uncommented if the invariant should be enforced. Leaving dead comments clutters the code.🔎 Proposed fix
void ActiveDKGSessionHandler::UpdatedBlockTip(const CBlockIndex* pindexNew) { - //AssertLockNotHeld(cs_main); - //Indexed quorums (greater than 0) are enabled with Quorum Rotation + // Indexed quorums (greater than 0) are enabled with Quorum Rotation if (quorumIndex > 0 && !IsQuorumRotationEnabled(params, pindexNew)) {
122-123: Consider marking the exception classfinal.
AbortPhaseExceptionis a simple control-flow exception used locally. Addingfinalclarifies it's not meant to be subclassed.🔎 Proposed fix
-class AbortPhaseException : public std::exception { +class AbortPhaseException final : public std::exception { };
383-402: Remove redundant braces around single statements.The extra braces at lines 387-389 and 396-398 around
pendingMessages.Misbehaving()calls are unnecessary and inconsistent with the surrounding code style.🔎 Proposed fix
for (const auto& p : msgs) { const NodeId &nodeId = p.first; if (!p.second) { LogPrint(BCLog::LLMQ_DKG, "%s -- failed to deserialize message, peer=%d\n", __func__, nodeId); - { - pendingMessages.Misbehaving(nodeId, 100, peerman); - } + pendingMessages.Misbehaving(nodeId, 100, peerman); continue; } bool ban = false; if (!session.PreVerifyMessage(*p.second, ban)) { if (ban) { LogPrint(BCLog::LLMQ_DKG, "%s -- banning node due to failed preverification, peer=%d\n", __func__, nodeId); - { - pendingMessages.Misbehaving(nodeId, 100, peerman); - } + pendingMessages.Misbehaving(nodeId, 100, peerman); } LogPrint(BCLog::LLMQ_DKG, "%s -- skipping message due to failed preverification, peer=%d\n", __func__, nodeId); continue; }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/Makefile.amsrc/active/context.cppsrc/active/context.hsrc/active/dkgsession.cppsrc/active/dkgsessionhandler.cppsrc/active/masternode.cppsrc/active/masternode.hsrc/active/quorums.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/governance/signing.cppsrc/init.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/signing_shares.cppsrc/masternode/active/context.cppsrc/masternode/active/notificationinterface.cppsrc/masternode/active/notificationinterface.hsrc/net_processing.cppsrc/net_processing.hsrc/node/context.cppsrc/node/context.hsrc/node/interfaces.cppsrc/rpc/coinjoin.cppsrc/rpc/governance.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/coinjoin_queue_tests.cppsrc/test/denialofservice_tests.cppsrc/test/net_peer_connection_tests.cppsrc/test/util/setup_common.cppsrc/test/util/setup_common.htest/lint/lint-circular-dependencies.pytest/util/data/non-backported.txt
💤 Files with no reviewable changes (7)
- src/test/util/setup_common.h
- src/masternode/active/context.cpp
- test/util/data/non-backported.txt
- src/masternode/active/notificationinterface.h
- src/node/context.h
- src/masternode/active/notificationinterface.cpp
- src/net_processing.h
✅ Files skipped from review due to trivial changes (1)
- src/governance/signing.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/util/setup_common.cpp
- src/rpc/coinjoin.cpp
- src/active/quorums.cpp
🧰 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/rpc/governance.cppsrc/test/coinjoin_queue_tests.cppsrc/node/interfaces.cppsrc/test/net_peer_connection_tests.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/active/dkgsession.cppsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/test/denialofservice_tests.cppsrc/active/context.cppsrc/active/masternode.cppsrc/active/context.hsrc/active/dkgsessionhandler.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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/coinjoin_queue_tests.cppsrc/test/net_peer_connection_tests.cppsrc/test/denialofservice_tests.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/interfaces.cppsrc/node/context.cpp
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/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/evo/mnauth.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cpp
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/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/llmq/signing_shares.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/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/llmq/signing_shares.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/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/evo/mnauth.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cpp
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/mnauth.cpp
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/mnauth.cpp
src/coinjoin/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Files:
src/coinjoin/server.cpp
🧠 Learnings (38)
📓 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: 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.
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: 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
📚 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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Applied to files:
src/rpc/governance.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/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/Makefile.amsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/active/masternode.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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/{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/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/Makefile.amsrc/net_processing.cppsrc/init.cppsrc/test/denialofservice_tests.cppsrc/active/context.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/rpc/governance.cppsrc/test/coinjoin_queue_tests.cppsrc/test/net_peer_connection_tests.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/active/dkgsession.cppsrc/Makefile.amsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/test/denialofservice_tests.cppsrc/active/masternode.cppsrc/active/context.hsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.cpp
📚 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/rpc/governance.cppsrc/test/coinjoin_queue_tests.cppsrc/test/net_peer_connection_tests.cppsrc/rpc/quorums.cppsrc/test/denialofservice_tests.cppsrc/active/masternode.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/{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/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/Makefile.amsrc/net_processing.cppsrc/init.cppsrc/test/denialofservice_tests.cppsrc/active/context.hsrc/coinjoin/server.cppsrc/rpc/masternode.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/rpc/governance.cppsrc/node/interfaces.cppsrc/test/net_peer_connection_tests.cppsrc/Makefile.amsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/active/masternode.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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,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/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/Makefile.amtest/lint/lint-circular-dependencies.pysrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/active/masternode.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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/rpc/governance.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/active/masternode.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/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
src/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/rpc/masternode.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,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety
Applied to files:
src/rpc/governance.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/Makefile.amsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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/rpc/governance.cppsrc/node/context.cppsrc/init.cppsrc/test/denialofservice_tests.cppsrc/active/context.cppsrc/active/context.hsrc/rpc/masternode.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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/test/coinjoin_queue_tests.cppsrc/test/net_peer_connection_tests.cppsrc/node/context.cppsrc/rpc/quorums.cppsrc/active/dkgsession.cppsrc/Makefile.amsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/evo/mnauth.cppsrc/active/masternode.hsrc/init.cppsrc/active/masternode.cppsrc/active/context.hsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/masternode.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/coinjoin_queue_tests.cpp
📚 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/node/interfaces.cppsrc/net_processing.cppsrc/init.cppsrc/active/context.cppsrc/active/context.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/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
src/node/context.cppsrc/init.cppsrc/active/context.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/rpc/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/Makefile.amsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/init.cppsrc/llmq/signing_shares.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} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/rpc/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/active/dkgsession.cppsrc/Makefile.amsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/init.cppsrc/active/context.hsrc/active/dkgsessionhandler.cppsrc/llmq/signing_shares.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/rpc/quorums.cppsrc/evo/mnauth.cppsrc/coinjoin/server.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} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/rpc/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/init.cppsrc/active/context.hsrc/llmq/signing_shares.cpp
📚 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/rpc/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsession.cppsrc/net_processing.cppsrc/init.cppsrc/active/context.cppsrc/active/context.hsrc/active/dkgsessionhandler.cppsrc/llmq/signing_shares.cpp
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Applied to files:
src/rpc/quorums.cppsrc/net_processing.cppsrc/active/context.hsrc/active/dkgsessionhandler.cppsrc/llmq/signing_shares.cpp
📚 Learning: 2025-08-19T15:08:00.835Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/commitment.cpp:54-60
Timestamp: 2025-08-19T15:08:00.835Z
Learning: In Dash Core, llmq_params.size == 1 is used only for regtest environments, not on public networks, which significantly reduces the risk profile of code paths specific to single-member quorums.
Applied to files:
src/rpc/quorums.cpp
📚 Learning: 2025-07-23T09:30:34.631Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.
Applied to files:
src/Makefile.am
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
src/Makefile.am
📚 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/net_processing.cppsrc/init.cppsrc/rpc/masternode.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/net_processing.cpp
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Applied to files:
test/lint/lint-circular-dependencies.py
📚 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/active/masternode.hsrc/active/context.h
📚 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/active/context.cppsrc/active/context.h
📚 Learning: 2025-05-10T00:54:15.597Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6629
File: src/evo/deterministicmns.cpp:473-485
Timestamp: 2025-05-10T00:54:15.597Z
Learning: In the current implementation of `NetInfoEntry`, only `CService` is a valid type and `std::monostate` represents an invalid state. Entries that don't provide a valid `CService` through `GetAddrPort()` should be rejected with an exception, not silently skipped.
Applied to files:
src/active/masternode.cpp
📚 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/active/masternode.cpp
📚 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/active/masternode.cpp
📚 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/active/context.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,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
src/coinjoin/server.cppsrc/llmq/signing_shares.cpp
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.
Applied to files:
src/rpc/masternode.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/masternode.cpp
🧬 Code graph analysis (12)
src/test/coinjoin_queue_tests.cpp (1)
src/test/util/setup_common.cpp (2)
TestingSetup(312-395)TestingSetup(397-422)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/rpc/quorums.cpp (1)
src/interfaces/chain.h (1)
node(40-42)
src/active/dkgsession.cpp (1)
src/llmq/dkgsession.cpp (4)
ShouldSimulateError(52-59)ShouldSimulateError(52-52)MarkBadMember(686-697)MarkBadMember(686-686)
src/net_processing.cpp (1)
src/evo/mnauth.cpp (2)
PushMNAUTH(19-49)PushMNAUTH(19-19)
src/active/masternode.h (1)
src/active/masternode.cpp (9)
CActiveMasternodeManager(52-63)CActiveMasternodeManager(65-65)nodiscard(266-271)nodiscard(277-281)nodiscard(283-289)nodiscard(293-297)GetPubKey(293-293)GetLocalAddress(217-255)GetLocalAddress(217-217)
src/init.cpp (3)
src/interfaces/node.h (2)
node(48-50)node(481-481)src/init.h (1)
node(23-25)src/validationinterface.cpp (4)
UnregisterValidationInterface(145-150)UnregisterValidationInterface(145-145)RegisterValidationInterface(133-138)RegisterValidationInterface(133-133)
src/test/denialofservice_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/active/masternode.cpp (1)
src/active/masternode.h (1)
cs(40-75)
src/active/context.h (1)
src/active/context.cpp (6)
ActiveContext(26-61)ActiveContext(63-68)NotifyRecoveredSig(119-122)NotifyRecoveredSig(119-119)UpdatedBlockTip(107-117)UpdatedBlockTip(107-107)
src/active/dkgsessionhandler.cpp (5)
src/llmq/observer/context.cpp (2)
UpdatedBlockTip(51-58)UpdatedBlockTip(51-51)src/llmq/dkgsessionmgr.cpp (2)
UpdatedBlockTip(60-74)UpdatedBlockTip(60-60)src/llmq/commitment.h (1)
quorumIndex(56-56)src/util/time.h (1)
TicksSinceEpoch(86-89)src/active/dkgsession.cpp (2)
Contribute(38-59)Contribute(38-38)
src/rpc/masternode.cpp (1)
src/interfaces/node.h (2)
node(48-50)node(481-481)
🪛 Cppcheck (2.19.0)
src/rpc/governance.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/test/coinjoin_queue_tests.cpp
[error] 15-15: There is an unknown macro here somewhere. Configuration is required. If BOOST_FIXTURE_TEST_SUITE is a macro then please configure it.
(unknownMacro)
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/rpc/quorums.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/llmq/dkgsessionhandler.cpp
[error] 10-10: 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/active/dkgsession.cpp
[error] 10-10: 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/mnauth.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/active/context.cpp
[error] 10-10: 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/active/dkgsessionhandler.cpp
[error] 10-10: 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/coinjoin/server.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/llmq/signing_shares.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/rpc/masternode.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
🪛 GitHub Actions: Clang Diff Format Check
src/active/dkgsession.cpp
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
src/active/masternode.h
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
src/active/context.cpp
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
src/active/masternode.cpp
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
src/active/dkgsessionhandler.cpp
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
⏰ 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: linux64_sqlite-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: Lint / Run linters
| // Copyright (c) 2025 The Dash Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #include <active/context.h> | ||
|
|
||
| #include <active/dkgsessionhandler.h> | ||
| #include <active/masternode.h> | ||
| #include <active/quorums.h> | ||
| #include <chainlock/chainlock.h> | ||
| #include <chainlock/signing.h> | ||
| #include <governance/governance.h> | ||
| #include <governance/signing.h> | ||
| #include <instantsend/instantsend.h> | ||
| #include <instantsend/signing.h> | ||
| #include <llmq/context.h> | ||
| #include <llmq/debug.h> | ||
| #include <llmq/dkgsessionmgr.h> | ||
| #include <llmq/ehf_signals.h> | ||
| #include <llmq/quorumsman.h> | ||
| #include <llmq/signing_shares.h> | ||
|
|
||
| #include <util/check.h> | ||
| #include <validation.h> | ||
|
|
||
| ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman, | ||
| CDeterministicMNManager& dmnman, CGovernanceManager& govman, | ||
| CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman, | ||
| CTxMemPool& mempool, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman, | ||
| llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman, | ||
| llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman, | ||
| PeerManager& peerman, const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk, | ||
| const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params, | ||
| bool quorums_recovery, bool quorums_watch) : | ||
| m_clhandler{clhandler}, | ||
| m_isman{isman}, | ||
| m_qman{qman}, | ||
| nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)}, | ||
| dkgdbgman{std::make_unique<llmq::CDKGDebugManager>()}, | ||
| qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)}, | ||
| shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman.ActiveChainstate(), sigman, peerman, *nodeman, | ||
| qman, sporkman)}, | ||
| gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)}, | ||
| ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, mnhfman, sigman, *shareman, qman)}, | ||
| qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman, | ||
| mn_sync, sporkman, sync_map, quorums_recovery, quorums_watch)}, | ||
| cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), clhandler, sigman, *shareman, | ||
| sporkman, mn_sync)}, | ||
| is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), clhandler, isman, sigman, | ||
| *shareman, qman, sporkman, mempool, mn_sync)} | ||
| { | ||
| qdkgsman->InitializeHandlers([&](const Consensus::LLMQParams& llmq_params, | ||
| int quorum_idx) -> std::unique_ptr<llmq::ActiveDKGSessionHandler> { | ||
| return std::make_unique<llmq::ActiveDKGSessionHandler>(bls_worker, dmnman, mn_metaman, *dkgdbgman, *qdkgsman, | ||
| qblockman, qsnapman, *nodeman, chainman, sporkman, | ||
| llmq_params, quorums_watch, quorum_idx); | ||
| }); | ||
| m_clhandler.ConnectSigner(cl_signer.get()); | ||
| m_isman.ConnectSigner(is_signer.get()); | ||
| m_qman.ConnectManagers(qman_handler.get(), qdkgsman.get()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix clang-format issues to satisfy CI
CI reports a Clang Diff Format Check failure for this file. Please run the project’s clang-format script (e.g. clang-format-diff.py over this hunk) to normalize indentation and spacing before merging.
🧰 Tools
🪛 Cppcheck (2.19.0)
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🪛 GitHub Actions: Clang Diff Format Check
[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.
🤖 Prompt for AI Agents
In @src/active/context.cpp around lines 1-61, The ActiveContext constructor
formatting in this file fails clang-format CI; run the project's clang-format
wrapper (e.g. clang-format-diff.py or the repo's format script) on the changed
hunk or on src/active/context.cpp to normalize indentation/spacing around the
ActiveContext constructor, member initializer list (m_clhandler, m_isman,
m_qman, nodeman, dkgdbgman, qdkgsman, shareman, gov_signer, ehf_sighandler,
qman_handler, cl_signer, is_signer) and the lambda passed to
qdkgsman->InitializeHandlers, then add the resulting formatted changes and push
the commit.
| for (const auto& [nodeId, msg] : messages) { | ||
| if (ret.count(nodeId)) { | ||
| continue; | ||
| } | ||
|
|
||
| auto member = session.GetMember(msg->proTxHash); | ||
| bool valid = msg->sig.VerifyInsecure(member->dmn->pdmnState->pubKeyOperator.Get(), msg->GetSignHash()); | ||
| if (!valid) { | ||
| ret.emplace(nodeId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null check for member in fallback verification loop.
At line 329, session.GetMember() is called again without a null check. While the earlier loop (lines 276-281) would have added the nodeId to ret if the member was null, session state could theoretically change. Adding a defensive check prevents a potential null pointer dereference.
🔎 Proposed fix
for (const auto& [nodeId, msg] : messages) {
if (ret.count(nodeId)) {
continue;
}
auto member = session.GetMember(msg->proTxHash);
+ if (!member) {
+ ret.emplace(nodeId);
+ continue;
+ }
bool valid = msg->sig.VerifyInsecure(member->dmn->pdmnState->pubKeyOperator.Get(), msg->GetSignHash());
if (!valid) {
ret.emplace(nodeId);
}
}🤖 Prompt for AI Agents
In @src/active/dkgsessionhandler.cpp around lines 324-334, The fallback
verification loop iterates messages and calls session.GetMember(msg->proTxHash)
without checking for a null member, risking a null dereference when using
member->dmn->pdmnState->pubKeyOperator in msg->sig.VerifyInsecure; update the
loop that iterates messages (the for (const auto& [nodeId, msg] : messages)
block), add a null check after obtaining member from session.GetMember and treat
a null member the same as an invalid signature (e.g., emplace nodeId into ret
and continue) before calling msg->sig.VerifyInsecure.
| CDKGSessionHandler::CDKGSessionHandler(CBLSWorker& _blsWorker, CDeterministicMNManager& dmnman, | ||
| CDKGDebugManager& _dkgDebugManager, CDKGSessionManager& _dkgManager, | ||
| CMasternodeMetaMan& mn_metaman, CQuorumBlockProcessor& _quorumBlockProcessor, | ||
| CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager* const mn_activeman, | ||
| const ChainstateManager& chainman, const CSporkManager& sporkman, | ||
| CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, | ||
| const Consensus::LLMQParams& _params, bool quorums_watch, int _quorumIndex) : | ||
| blsWorker{_blsWorker}, | ||
| m_dmnman{dmnman}, | ||
| dkgDebugManager{_dkgDebugManager}, | ||
| dkgManager{_dkgManager}, | ||
| m_mn_metaman{mn_metaman}, | ||
| quorumBlockProcessor{_quorumBlockProcessor}, | ||
| m_qsnapman{qsnapman}, | ||
| m_mn_activeman{mn_activeman}, | ||
| m_chainman{chainman}, | ||
| m_sporkman{sporkman}, | ||
| curSession{std::make_unique<CDKGSession>(_blsWorker, dmnman, _dkgDebugManager, _dkgManager, qsnapman, chainman, | ||
| /*pQuorumBaseBlockIndex=*/nullptr, _params)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CDKGSessionHandler constructor invocations (excluding the definition file)
rg -nP --type=cpp -C3 'CDKGSessionHandler\s*\(' -g '!src/llmq/dkgsessionhandler.cpp'Repository: dashpay/dash
Length of output: 1767
🏁 Script executed:
#!/bin/bash
# Search for potential issues with nullptr pQuorumBaseBlockIndex usage in CDKGSession
rg -nP --type=cpp -C3 'pQuorumBaseBlockIndex' -g 'src/llmq/dkgsession.*'Repository: dashpay/dash
Length of output: 1577
🏁 Script executed:
#!/bin/bash
# Examine CDKGSessionHandler constructor definition to verify parameters
cd src/llmq && head -40 dkgsessionhandler.cpp | cat -nRepository: dashpay/dash
Length of output: 2171
🏁 Script executed:
#!/bin/bash
# Search for CDKGSessionHandler instantiation patterns across the codebase
rg -nP --type=cpp 'new CDKGSessionHandler|CDKGSessionHandler\s*\{' -g '!src/llmq/dkgsessionhandler.cpp'Repository: dashpay/dash
Length of output: 38
🏁 Script executed:
#!/bin/bash
# Check CDKGSession constructor definition to understand nullptr parameter handling
fd -e cpp -e h 'dkgsession' src/llmq | head -5 | xargs grep -l 'class CDKGSession' | head -1 | xargs head -100 | cat -nRepository: dashpay/dash
Length of output: 3196
🏁 Script executed:
#!/bin/bash
# Search for all CDKGSessionHandler instantiations across entire codebase
rg -nP --type=cpp 'CDKGSessionHandler' | grep -v 'dkgsessionhandler\.(cpp|h)' | grep -v '//'Repository: dashpay/dash
Length of output: 2368
🏁 Script executed:
#!/bin/bash
# Check how m_quorum_base_block_index is used in CDKGSession class
rg -nP --type=cpp 'm_quorum_base_block_index' src/llmq/Repository: dashpay/dash
Length of output: 4138
🏁 Script executed:
#!/bin/bash
# Search for nullptr checks or usage patterns of pQuorumBaseBlockIndex in CDKGSession
rg -nP --type=cpp -A5 -B5 'pQuorumBaseBlockIndex' src/llmq/dkgsession.cpp | head -60Repository: dashpay/dash
Length of output: 811
Remove nullptr initialization; CDKGSession requires valid pQuorumBaseBlockIndex.
The constructor call sites in src/llmq/observer/context.cpp and src/active/dkgsessionhandler.cpp have been correctly updated to pass the new CQuorumSnapshotManager& qsnapman parameter. However, initializing CDKGSession with pQuorumBaseBlockIndex=nullptr is unsafe.
CDKGSession dereferences m_quorum_base_block_index without nullptr checks throughout its implementation (lines 33, 88, 116-118, 132-133, 160, 250, 290, 394, 521, 545, 624 in dkgsession.cpp). This will cause a null pointer dereference crash immediately upon construction.
Either provide a valid CBlockIndex pointer or add comprehensive nullptr guards before all dereferencing operations.
🧰 Tools
🪛 Cppcheck (2.19.0)
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🤖 Prompt for AI Agents
In @src/llmq/dkgsessionhandler.cpp around lines 22-27, CDKGSession is being
constructed with pQuorumBaseBlockIndex=nullptr which is unsafe because
CDKGSession dereferences m_quorum_base_block_index; replace the nullptr by
obtaining a valid CBlockIndex* from the available CQuorumSnapshotManager
(qsnapman) and pass that into CDKGSession in the CDKGSessionHandler constructor
(or, if no such accessor exists, add a method on CQuorumSnapshotManager to
return the appropriate quorum base CBlockIndex* and use it here); do not leave a
nullptr, or alternatively implement comprehensive nullptr checks inside
CDKGSession where m_quorum_base_block_index is dereferenced (references:
CDKGSessionHandler constructor, CDKGSession, m_quorum_base_block_index).
Additional Information
Depends on refactor: extract
CActiveMasternodeManagerfromLLMQContext(2/n,CQuorumManagerhandler separation) #7063Dependency for refactor: extract
CActiveMasternodeManagerfromLLMQContext(4/n, followups) #7066As
ActiveNotificationInterfaceexclusively interacts withActiveContextmembers, we can squash them together likeObserverContext.Breaking Changes
None expected.
Checklist: