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/, introducing ActiveContext (now a CValidationInterface) and wiring it into NodeContext as node.active_ctx. It adds ActiveDKGSession and ActiveDKGSessionHandler, changes CDKGSession/CDKGSessionHandler and CDKGSessionManager APIs (handler factory/SessionHandlerKey), reorders several constructor parameter lists (LLMQContext, LLMQ components), refactors CActiveMasternodeManager internal state and constructor, updates many includes/Makefile exports, and replaces uses of node.mn_activeman with node.active_ctx across the codebase.

Sequence Diagram(s)

sequenceDiagram
    %% Colors for clarity (works in many renderers)
    participant Node as Node
    participant ActiveCtx as ActiveContext
    participant DKGMgr as CDKGSessionManager
    participant DKGH as ActiveDKGSessionHandler
    participant Peer as PeerManager
    participant Chain as ChainstateManager

    Note over Node,ActiveCtx: Startup wiring
    Node->>ActiveCtx: construct ActiveContext(deps...)
    ActiveCtx->>DKGMgr: InitializeHandlers(factory)
    DKGMgr-->>DKGH: factory(llmq_params, quorum_idx) → new ActiveDKGSessionHandler
    ActiveCtx->>Peer: SetCJServer / register managers

    Note over DKGH,Peer: DKG runtime (repeating per quorum/phase)
    loop DKG phases
        DKGH->>Peer: Relay/Send messages (Inv/Contrib/Complaint/Justify/Commit)
        Peer->>DKGH: Deliver messages via ProcessMessage
        DKGH->>DKGMgr: Update session state / submit commitments
        DKGMgr->>Chain: Query block tip / quorum base index
        DKGMgr->>ActiveCtx: NotifyRecoveredSig / UpdatedBlockTip
        ActiveCtx->>DKGH: Forward UpdatedBlockTip / coordinate signers
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • UdjinM6
  • knst
  • PastaPastaPasta

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% 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, DKG session isolation, and ActiveContext consolidation, which matches the changeset content.
Description check ✅ Passed The description provides context about dependencies, consolidation of ActiveNotificationInterface with ActiveContext, and expected changes without being off-topic or generic.
✨ 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 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

@UdjinM6
Copy link

UdjinM6 commented Jan 6, 2026

pls check commits in https://github.com/UdjinM6/dash/commits/refs/heads/pr-7065-attempt1/

kwvg and others added 6 commits January 6, 2026 23:02
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
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: 1

🤖 Fix all issues with AI Agents
In @src/active/dkgsessionhandler.cpp:
- Around line 319-329: The fallback verification loop calls
session.GetMember(msg->proTxHash) and dereferences member without a null check;
update the loop (the for (const auto& [nodeId, msg] : messages) block) to test
if member is null right after assignment and, if so, emplace nodeId into ret and
continue, before accessing member->dmn->pdmnState->pubKeyOperator or calling
msg->GetSignHash(), ensuring no dereference of a null member during fallback
verification.
🧹 Nitpick comments (4)
src/llmq/dkgsession.h (1)

353-383: Template method pattern requires complete overrides in derived class.

The virtual methods with empty default implementations establish a template method pattern where ActiveDKGSession is expected to override all these methods. Ensure that ActiveDKGSession provides complete implementations for all virtual methods, as the base class returns empty/default values.

Consider adding = 0 to make critical methods pure virtual, forcing derived classes to implement them and preventing accidental calls to empty base implementations:

-virtual std::vector<CFinalCommitment> FinalizeCommitments() EXCLUSIVE_LOCKS_REQUIRED(!invCs) { return {}; }
+virtual std::vector<CFinalCommitment> FinalizeCommitments() EXCLUSIVE_LOCKS_REQUIRED(!invCs) = 0;

However, if the base class is intentionally usable without these methods (e.g., for passive/observer nodes), the current approach is acceptable.

src/llmq/observer/context.cpp (1)

28-31: Consider empty capture for lambda clarity.

The lambda captures by reference ([&]) but doesn't use any captured variables. Consider using an empty capture list ([]) for clarity and to avoid unintended dependencies.

🔎 Proposed fix
-    qdkgsman->InitializeHandlers([&](const Consensus::LLMQParams& llmq_params,
+    qdkgsman->InitializeHandlers([](const Consensus::LLMQParams& llmq_params,
                                      [[maybe_unused]] int quorum_idx) -> std::unique_ptr<llmq::CDKGSessionHandler> {
         return std::make_unique<llmq::CDKGSessionHandler>(llmq_params);
     });
src/active/dkgsessionhandler.h (1)

45-68: Consider consolidating the two private: sections.

There are two separate private: sections (lines 45-57 and 59-68). While valid C++, consolidating them would improve readability.

Also note minor naming inconsistency: quorumIndex doesn't follow the m_ prefix pattern used by other members like m_bls_worker, m_dmnman, etc.

src/active/dkgsessionhandler.cpp (1)

47-47: Clarify the commented-out assertion.

The //AssertLockNotHeld(cs_main); is commented out. If this is intentional for this refactoring, consider adding a brief comment explaining why, or remove it if no longer applicable.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

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

📒 Files selected for processing (44)
  • 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/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/net_processing.h
  • src/masternode/active/context.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/node/context.h
  • src/test/util/setup_common.h
  • src/node/chainstate.h
  • src/masternode/active/notificationinterface.h
🚧 Files skipped from review as they are similar to previous changes (6)
  • src/governance/signing.cpp
  • src/node/interfaces.cpp
  • src/Makefile.am
  • test/lint/lint-circular-dependencies.py
  • src/active/quorums.cpp
  • src/node/chainstate.cpp
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

  • src/rpc/coinjoin.cpp
  • src/llmq/dkgsession.h
  • src/active/masternode.cpp
  • src/llmq/context.h
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/active/dkgsession.h
  • src/llmq/dkgsessionmgr.cpp
  • src/active/dkgsession.cpp
  • src/llmq/dkgsession.cpp
  • src/active/masternode.h
  • src/test/coinjoin_queue_tests.cpp
  • src/active/dkgsessionhandler.h
  • src/active/dkgsessionhandler.cpp
  • src/test/denialofservice_tests.cpp
  • src/node/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/active/context.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.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/dkgsession.h
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.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/dkgsession.h
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.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/dkgsession.h
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
src/{test,wallet/test}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Files:

  • src/test/coinjoin_queue_tests.cpp
  • src/test/denialofservice_tests.cpp
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Files:

  • src/node/context.cpp
src/coinjoin/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Files:

  • src/coinjoin/server.cpp
src/{masternode,evo}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Masternode lists must use immutable data structures (Immer library) for thread safety

Files:

  • src/evo/mnauth.cpp
src/evo/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)

Files:

  • src/evo/mnauth.cpp
🧠 Learnings (48)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/llmq/dkgsession.h
  • src/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/active/dkgsession.h
  • src/llmq/dkgsession.cpp
  • src/active/masternode.h
  • src/test/coinjoin_queue_tests.cpp
  • src/active/dkgsessionhandler.h
  • src/node/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/active/context.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-09-02T07:34:28.226Z
Learnt from: knst
Repo: dashpay/dash PR: 6834
File: test/functional/wallet_mnemonicbits.py:50-51
Timestamp: 2025-09-02T07:34:28.226Z
Learning: CJ (CoinJoin) descriptors with derivation path "9'/1" are intentionally inactive in descriptor wallets, while regular internal/external descriptors with different derivation paths remain active.

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/test/util/setup_common.cpp
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: In src/net_processing.cpp, if ActiveContext (m_active_ctx) is non-null, its members (including cj_server) are guaranteed to be initialized; call sites can safely dereference m_active_ctx->cj_server without an additional null-check.

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/init.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • 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}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/llmq/dkgsession.h
  • src/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/active/dkgsession.h
  • src/active/dkgsession.cpp
  • src/llmq/dkgsession.cpp
  • src/active/masternode.h
  • src/test/coinjoin_queue_tests.cpp
  • src/active/dkgsessionhandler.h
  • src/test/denialofservice_tests.cpp
  • src/node/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • 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/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/rpc/masternode.cpp
  • src/init.cpp
  • src/net_processing.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,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • src/node/context.cpp
  • src/init.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-10-03T11:20:37.475Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.

