-
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
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.
WalkthroughThis PR moves masternode-active code from src/masternode/active/ into src/active/, introducing ActiveContext (now a CValidationInterface) and wiring it into NodeContext as node.active_ctx. It adds ActiveDKGSession and ActiveDKGSessionHandler, changes CDKGSession/CDKGSessionHandler and CDKGSessionManager APIs (handler factory/SessionHandlerKey), reorders several constructor parameter lists (LLMQContext, LLMQ components), refactors CActiveMasternodeManager internal state and constructor, updates many includes/Makefile exports, and replaces uses of node.mn_activeman with node.active_ctx across the codebase. Sequence Diagram(s)sequenceDiagram
%% Colors for clarity (works in many renderers)
participant Node as Node
participant ActiveCtx as ActiveContext
participant DKGMgr as CDKGSessionManager
participant DKGH as ActiveDKGSessionHandler
participant Peer as PeerManager
participant Chain as ChainstateManager
Note over Node,ActiveCtx: Startup wiring
Node->>ActiveCtx: construct ActiveContext(deps...)
ActiveCtx->>DKGMgr: InitializeHandlers(factory)
DKGMgr-->>DKGH: factory(llmq_params, quorum_idx) → new ActiveDKGSessionHandler
ActiveCtx->>Peer: SetCJServer / register managers
Note over DKGH,Peer: DKG runtime (repeating per quorum/phase)
loop DKG phases
DKGH->>Peer: Relay/Send messages (Inv/Contrib/Complaint/Justify/Commit)
Peer->>DKGH: Deliver messages via ProcessMessage
DKGH->>DKGMgr: Update session state / submit commitments
DKGMgr->>Chain: Query block tip / quorum base index
DKGMgr->>ActiveCtx: NotifyRecoveredSig / UpdatedBlockTip
ActiveCtx->>DKGH: Forward UpdatedBlockTip / coordinate signers
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.
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
|
pls check commits in https://github.com/UdjinM6/dash/commits/refs/heads/pr-7065-attempt1/ |
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Co-authored-by: UdjinM6 <[email protected]>
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
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
…s are created Co-authored-by: UdjinM6 <[email protected]>
Co-authored-by: UdjinM6 <[email protected]>
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: 1
🤖 Fix all issues with AI Agents
In @src/active/dkgsessionhandler.cpp:
- Around line 319-329: The fallback verification loop calls
session.GetMember(msg->proTxHash) and dereferences member without a null check;
update the loop (the for (const auto& [nodeId, msg] : messages) block) to test
if member is null right after assignment and, if so, emplace nodeId into ret and
continue, before accessing member->dmn->pdmnState->pubKeyOperator or calling
msg->GetSignHash(), ensuring no dereference of a null member during fallback
verification.
🧹 Nitpick comments (4)
src/llmq/dkgsession.h (1)
353-383: Template method pattern requires complete overrides in derived class.The virtual methods with empty default implementations establish a template method pattern where
ActiveDKGSessionis expected to override all these methods. Ensure thatActiveDKGSessionprovides complete implementations for all virtual methods, as the base class returns empty/default values.Consider adding
= 0to make critical methods pure virtual, forcing derived classes to implement them and preventing accidental calls to empty base implementations:-virtual std::vector<CFinalCommitment> FinalizeCommitments() EXCLUSIVE_LOCKS_REQUIRED(!invCs) { return {}; } +virtual std::vector<CFinalCommitment> FinalizeCommitments() EXCLUSIVE_LOCKS_REQUIRED(!invCs) = 0;However, if the base class is intentionally usable without these methods (e.g., for passive/observer nodes), the current approach is acceptable.
src/llmq/observer/context.cpp (1)
28-31: Consider empty capture for lambda clarity.The lambda captures by reference (
[&]) but doesn't use any captured variables. Consider using an empty capture list ([]) for clarity and to avoid unintended dependencies.🔎 Proposed fix
- qdkgsman->InitializeHandlers([&](const Consensus::LLMQParams& llmq_params, + qdkgsman->InitializeHandlers([](const Consensus::LLMQParams& llmq_params, [[maybe_unused]] int quorum_idx) -> std::unique_ptr<llmq::CDKGSessionHandler> { return std::make_unique<llmq::CDKGSessionHandler>(llmq_params); });src/active/dkgsessionhandler.h (1)
45-68: Consider consolidating the twoprivate:sections.There are two separate
private:sections (lines 45-57 and 59-68). While valid C++, consolidating them would improve readability.Also note minor naming inconsistency:
quorumIndexdoesn't follow them_prefix pattern used by other members likem_bls_worker,m_dmnman, etc.src/active/dkgsessionhandler.cpp (1)
47-47: Clarify the commented-out assertion.The
//AssertLockNotHeld(cs_main);is commented out. If this is intentional for this refactoring, consider adding a brief comment explaining why, or remove it if no longer applicable.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
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/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/net_processing.h
- src/masternode/active/context.cpp
- src/masternode/active/notificationinterface.cpp
- src/node/context.h
- src/test/util/setup_common.h
- src/node/chainstate.h
- src/masternode/active/notificationinterface.h
🚧 Files skipped from review as they are similar to previous changes (6)
- src/governance/signing.cpp
- src/node/interfaces.cpp
- src/Makefile.am
- test/lint/lint-circular-dependencies.py
- src/active/quorums.cpp
- src/node/chainstate.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/coinjoin.cppsrc/llmq/dkgsession.hsrc/active/masternode.cppsrc/llmq/context.hsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/active/dkgsession.hsrc/llmq/dkgsessionmgr.cppsrc/active/dkgsession.cppsrc/llmq/dkgsession.cppsrc/active/masternode.hsrc/test/coinjoin_queue_tests.cppsrc/active/dkgsessionhandler.hsrc/active/dkgsessionhandler.cppsrc/test/denialofservice_tests.cppsrc/node/context.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cppsrc/active/context.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.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/dkgsession.hsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.cppsrc/coinjoin/server.cppsrc/evo/mnauth.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/dkgsession.hsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.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/dkgsession.hsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.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/dkgsession.hsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.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/denialofservice_tests.cppsrc/test/util/setup_common.cppsrc/test/net_peer_connection_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/context.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
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
🧠 Learnings (48)
📓 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: 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: 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: 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: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
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: 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.
📚 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/rpc/coinjoin.cppsrc/llmq/dkgsession.hsrc/active/masternode.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/active/dkgsession.hsrc/llmq/dkgsession.cppsrc/active/masternode.hsrc/test/coinjoin_queue_tests.cppsrc/active/dkgsessionhandler.hsrc/node/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cppsrc/active/context.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.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-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/rpc/coinjoin.cppsrc/init.cppsrc/net_processing.cppsrc/active/context.hsrc/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}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/rpc/coinjoin.cppsrc/llmq/dkgsession.hsrc/active/masternode.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/active/dkgsession.hsrc/active/dkgsession.cppsrc/llmq/dkgsession.cppsrc/active/masternode.hsrc/test/coinjoin_queue_tests.cppsrc/active/dkgsessionhandler.hsrc/test/denialofservice_tests.cppsrc/node/context.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/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/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite
Applied to files:
src/rpc/coinjoin.cppsrc/rpc/masternode.cppsrc/init.cppsrc/net_processing.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,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/coinjoin.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/denialofservice_tests.cppsrc/node/context.cppsrc/init.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.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/coinjoin.cppsrc/llmq/context.hsrc/rpc/masternode.cppsrc/llmq/context.cppsrc/test/denialofservice_tests.cppsrc/node/context.cppsrc/init.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/active/context.cppsrc/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/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/llmq/dkgsession.hsrc/llmq/context.hsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/active/dkgsession.hsrc/llmq/dkgsessionmgr.cppsrc/active/dkgsession.cppsrc/llmq/dkgsession.cppsrc/active/dkgsessionhandler.hsrc/active/dkgsessionhandler.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.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} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Applied to files:
src/llmq/dkgsession.hsrc/llmq/context.hsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/test/util/setup_common.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} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/llmq/dkgsession.hsrc/llmq/context.hsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/active/dkgsessionhandler.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/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/llmq/dkgsession.hsrc/llmq/context.hsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsession.cppsrc/llmq/observer/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/active/context.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/{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/llmq/dkgsession.hsrc/active/masternode.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/node/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/llmq/dkgsessionhandler.cppsrc/test/util/setup_common.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.cpp
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/llmq/dkgsession.hsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/observer/context.cppsrc/active/context.hsrc/active/context.cpp
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/llmq/dkgsession.hsrc/llmq/context.hsrc/active/dkgsession.hsrc/active/masternode.hsrc/active/dkgsessionhandler.hsrc/llmq/dkgsessionhandler.hsrc/active/context.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/active/masternode.cppsrc/active/masternode.hsrc/evo/mnauth.cppsrc/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/active/masternode.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/active/masternode.hsrc/node/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/net_processing.cppsrc/test/util/setup_common.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.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/active/masternode.cppsrc/rpc/masternode.cppsrc/rpc/quorums.cppsrc/test/coinjoin_queue_tests.cppsrc/test/denialofservice_tests.cppsrc/llmq/dkgsessionhandler.cppsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cppsrc/rpc/governance.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/active/masternode.cppsrc/rpc/masternode.cppsrc/active/masternode.hsrc/init.cppsrc/net_processing.cppsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/rpc/governance.cpp
📚 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/llmq/context.hsrc/llmq/context.cppsrc/llmq/observer/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/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety
Applied to files:
src/rpc/masternode.cppsrc/rpc/quorums.cppsrc/node/context.cppsrc/net_processing.cppsrc/coinjoin/server.cppsrc/evo/mnauth.cppsrc/llmq/signing_shares.cppsrc/rpc/governance.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-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/masternode.cppsrc/rpc/quorums.cppsrc/llmq/context.cppsrc/node/context.cppsrc/llmq/dkgsessionhandler.hsrc/test/util/setup_common.cppsrc/rpc/governance.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-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.cppsrc/llmq/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,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/quorums.cppsrc/test/denialofservice_tests.cppsrc/node/context.cppsrc/llmq/dkgsessionhandler.hsrc/init.cppsrc/net_processing.cppsrc/active/context.hsrc/test/util/setup_common.cppsrc/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/**/*.{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/coinjoin/server.cppsrc/evo/mnauth.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/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.cppsrc/llmq/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/{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-07-29T14:32:48.369Z
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.
Applied to files:
src/active/dkgsessionhandler.cppsrc/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/active/dkgsessionhandler.cppsrc/llmq/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/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/test/util/setup_common.cppsrc/active/context.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:
src/llmq/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/{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/dkgsessionhandler.cppsrc/coinjoin/server.cppsrc/llmq/signing_shares.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/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-07-17T15:48:29.418Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
Applied to files:
src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-07-29T14:33:01.040Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Applied to files:
src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Applied to files:
src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-10-13T12:37:12.357Z
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Applied to files:
src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
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.
Applied to files:
src/llmq/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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Applied to files:
src/test/util/setup_common.cppsrc/rpc/governance.cpp
📚 Learning: 2025-02-14T15:15:58.165Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Applied to files:
src/active/context.cpp
📚 Learning: 2025-07-09T15:05:36.250Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Applied to files:
src/active/context.cpp
🧬 Code graph analysis (19)
src/llmq/dkgsession.h (5)
src/active/dkgsessionhandler.h (2)
llmq(31-37)Consensus(28-30)src/active/dkgsession.cpp (4)
ActiveDKGSession(23-34)ActiveDKGSession(36-36)MaybeDecrypt(713-717)MaybeDecrypt(713-714)src/llmq/dkgsession.cpp (2)
CDKGSession(68-82)CDKGSession(84-84)src/llmq/dkgsessionhandler.cpp (2)
CDKGSessionHandler(14-25)CDKGSessionHandler(27-27)src/llmq/dkgsessionhandler.h (1)
Consensus(29-31)
src/active/masternode.cpp (1)
src/active/masternode.h (1)
cs(40-75)
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/llmq/context.cpp (1)
src/node/interfaces.cpp (3)
chainman(754-756)chainman(1239-1241)chainman(1309-1312)
src/active/dkgsession.h (1)
src/llmq/dkgsession.h (4)
llmq(30-39)llmq(41-67)CDKGSession(276-409)CFinalCommitment(383-383)
src/active/dkgsession.cpp (1)
src/llmq/dkgsession.cpp (4)
ShouldSimulateError(52-59)ShouldSimulateError(52-52)MarkBadMember(686-697)MarkBadMember(686-686)
src/llmq/dkgsession.cpp (2)
src/llmq/dkgsession.h (1)
CDKGSession(276-409)src/active/dkgsession.cpp (2)
MaybeDecrypt(713-717)MaybeDecrypt(713-714)
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/test/coinjoin_queue_tests.cpp (1)
src/test/util/setup_common.cpp (2)
TestingSetup(312-395)TestingSetup(397-422)
src/active/dkgsessionhandler.h (3)
src/llmq/dkgsessionhandler.h (1)
Consensus(29-31)src/active/dkgsessionhandler.cpp (10)
ActiveDKGSessionHandler(22-41)ActiveDKGSessionHandler(43-43)GetContribution(528-540)GetContribution(528-528)GetComplaint(542-554)GetComplaint(542-542)GetJustification(556-568)GetJustification(556-556)GetPrematureCommitment(570-582)GetPrematureCommitment(570-570)src/llmq/dkgsessionhandler.cpp (2)
CDKGSessionHandler(14-25)CDKGSessionHandler(27-27)
src/active/dkgsessionhandler.cpp (2)
src/llmq/dkgsessionmgr.cpp (10)
UpdatedBlockTip(60-74)UpdatedBlockTip(60-60)GetContribution(187-202)GetContribution(187-187)GetComplaint(204-219)GetComplaint(204-204)GetJustification(221-236)GetJustification(221-221)GetPrematureCommitment(238-253)GetPrematureCommitment(238-238)src/llmq/debug.h (1)
phase(60-60)
src/test/denialofservice_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/llmq/dkgsessionhandler.h (2)
src/active/dkgsessionhandler.h (1)
Consensus(28-30)src/llmq/options.h (1)
Consensus(18-21)
src/net_processing.cpp (2)
src/evo/mnauth.cpp (2)
PushMNAUTH(19-49)PushMNAUTH(19-19)src/llmq/signing_shares.cpp (2)
ProcessMessage(255-332)ProcessMessage(255-255)
src/active/context.h (1)
src/active/context.cpp (12)
ActiveContext(26-61)ActiveContext(63-68)Start(75-83)Start(75-75)GetCJServer(95-98)GetCJServer(95-95)SetCJServer(100-105)SetCJServer(100-100)NotifyRecoveredSig(119-122)NotifyRecoveredSig(119-119)UpdatedBlockTip(107-117)UpdatedBlockTip(107-107)
src/test/util/setup_common.cpp (1)
src/node/interfaces.cpp (8)
Assert(197-197)Assert(198-198)Assert(223-223)Assert(364-364)Assert(387-387)Assert(520-520)Assert(791-794)Assert(1003-1003)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(129-138)MakePeerManager(129-133)
src/active/context.cpp (1)
src/active/context.h (1)
m_cj_server(105-105)
🪛 Cppcheck (2.19.0)
src/rpc/masternode.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/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/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/llmq/observer/context.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)
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/coinjoin/server.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
src/evo/mnauth.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/governance.cpp
[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: win64-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: linux64_ubsan-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: Lint / Run linters
| 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.
Missing null check for member in fallback verification loop.
At line 324, session.GetMember(msg->proTxHash) is called but the result is not checked for null before dereferencing at line 325. While the earlier loop (lines 271-276) adds nodeId to ret if member is null, this fallback loop could still receive messages that weren't processed in the first loop due to revertToSingleVerification being set.
🔎 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 319 - 329, The fallback
verification loop calls session.GetMember(msg->proTxHash) and dereferences
member without a null check; update the loop (the for (const auto& [nodeId, msg]
: messages) block) to test if member is null right after assignment and, if so,
emplace nodeId into ret and continue, before accessing
member->dmn->pdmnState->pubKeyOperator or calling msg->GetSignHash(), ensuring
no dereference of a null member during fallback verification.
|
If Dash Core is created only with consensus code (as it is required by project "kernel" which is being backported from bitcoin, to support utxo snapshots), we don't need On other hand, if Dash Core is created with network code (not only consensus), so, it can participate in any DKGSession as a watch-only member anyway, and this separation (as done in PR) is not required; it just over-complicates implementation, creates multiple virtual functions, etc. Beside performance degradation, there is significant code move, files renaming, etc. To separate network & consensus code this PR doesn't bring anything new but a new circular dependency Please, consider instead this commit: 91f8f9b (this commit ID is updated, v2, due to compiler error) because everything to isolate CActiveMasternodeManager is already done in these 3 PRs: #7070 #7063 #7062 I am going to attempt 619f8a2 (Merge bitcoin#24304): [kernel 0/n] Introduce |
|
It's true that this change doesn't directly contribute to To work on unit tests/fuzzing, completing the bifurcation is in the better interest. Plus, if Fundamentally the circular dependency is a product of the There is overhead but two things come into play, firstly, this is not running on all nodes, only nodes observing/participating and in terms of CPU cycles, vtable lookups are relatively cheap compared to heavy consensus primitives and the inheritance structure is unlikely to cause slowdowns, especially since we don't expect rapid session churn. |
PastaPastaPasta
left a 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.
utACK de4d0ea
UdjinM6
left a 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.
utACK de4d0ea
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: