Skip to content

Conversation

@kwvg
Copy link
Collaborator

@kwvg kwvg commented Dec 13, 2025

Additional Information

Breaking Changes

None expected.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation (note: N/A)
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@github-actions
Copy link

github-actions bot commented Dec 13, 2025

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

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

@kwvg kwvg changed the title refactor: extract CActiveMasternodeManager from LLMQContext (3/3, DKG session isolation, ActiveContext consolidation) refactor: extract CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation) Dec 13, 2025
@github-actions
Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request Jan 3, 2026
…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
@kwvg kwvg marked this pull request as ready for review January 4, 2026 21:31
@coderabbitai
Copy link

coderabbitai bot commented Jan 4, 2026

Walkthrough

This PR moves masternode-active code from src/masternode/active/ into src/active/, refactors ActiveContext to inherit from CValidationInterface and manage masternode/LLMQ subsystems, introduces ActiveDKGSession and ActiveDKGSessionHandler for DKG lifecycle, reworks CDKGSession/CDKGSessionHandler and CDKGSessionManager APIs (handler factory), reorders several constructor parameter lists (LLMQContext, LLMQ components), changes CActiveMasternodeManager construction/fields, replaces node.mn_activeman with node.active_ctx throughout, and updates many includes and Makefile target exports.

Sequence Diagram(s)

sequenceDiagram
    %% Styling
    participant Node
    participant ActiveContext
    participant DKGMgr as CDKGSessionManager
    participant DKGHandler as ActiveDKGSessionHandler
    participant PeerMgr as PeerManager
    participant Chain as ChainstateManager

    Note over Node,ActiveContext: Node constructs ActiveContext during startup
    Node->>ActiveContext: new ActiveContext(...deps...)
    ActiveContext->>DKGMgr: InitializeHandlers(factory)
    DKGMgr-->>DKGHandler: factory(llmq_params, quorum_idx) -> create handler
    ActiveContext->>PeerMgr: SetCJServer / register managers
    Note over ActiveContext,DKGHandler: DKGHandler runs per-quorum loop (phases)

    loop DKG phases
        DKGHandler->>PeerMgr: RelayInvToParticipants(invs)
        PeerMgr->>DKGHandler: Receive messages / CConnman relay
        DKGHandler->>DKGMgr: Update contributions / verify / commit
        DKGMgr->>Chain: Query block/proof state
        DKGMgr->>ActiveContext: NotifyRecoveredSig / UpdatedBlockTip
        ActiveContext->>DKGHandler: Forward block-tip updates
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • PastaPastaPasta
  • UdjinM6
  • knst

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.79% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main refactoring: extracting CActiveMasternodeManager from LLMQContext and consolidating with ActiveContext, with specific focus on DKG session isolation.
Description check ✅ Passed The description is related to the changeset, explaining the consolidation of ActiveNotificationInterface into ActiveContext and the dependencies on related PRs, though it could be more detailed about the extensive file relocations and refactoring scope.
✨ Finishing touches
  • 📝 Generate docstrings

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

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a 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 multiple private: 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: Using m_active_ctx != nullptr as the is_masternode signal is coherent across request scheduling

Deriving is_masternode from m_active_ctx != nullptr in both ProcessMessage and SendMessages, and threading that into CalculateObjectGetDataTime (in RequestObject and in the requeue path) keeps the previous behaviour of “masternode nodes skip inbound TX delay” while now keying off the new ActiveContext as 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 the inv.IsMsgTx() case, so non-TX objects and non-masternode nodes remain unaffected. As a minor readability tweak, you could factor a tiny helper like bool 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 sound

The new constructor parameter ordering (worker, dmnman, debug, manager, qsnapman, chainman, base index, params) and reference members in CDKGSession are coherent with the call sites, and the defaulted virtual destructor is appropriate for subclassing.

In ReceiveMessage(const CDKGContribution& qc), the AreWeMember() guard before calling MaybeDecrypt(*qc.contributions, *myIdx, …) matches the Init logic that only sets myProTxHash and myIdx when 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 that myIdx.has_value() in the AreWeMember() 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 correctly