Applied to files:

  • src/rpc/coinjoin.cpp
  • src/llmq/context.h
  • src/rpc/masternode.cpp
  • src/llmq/context.cpp
  • src/test/denialofservice_tests.cpp
  • src/node/context.cpp
  • src/init.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/active/context.cpp
  • 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/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/active/dkgsession.h
  • src/llmq/dkgsessionmgr.cpp
  • src/active/dkgsession.cpp
  • src/llmq/dkgsession.cpp
  • src/active/dkgsessionhandler.h
  • src/active/dkgsessionhandler.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.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} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/test/util/setup_common.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} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/active/dkgsessionhandler.h
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • 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/llmq/dkgsession.h
  • src/llmq/context.h
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/dkgsession.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • src/active/context.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/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Applied to files:

  • src/llmq/dkgsession.h
  • src/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/node/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/test/util/setup_common.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.

Applied to files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/llmq/context.cpp
  • src/llmq/dkgsessionmgr.cpp
  • src/llmq/observer/context.cpp
  • src/active/context.h
  • src/active/context.cpp
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.

Applied to files:

  • src/llmq/dkgsession.h
  • src/llmq/context.h
  • src/active/dkgsession.h
  • src/active/masternode.h
  • src/active/dkgsessionhandler.h
  • src/llmq/dkgsessionhandler.h
  • src/active/context.h
📚 Learning: 2025-11-13T20:02:55.480Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6969
File: src/evo/deterministicmns.h:441-479
Timestamp: 2025-11-13T20:02:55.480Z
Learning: In `src/evo/deterministicmns.h`, the `internalId` field in `CDeterministicMN` and the `mnInternalIdMap` in `CDeterministicMNList` are non-deterministic and used only for internal bookkeeping and efficient lookups. Different nodes can assign different internalIds to the same masternode depending on their sync history. Methods like `IsEqual()` intentionally ignore internalId mappings and only compare consensus-critical deterministic fields (proTxHash, collateral, state, etc.).

Applied to files:

  • src/active/masternode.cpp
  • src/active/masternode.h
  • src/evo/mnauth.cpp
  • 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/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/active/masternode.h
  • src/node/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/net_processing.cpp
  • src/test/util/setup_common.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

  • src/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/test/coinjoin_queue_tests.cpp
  • src/test/denialofservice_tests.cpp
  • src/llmq/dkgsessionhandler.cpp
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.

Applied to files:

  • src/active/masternode.cpp
  • src/rpc/masternode.cpp
  • src/active/masternode.h
  • src/init.cpp
  • src/net_processing.cpp
  • src/test/util/setup_common.cpp
  • src/test/net_peer_connection_tests.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-05-10T00:54:15.597Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6629
File: src/evo/deterministicmns.cpp:473-485
Timestamp: 2025-05-10T00:54:15.597Z
Learning: In the current implementation of `NetInfoEntry`, only `CService` is a valid type and `std::monostate` represents an invalid state. Entries that don't provide a valid `CService` through `GetAddrPort()` should be rejected with an exception, not silently skipped.

Applied to files:

  • src/active/masternode.cpp
📚 Learning: 2025-06-16T17:59:55.669Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6665
File: src/evo/deterministicmns.cpp:1284-1287
Timestamp: 2025-06-16T17:59:55.669Z
Learning: In Dash masternode ProRegTx validation, platform ports (platformHTTPPort and platformP2PPort) are mandatory and must be non-zero, while netInfo (ipAndPort) is optional. This means that even if an empty netInfo returns 0 from GetPrimary().GetPort(), it won't cause false positives in port duplication checks since platform ports cannot be 0.

Applied to files:

  • src/active/masternode.cpp
📚 Learning: 2025-07-09T15:02:26.899Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6729
File: src/evo/deterministicmns.cpp:1313-1316
Timestamp: 2025-07-09T15:02:26.899Z
Learning: In Dash's masternode transaction validation, `IsVersionChangeValid()` is only called by transaction types that update existing masternode entries (like `ProUpServTx`, `ProUpRegTx`, `ProUpRevTx`), not by `ProRegTx` which creates new entries. This means validation logic in `IsVersionChangeValid()` only applies to the subset of transaction types that actually call it, not all masternode transaction types.

Applied to files:

  • src/active/masternode.cpp
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.

Applied to files:

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

Applied to files:

  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/node/context.cpp
  • src/net_processing.cpp
  • src/coinjoin/server.cpp
  • src/evo/mnauth.cpp
  • src/llmq/signing_shares.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