The new ActiveDKGSession cleanly specializes CDKGSession for 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_bls once from DeploymentActiveAfter(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 via m_mn_activeman, which fits the new ActiveContext ownership model.
  • VerifyPendingContributions’s use of cs_pending, pendingContributionVerifications, and the various received* arrays is internally consistent and writes encrypted/verified contributions through dkgManager in the same places as before.
  • FinalizeCommitments and FinalizeSingleCommitment construct CFinalCommitment objects 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 MaybeDecrypt override correctly delegates to CActiveMasternodeManager::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 != nullptr for 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 control

The new ActiveDKGSessionHandler cleanly extends CDKGSessionHandler with 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) and cs_phase_qhash-guarded phase/quorumHash give a clear separation between external notifications (UpdatedBlockTip) and the internal phase loop (PhaseHandlerThread/HandleDKGRound).
  • Overridden GetPhase uses WITH_LOCK(cs_phase_qhash, …) and is annotated with EXCLUSIVE_LOCKS_REQUIRED(!cs_phase_qhash), matching the intended locking model.
  • StartPhaseFunc/WhileWaitFunc provide a flexible callback mechanism for the various Wait*/Handle* helpers.

If <functional> is not already pulled in by the included llmq/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 new ActiveDKGSession. While functionally correct, this creates and immediately destroys a CDKGSession object.

🔎 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 PhaseHandlerThread correctly catches AbortPhaseException for normal flow control (phase changes, shutdown requests) and continues looping. This is the intended behavior.