Repo: dashpay/dash PR: 6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.

Applied to files:

  • src/rpc/masternode.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/rpc/masternode.cpp
  • src/rpc/quorums.cpp
  • src/llmq/context.cpp
  • src/node/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/test/util/setup_common.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.

Applied to files:

  • src/rpc/masternode.cpp
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.

Applied to files:

  • src/rpc/quorums.cpp
  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing

Applied to files:

  • src/rpc/quorums.cpp
  • src/test/denialofservice_tests.cpp
  • src/node/context.cpp
  • src/llmq/dkgsessionhandler.h
  • src/init.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • src/test/util/setup_common.cpp
  • 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/**/*.{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/coinjoin/server.cpp
  • src/evo/mnauth.cpp
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.

Applied to files:

  • src/rpc/quorums.cpp
  • src/net_processing.cpp
  • src/active/context.h
  • 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
  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Applied to files:

  • src/test/coinjoin_queue_tests.cpp
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Applied to files:

  • src/active/dkgsessionhandler.cpp
  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Applied to files:

  • src/active/dkgsessionhandler.cpp
  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/test/util/setup_common.cpp
  • src/active/context.cpp
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
  • src/coinjoin/server.cpp
  • src/llmq/signing_shares.cpp
📚 Learning: 2025-07-23T09:30:34.631Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-07-17T15:48:29.418Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-07-29T14:33:01.040Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-10-13T12:37:12.357Z
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Applied to files:

  • src/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-12-02T12:59:28.247Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7026
File: src/rpc/evo.cpp:1713-1799
Timestamp: 2025-12-02T12:59:28.247Z
Learning: In the Dash codebase, CChain::operator[] has built-in safe bounds checking. Accessing chainman.ActiveChain()[height] is safe even if height exceeds the current chain height, as the operator will handle bounds checking internally. There is no need to manually check chain height before using the [] operator on CChain.

Applied to files:

  • src/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

  • src/test/util/setup_common.cpp
  • src/rpc/governance.cpp
📚 Learning: 2025-02-14T15:15:58.165Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.

Applied to files:

  • src/active/context.cpp
📚 Learning: 2025-07-09T15:05:36.250Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.

Applied to files:

  • src/active/context.cpp
🧬 Code graph analysis (19)
src/llmq/dkgsession.h (5)
src/active/dkgsessionhandler.h (2)
  • llmq (31-37)
  • Consensus (28-30)
src/active/dkgsession.cpp (4)
  • ActiveDKGSession (23-34)
  • ActiveDKGSession (36-36)
  • MaybeDecrypt (713-717)
  • MaybeDecrypt (713-714)
src/llmq/dkgsession.cpp (2)
  • CDKGSession (68-82)
  • CDKGSession (84-84)
src/llmq/dkgsessionhandler.cpp (2)
  • CDKGSessionHandler (14-25)
  • CDKGSessionHandler (27-27)
src/llmq/dkgsessionhandler.h (1)
  • Consensus (29-31)
src/active/masternode.cpp (1)
src/active/masternode.h (1)
  • cs (40-75)
src/rpc/masternode.cpp (1)
src/interfaces/node.h (2)
  • node (48-50)
  • node (481-481)
src/rpc/quorums.cpp (1)
src/interfaces/chain.h (1)
  • node (40-42)
src/llmq/context.cpp (1)
src/node/interfaces.cpp (3)
  • chainman (754-756)
  • chainman (1239-1241)
  • chainman (1309-1312)
src/active/dkgsession.h (1)
src/llmq/dkgsession.h (4)
  • llmq (30-39)
  • llmq (41-67)
  • CDKGSession (276-409)
  • CFinalCommitment (383-383)
src/active/dkgsession.cpp (1)
src/llmq/dkgsession.cpp (4)
  • ShouldSimulateError (52-59)
  • ShouldSimulateError (52-52)
  • MarkBadMember (686-697)
  • MarkBadMember (686-686)
src/llmq/dkgsession.cpp (2)
src/llmq/dkgsession.h (1)
  • CDKGSession (276-409)
src/active/dkgsession.cpp (2)
  • MaybeDecrypt (713-717)
  • MaybeDecrypt (713-714)
src/active/masternode.h (1)
src/active/masternode.cpp (9)
  • CActiveMasternodeManager (52-63)
  • CActiveMasternodeManager (65-65)
  • nodiscard (266-271)
  • nodiscard (277-281)
  • nodiscard (283-289)
  • nodiscard (293-297)
  • GetPubKey (293-293)
  • GetLocalAddress (217-255)
  • GetLocalAddress (217-217)
src/test/coinjoin_queue_tests.cpp (1)
src/test/util/setup_common.cpp (2)
  • TestingSetup (312-395)
  • TestingSetup (397-422)
src/active/dkgsessionhandler.h (3)
src/llmq/dkgsessionhandler.h (1)
  • Consensus (29-31)
src/active/dkgsessionhandler.cpp (10)
  • ActiveDKGSessionHandler (22-41)
  • ActiveDKGSessionHandler (43-43)
  • GetContribution (528-540)
  • GetContribution (528-528)
  • GetComplaint (542-554)
  • GetComplaint (542-542)
  • GetJustification (556-568)
  • GetJustification (556-556)
  • GetPrematureCommitment (570-582)
  • GetPrematureCommitment (570-570)
src/llmq/dkgsessionhandler.cpp (2)
  • CDKGSessionHandler (14-25)
  • CDKGSessionHandler (27-27)
src/active/dkgsessionhandler.cpp (2)
src/llmq/dkgsessionmgr.cpp (10)
  • UpdatedBlockTip (60-74)
  • UpdatedBlockTip (60-60)
  • GetContribution (187-202)
  • GetContribution (187-187)
  • GetComplaint (204-219)
  • GetComplaint (204-204)
  • GetJustification (221-236)
  • GetJustification (221-221)
  • GetPrematureCommitment (238-253)
  • GetPrematureCommitment (238-238)
src/llmq/debug.h (1)
  • phase (60-60)
src/test/denialofservice_tests.cpp (1)
src/test/util/setup_common.cpp (2)
  • MakePeerManager (129-138)
  • MakePeerManager (129-133)
src/llmq/dkgsessionhandler.h (2)
src/active/dkgsessionhandler.h (1)
  • Consensus (28-30)
src/llmq/options.h (1)
  • Consensus (18-21)
src/net_processing.cpp (2)
src/evo/mnauth.cpp (2)
  • PushMNAUTH (19-49)
  • PushMNAUTH (19-19)
src/llmq/signing_shares.cpp (2)
  • ProcessMessage (255-332)
  • ProcessMessage (255-255)
src/active/context.h (1)
src/active/context.cpp (12)
  • ActiveContext (26-61)
  • ActiveContext (63-68)
  • Start (75-83)
  • Start (75-75)
  • GetCJServer (95-98)
  • GetCJServer (95-95)
  • SetCJServer (100-105)
  • SetCJServer (100-100)
  • NotifyRecoveredSig (119-122)
  • NotifyRecoveredSig (119-119)
  • UpdatedBlockTip (107-117)
  • UpdatedBlockTip (107-107)
src/test/util/setup_common.cpp (1)
src/node/interfaces.cpp (8)
  • Assert (197-197)
  • Assert (198-198)
  • Assert (223-223)
  • Assert (364-364)
  • Assert (387-387)
  • Assert (520-520)
  • Assert (791-794)
  • Assert (1003-1003)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
  • MakePeerManager (129-138)
  • MakePeerManager (129-133)
src/active/context.cpp (1)
src/active/context.h (1)
  • m_cj_server (105-105)
🪛 Cppcheck (2.19.0)
src/rpc/masternode.cpp

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

(syntaxError)

src/rpc/quorums.cpp

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

(preprocessorErrorDirective)

src/active/dkgsession.cpp

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

(syntaxError)


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

(preprocessorErrorDirective)

src/test/coinjoin_queue_tests.cpp

[error] 15-15: There is an unknown macro here somewhere. Configuration is required. If BOOST_FIXTURE_TEST_SUITE is a macro then please configure it.

(unknownMacro)


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

(syntaxError)

src/active/dkgsessionhandler.cpp

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

(syntaxError)


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

(preprocessorErrorDirective)

src/llmq/observer/context.cpp

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

(preprocessorErrorDirective)

src/llmq/dkgsessionhandler.cpp

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

(syntaxError)

src/active/context.cpp

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

(syntaxError)


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

(preprocessorErrorDirective)

src/coinjoin/server.cpp

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

(syntaxError)

src/evo/mnauth.cpp

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

(syntaxError)

src/llmq/signing_shares.cpp

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

(syntaxError)

src/rpc/governance.cpp

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

(syntaxError)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: Lint / Run linters

Comment on lines +319 to +329
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

Missing null check for member in fallback verification loop.

At line 324, session.GetMember(msg->proTxHash) is called but the result is not checked for null before dereferencing at line 325. While the earlier loop (lines 271-276) adds nodeId to ret if member is null, this fallback loop could still receive messages that weren't processed in the first loop due to revertToSingleVerification being set.

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

         auto member = session.GetMember(msg->proTxHash);
+        if (!member) {
+            ret.emplace(nodeId);
+            continue;
+        }
         bool valid = msg->sig.VerifyInsecure(member->dmn->pdmnState->pubKeyOperator.Get(), msg->GetSignHash());
         if (!valid) {
             ret.emplace(nodeId);
         }
     }
🤖 Prompt for AI Agents
In @src/active/dkgsessionhandler.cpp around lines 319 - 329, The fallback
verification loop calls session.GetMember(msg->proTxHash) and dereferences
member without a null check; update the loop (the for (const auto& [nodeId, msg]
: messages) block) to test if member is null right after assignment and, if so,
emplace nodeId into ret and continue, before accessing
member->dmn->pdmnState->pubKeyOperator or calling msg->GetSignHash(), ensuring
no dereference of a null member during fallback verification.

@kwvg kwvg requested review from UdjinM6 and removed request for UdjinM6 January 6, 2026 18:12
@knst
Copy link
Collaborator

knst commented Jan 7, 2026

If Dash Core is created only with consensus code (as it is required by project "kernel" which is being backported from bitcoin, to support utxo snapshots), we don't need CDKGSession, so far as it is purely network code; whatever it's active CDKGSession as a member, or it's just a watcher.

On other hand, if Dash Core is created with network code (not only consensus), so, it can participate in any DKGSession as a watch-only member anyway, and this separation (as done in PR) is not required; it just over-complicates implementation, creates multiple virtual functions, etc. Beside performance degradation, there is significant code move, files renaming, etc.

To separate network & consensus code this PR doesn't bring anything new but a new circular dependency active/context -> llmq/signing_shares -> net_processing -> active/context

Please, consider instead this commit: 91f8f9b (this commit ID is updated, v2, due to compiler error) because everything to isolate CActiveMasternodeManager is already done in these 3 PRs: #7070 #7063 #7062


I am going to attempt 619f8a2 (Merge bitcoin#24304): [kernel 0/n] Introduce bitcoin-chainstate
to see what is a real blockers at the moment because many of them already gone by now, thanks for kwvg's hard work.

@kwvg
Copy link
Collaborator Author

kwvg commented Jan 7, 2026

It's true that this change doesn't directly contribute to dash-chainstate but it seems odd to leave this dual path logic as-is when everything else has been bifurcated. This PR would complete the watch-only and masternode mode separation, which means the code paths being evaluated are for the most part independent of each other, particularly valuable since unit tests right now cover neither watch-only nor masternode mode.

To work on unit tests/fuzzing, completing the bifurcation is in the better interest. Plus, if ObserverContext can be freed from the knowledge of CActiveMasternodeManager, that would be wonderful. The idea is that while the primary intent was to move along work on dash-chainstate, it was also an isolation project that it seems would benefit from being seen to completion.

Fundamentally the circular dependency is a product of the llmq/signing_shares -> net_processing chain that are present for 8 chains. Breaking that chain would be part of NetHandler's scope, as the changes in this PR are behavior-identical, even if restructured, the most that has changed is the reduction of dependencies in ObserverContext, not the creation of new ones.

There is overhead but two things come into play, firstly, this is not running on all nodes, only nodes observing/participating and in terms of CPU cycles, vtable lookups are relatively cheap compared to heavy consensus primitives and the inheritance structure is unlikely to cause slowdowns, especially since we don't expect rapid session churn.

@knst

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK de4d0ea

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK de4d0ea

@PastaPastaPasta PastaPastaPasta merged commit 7482956 into dashpay:develop Jan 8, 2026
42 of 48 checks passed
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.

4 participants