However, any other exception (e.g., std::runtime_error from 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 and catch(...) 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

📥 Commits

Reviewing files that changed from the base of the PR and between e376079 and 7f946a2.

📒 Files selected for processing (45)
  • src/Makefile.am
  • src/active/context.cpp
  • src/active/context.h
  • src/active/dkgsession.cpp
  • src/active/dkgsession.h
  • src/active/dkgsessionhandler.cpp
  • src/active/dkgsessionhandler.h
  • src/active/masternode.cpp
  • src/active/masternode.h
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/init.cpp
  • src/llmq/context.cpp
  • src/llmq/context.h
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsession.h
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsessionmgr.h
  • src/llmq/observer/context.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/notificationinterface.h
  • src/net_processing.cpp
  • src/net_processing.h
  • src/node/chainstate.cpp
  • src/node/chainstate.h
  • src/node/context.cpp
  • src/node/context.h
  • src/node/interfaces.cpp
  • src/rpc/coinjoin.cpp
  • src/rpc/governance.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/test/util/setup_common.cpp
  • src/test/util/setup_common.h
  • test/lint/lint-circular-dependencies.py
  • test/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.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/rpc/coinjoin.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/dkgsession.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/active/dkgsessionhandler.h
  • src/node/chainstate.cpp
  • src/active/context.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/active/dkgsession.h
  • src/active/masternode.cpp
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/active/masternode.h
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsessionmgr.h
  • src/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.cpp
  • src/node/context.cpp
  • src/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.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/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.cpp
  • src/governance/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/coinjoin/server.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/governance/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/coinjoin/server.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/active/masternode.cpp
  • src/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.cpp
  • src/net_processing.cpp
  • src/rpc/coinjoin.cpp
  • src/init.cpp
  • src/active/context.cpp
  • src/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.cpp
  • src/rpc/governance.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/active/masternode.cpp
  • src/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.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/dkgsession.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/active/dkgsessionhandler.h
  • src/active/dkgsession.h
  • src/active/masternode.cpp
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/active/masternode.h
  • src/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.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/rpc/coinjoin.cpp
  • src/active/dkgsession.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/active/dkgsession.h
  • src/active/masternode.cpp
  • src/llmq/dkgsession.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/llmq/dkgsession.h
  • src/active/context.h
  • src/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.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/active/masternode.cpp
  • src/active/context.h
  • src/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.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • src/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.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/node/context.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/masternode.cpp
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/node/chainstate.cpp
  • src/active/masternode.cpp
  • test/lint/lint-circular-dependencies.py
  • src/llmq/dkgsession.h
  • src/active/context.h
  • src/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.cpp
  • src/governance/signing.cpp
  • src/rpc/governance.cpp
  • src/active/quorums.cpp
  • src/active/masternode.cpp
  • src/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.cpp
  • src/coinjoin/server.cpp
  • src/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.cpp
  • src/rpc/governance.cpp
  • src/test/util/setup_common.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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.cpp
  • src/net_processing.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • 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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/rpc/governance.cpp
  • src/node/context.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/node/chainstate.cpp
  • src/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.cpp
  • src/node/context.cpp
  • src/rpc/masternode.cpp
  • src/rpc/coinjoin.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/node/chainstate.cpp
  • src/active/context.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/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.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/active/dkgsessionhandler.h
  • src/node/chainstate.cpp
  • src/active/context.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/node/context.cpp
  • src/rpc/masternode.cpp
  • src/coinjoin/server.cpp
  • src/rpc/quorums.cpp
  • src/rpc/coinjoin.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • 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/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

  • src/net_processing.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/active/dkgsessionhandler.h
  • src/node/chainstate.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/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.cpp
  • src/rpc/masternode.cpp
  • src/rpc/coinjoin.cpp
  • src/test/util/setup_common.cpp
  • src/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.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/rpc/quorums.cpp
  • src/active/dkgsession.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/active/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/active/dkgsession.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsessionmgr.h
  • src/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.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/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.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/quorums.cpp
  • src/active/context.h
  • src/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.cpp
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/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.cpp
  • src/coinjoin/server.cpp
  • src/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.cpp
  • src/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.h
  • src/llmq/context.h
  • src/active/dkgsession.h
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsession.h
  • src/active/context.h
  • src/active/masternode.h
  • src/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.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsession.h
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/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.cpp
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/observer/context.cpp
  • src/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:
Copy link
Collaborator

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

Copy link
Collaborator Author

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>>;
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

kwvg added 5 commits January 5, 2026 15:27
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
@kwvg kwvg requested a review from knst January 5, 2026 10:03
Copy link

@coderabbitai coderabbitai bot left a 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: Using m_active_ctx != nullptr as the masternode indicator is coherent across scheduling paths

You now derive is_masternode from m_active_ctx != nullptr and feed that into both:

  • RequestObjectCalculateObjectGetDataTime(..., /*is_masternode=*/m_active_ctx != nullptr, !state->fPreferredDownload)
  • ProcessMessage / SendMessages via a local const 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_ctx is 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 cleanup

Moving to m_outpoint, m_service, and m_protx_hash guarded by cs with inline READ-locked getters aligns with the mutex annotations and removes the need for a public info struct; keeping operator keys immutable via const members 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, and GetPubKey consistently operate on m_operator_sk/m_operator_pk under read locks and assert the mutex is not already held at the public API, which matches the new role of CActiveMasternodeManager as 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 nits

The 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; MaybeDecrypt cleanly delegates to CActiveMasternodeManager::Decrypt. There is some minor dead/unused work (signerIds/thresholdSigs and the unused sk1 pubkey 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 class final.

AbortPhaseException is a simple control-flow exception used locally. Adding final clarifies 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7f946a2 and 7ae88ee.

📒 Files selected for processing (34)
  • src/Makefile.am
  • src/active/context.cpp
  • src/active/context.h
  • src/active/dkgsession.cpp
  • src/active/dkgsessionhandler.cpp
  • src/active/masternode.cpp
  • src/active/masternode.h
  • src/active/quorums.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/governance/signing.cpp
  • src/init.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/notificationinterface.h
  • src/net_processing.cpp
  • src/net_processing.h
  • src/node/context.cpp
  • src/node/context.h
  • src/node/interfaces.cpp
  • src/rpc/coinjoin.cpp
  • src/rpc/governance.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/test/util/setup_common.cpp
  • src/test/util/setup_common.h
  • test/lint/lint-circular-dependencies.py
  • test/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.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/node/interfaces.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/active/dkgsession.cpp
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/context.cpp
  • src/active/masternode.cpp
  • src/active/context.h
  • src/active/dkgsessionhandler.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • src/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.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/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.cpp
  • src/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.cpp
  • src/llmq/dkgsession.cpp
  • src/evo/mnauth.cpp
  • src/coinjoin/server.cpp
  • src/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.cpp
  • src/llmq/dkgsession.cpp
  • src/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.cpp
  • src/llmq/dkgsession.cpp
  • src/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.cpp
  • src/llmq/dkgsession.cpp
  • src/evo/mnauth.cpp
  • src/coinjoin/server.cpp
  • src/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.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/active/masternode.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • 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/{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.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • src/net_processing.cpp
  • src/init.cpp
  • src/test/denialofservice_tests.cpp
  • 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}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

  • src/rpc/governance.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/active/dkgsession.cpp
  • src/Makefile.am
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/masternode.cpp
  • src/active/context.h
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • src/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.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • 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/{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.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • src/net_processing.cpp
  • src/init.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/context.h
  • src/coinjoin/server.cpp
  • src/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.cpp
  • src/node/interfaces.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/Makefile.am
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/active/masternode.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • 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/{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.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • test/lint/lint-circular-dependencies.py
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/active/masternode.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • src/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.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • 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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/rpc/governance.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • 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/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety

Applied to files:

  • src/rpc/governance.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/Makefile.am
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • src/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.cpp
  • src/node/context.cpp
  • src/init.cpp
  • src/test/denialofservice_tests.cpp
  • src/active/context.cpp
  • src/active/context.h
  • 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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

  • src/test/coinjoin_queue_tests.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/node/context.cpp
  • src/rpc/quorums.cpp
  • src/active/dkgsession.cpp
  • src/Makefile.am
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/evo/mnauth.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/active/masternode.cpp
  • src/active/context.h
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • 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/{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.cpp
  • src/net_processing.cpp
  • src/init.cpp
  • src/active/context.cpp
  • 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/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Applied to files:

  • src/node/context.cpp
  • src/init.cpp
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/Makefile.am
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/init.cpp
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/active/dkgsession.cpp
  • src/Makefile.am
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/init.cpp
  • src/active/context.h
  • src/active/dkgsessionhandler.cpp
  • src/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.cpp
  • src/evo/mnauth.cpp
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/init.cpp
  • src/active/context.h
  • src/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.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/llmq/dkgsession.cpp
  • src/net_processing.cpp
  • src/init.cpp
  • src/active/context.cpp
  • src/active/context.h
  • src/active/dkgsessionhandler.cpp
  • src/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.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/active/dkgsessionhandler.cpp
  • src/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.cpp
  • src/init.cpp
  • src/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.h
  • src/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.cpp
  • src/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.cpp
  • src/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

Comment on lines +1 to +61
// Copyright (c) 2025 The Dash Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <active/context.h>

#include <active/dkgsessionhandler.h>
#include <active/masternode.h>
#include <active/quorums.h>
#include <chainlock/chainlock.h>
#include <chainlock/signing.h>
#include <governance/governance.h>
#include <governance/signing.h>
#include <instantsend/instantsend.h>
#include <instantsend/signing.h>
#include <llmq/context.h>
#include <llmq/debug.h>
#include <llmq/dkgsessionmgr.h>
#include <llmq/ehf_signals.h>
#include <llmq/quorumsman.h>
#include <llmq/signing_shares.h>

#include <util/check.h>
#include <validation.h>

ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman,
CDeterministicMNManager& dmnman, CGovernanceManager& govman,
CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman,
CTxMemPool& mempool, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman,
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
PeerManager& peerman, const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
bool quorums_recovery, bool quorums_watch) :
m_clhandler{clhandler},
m_isman{isman},
m_qman{qman},
nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)},
dkgdbgman{std::make_unique<llmq::CDKGDebugManager>()},
qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)},
shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman.ActiveChainstate(), sigman, peerman, *nodeman,
qman, sporkman)},
gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)},
ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, mnhfman, sigman, *shareman, qman)},
qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman,
mn_sync, sporkman, sync_map, quorums_recovery, quorums_watch)},
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), clhandler, sigman, *shareman,
sporkman, mn_sync)},
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), clhandler, isman, sigman,
*shareman, qman, sporkman, mempool, mn_sync)}
{
qdkgsman->InitializeHandlers([&](const Consensus::LLMQParams& llmq_params,
int quorum_idx) -> std::unique_ptr<llmq::ActiveDKGSessionHandler> {
return std::make_unique<llmq::ActiveDKGSessionHandler>(bls_worker, dmnman, mn_metaman, *dkgdbgman, *qdkgsman,
qblockman, qsnapman, *nodeman, chainman, sporkman,
llmq_params, quorums_watch, quorum_idx);
});
m_clhandler.ConnectSigner(cl_signer.get());
m_isman.ConnectSigner(is_signer.get());
m_qman.ConnectManagers(qman_handler.get(), qdkgsman.get());
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix clang-format issues to satisfy CI

CI reports a Clang Diff Format Check failure for this file. Please run the project’s clang-format script (e.g. clang-format-diff.py over this hunk) to normalize indentation and spacing before merging.

🧰 Tools
🪛 Cppcheck (2.19.0)

[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)


[error] 24-24: #error No known always_inline attribute for this platform.

(preprocessorErrorDirective)

🪛 GitHub Actions: Clang Diff Format Check

[error] 1-1: Clang format differences found. Run the clang-format-diff.py script to fix formatting in this file.

🤖 Prompt for AI Agents
In @src/active/context.cpp around lines 1-61, The ActiveContext constructor
formatting in this file fails clang-format CI; run the project's clang-format
wrapper (e.g. clang-format-diff.py or the repo's format script) on the changed
hunk or on src/active/context.cpp to normalize indentation/spacing around the
ActiveContext constructor, member initializer list (m_clhandler, m_isman,
m_qman, nodeman, dkgdbgman, qdkgsman, shareman, gov_signer, ehf_sighandler,
qman_handler, cl_signer, is_signer) and the lambda passed to
qdkgsman->InitializeHandlers, then add the resulting formatted changes and push
the commit.

Comment on lines +324 to +334
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);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add null check for member in fallback verification loop.

At line 329, session.GetMember() is called again without a null check. While the earlier loop (lines 276-281) would have added the nodeId to ret if the member was null, session state could theoretically change. Adding a defensive check prevents a potential null pointer dereference.

🔎 Proposed fix
     for (const auto& [nodeId, msg] : messages) {
         if (ret.count(nodeId)) {
             continue;
         }

         auto member = session.GetMember(msg->proTxHash);
+        if (!member) {
+            ret.emplace(nodeId);
+            continue;
+        }
         bool valid = msg->sig.VerifyInsecure(member->dmn->pdmnState->pubKeyOperator.Get(), msg->GetSignHash());
         if (!valid) {
             ret.emplace(nodeId);
         }
     }
🤖 Prompt for AI Agents
In @src/active/dkgsessionhandler.cpp around lines 324-334, The fallback
verification loop iterates messages and calls session.GetMember(msg->proTxHash)
without checking for a null member, risking a null dereference when using
member->dmn->pdmnState->pubKeyOperator in msg->sig.VerifyInsecure; update the
loop that iterates messages (the for (const auto& [nodeId, msg] : messages)
block), add a null check after obtaining member from session.GetMember and treat
a null member the same as an invalid signature (e.g., emplace nodeId into ret
and continue) before calling msg->sig.VerifyInsecure.

Comment on lines 22 to +27
CDKGSessionHandler::CDKGSessionHandler(CBLSWorker& _blsWorker, CDeterministicMNManager& dmnman,
CDKGDebugManager& _dkgDebugManager, CDKGSessionManager& _dkgManager,
CMasternodeMetaMan& mn_metaman, CQuorumBlockProcessor& _quorumBlockProcessor,
CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager* const mn_activeman,
const ChainstateManager& chainman, const CSporkManager& sporkman,
CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman,
const Consensus::LLMQParams& _params, bool quorums_watch, int _quorumIndex) :
blsWorker{_blsWorker},
m_dmnman{dmnman},
dkgDebugManager{_dkgDebugManager},
dkgManager{_dkgManager},
m_mn_metaman{mn_metaman},
quorumBlockProcessor{_quorumBlockProcessor},
m_qsnapman{qsnapman},
m_mn_activeman{mn_activeman},
m_chainman{chainman},
m_sporkman{sporkman},
curSession{std::make_unique<CDKGSession>(_blsWorker, dmnman, _dkgDebugManager, _dkgManager, qsnapman, chainman,
/*pQuorumBaseBlockIndex=*/nullptr, _params)},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for CDKGSessionHandler constructor invocations (excluding the definition file)
rg -nP --type=cpp -C3 'CDKGSessionHandler\s*\(' -g '!src/llmq/dkgsessionhandler.cpp'

Repository: dashpay/dash

Length of output: 1767


🏁 Script executed:

#!/bin/bash
# Search for potential issues with nullptr pQuorumBaseBlockIndex usage in CDKGSession
rg -nP --type=cpp -C3 'pQuorumBaseBlockIndex' -g 'src/llmq/dkgsession.*'

Repository: dashpay/dash

Length of output: 1577


🏁 Script executed:

#!/bin/bash
# Examine CDKGSessionHandler constructor definition to verify parameters
cd src/llmq && head -40 dkgsessionhandler.cpp | cat -n

Repository: dashpay/dash

Length of output: 2171


🏁 Script executed:

#!/bin/bash
# Search for CDKGSessionHandler instantiation patterns across the codebase
rg -nP --type=cpp 'new CDKGSessionHandler|CDKGSessionHandler\s*\{' -g '!src/llmq/dkgsessionhandler.cpp'

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

#!/bin/bash
# Check CDKGSession constructor definition to understand nullptr parameter handling
fd -e cpp -e h 'dkgsession' src/llmq | head -5 | xargs grep -l 'class CDKGSession' | head -1 | xargs head -100 | cat -n

Repository: dashpay/dash

Length of output: 3196


🏁 Script executed:

#!/bin/bash
# Search for all CDKGSessionHandler instantiations across entire codebase
rg -nP --type=cpp 'CDKGSessionHandler' | grep -v 'dkgsessionhandler\.(cpp|h)' | grep -v '//'

Repository: dashpay/dash

Length of output: 2368


🏁 Script executed:

#!/bin/bash
# Check how m_quorum_base_block_index is used in CDKGSession class
rg -nP --type=cpp 'm_quorum_base_block_index' src/llmq/

Repository: dashpay/dash

Length of output: 4138


🏁 Script executed:

#!/bin/bash
# Search for nullptr checks or usage patterns of pQuorumBaseBlockIndex in CDKGSession
rg -nP --type=cpp -A5 -B5 'pQuorumBaseBlockIndex' src/llmq/dkgsession.cpp | head -60

Repository: dashpay/dash

Length of output: 811


Remove nullptr initialization; CDKGSession requires valid pQuorumBaseBlockIndex.

The constructor call sites in src/llmq/observer/context.cpp and src/active/dkgsessionhandler.cpp have been correctly updated to pass the new CQuorumSnapshotManager& qsnapman parameter. However, initializing CDKGSession with pQuorumBaseBlockIndex=nullptr is unsafe.

CDKGSession dereferences m_quorum_base_block_index without nullptr checks throughout its implementation (lines 33, 88, 116-118, 132-133, 160, 250, 290, 394, 521, 545, 624 in dkgsession.cpp). This will cause a null pointer dereference crash immediately upon construction.

Either provide a valid CBlockIndex pointer or add comprehensive nullptr guards before all dereferencing operations.

🧰 Tools
🪛 Cppcheck (2.19.0)

[error] 24-24: #error No known always_inline attribute for this platform.

(preprocessorErrorDirective)

🤖 Prompt for AI Agents
In @src/llmq/dkgsessionhandler.cpp around lines 22-27, CDKGSession is being
constructed with pQuorumBaseBlockIndex=nullptr which is unsafe because
CDKGSession dereferences m_quorum_base_block_index; replace the nullptr by
obtaining a valid CBlockIndex* from the available CQuorumSnapshotManager
(qsnapman) and pass that into CDKGSession in the CDKGSessionHandler constructor
(or, if no such accessor exists, add a method on CQuorumSnapshotManager to
return the appropriate quorum base CBlockIndex* and use it here); do not leave a
nullptr, or alternatively implement comprehensive nullptr checks inside
CDKGSession where m_quorum_base_block_index is dereferenced (references:
CDKGSessionHandler constructor, CDKGSession, m_quorum_base_block_index).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants