Skip to content

Conversation

@kwvg
Copy link
Collaborator

@kwvg kwvg commented Dec 13, 2025

Additional Information

  • Depends on refactor(rpc): add watch-only/masternode mode restrictions for quorum dkg{info,status}, move CDKGDebugManager to {Active,Observer}Context #7062

  • Dependency for refactor: extract CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation) #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

  • 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

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@github-actions
Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request Dec 28, 2025
…ns for `quorum dkg{info,status}`, move `CDKGDebugManager` to `{Active,Observer}Context`

e98fe4f doc: add release notes for breaking changes (Kittywhiskers Van Gogh)
f5a1617 refactor: move `CDKGDebugManager` to `{Active,Observer}Context` (Kittywhiskers Van Gogh)
9d09f91 rpc: add watch-only/masternode mode limits for `quorum dkg{info,status}` (Kittywhiskers Van Gogh)
d9fe923 test: only call `quorum dkg{info,status}` on nodes that can see DKG (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on #7056

  * Depends on #7061

  * Dependency for #7063

  ## Breaking Changes

  See release notes.

  ## Checklist

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

ACKs for top commit:
  UdjinM6:
    utACK e98fe4f

Tree-SHA512: c1430f8b9714ccd000552bd4116d2d339d483d6bfa206c6a01973acaf36da86639afc88342f6d1f80f738929c3f5b6068eee38d36cb5964765dcc6175a998d4d
@kwvg kwvg marked this pull request as ready for review December 29, 2025 11:58
@coderabbitai
Copy link

coderabbitai bot commented Dec 29, 2025

Walkthrough

Splits and reorganizes LLMQ/quorum responsibilities: introduces llmq::CQuorumManager (llmq/quorumsman.{h,cpp}) for mined-quorum lifecycle, caching, data exchange and signing; adds llmq::QuorumObserver (llmq/observer/quorums.{h,cpp}) for block-tip monitoring, connection maintenance, data-recovery and cleanup; and adds llmq::QuorumParticipant (active/quorums.{h,cpp}) for active masternode participation and contribution handling. Moves quorum helpers/constants into llmq/quorums.h, updates many includes to llmq/quorumsman.h, and adjusts constructor signatures and startup/shutdown wiring across ActiveContext, ObserverContext and LLMQContext. Build exports updated for new headers/sources.

Sequence Diagram(s)

mermaid
sequenceDiagram
autonumber
participant Block as Blockchain (BlockIndex)
participant Observer as llmq::QuorumObserver
participant Manager as llmq::CQuorumManager
participant Participant as llmq::QuorumParticipant
participant Net as CConnman
Note over Block,Observer: New block tip or reorg
Block->>Observer: UpdatedBlockTip(pindexNew, fInitialDownload)
alt Observer needs quorum info
Observer->>Manager: ScanQuorums() / GetQuorumsToDelete()
Manager-->>Observer: quorum list / metadata
Observer->>Observer: CheckQuorumConnections()
Observer->>Manager: CleanupExpiredDataRequests()
end
alt Data recovery required
Observer->>Participant: TriggerQuorumDataRecoveryThreads()
Participant->>Net: RequestQuorumData(QGETDATA) to peers
Net-->>Participant: QDATA / encrypted contributions
Participant->>Manager: GetEncryptedContributions() / SetQuorumSecretKeyShare()
Manager-->>Participant: Acknowledge / QueueQuorumForWarming()
end

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • knst
  • UdjinM6
  • PastaPastaPasta

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.06% 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 describes the main change: extracting CActiveMasternodeManager from LLMQContext and separating CQuorumManager handler logic.
Description check ✅ Passed The description is well-related to the changeset, providing context about the refactoring, behavior splits, threading logic, and architectural changes.
✨ 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: 1

🧹 Nitpick comments (4)
src/active/quorums.cpp (2)

87-107: Recovery start offset computation is safe but relies on our ProTxHash being present

The modulo is safe because quorum.qc->validMembers.size() is always > 0 for configured LLMQs, and the sorted MN list lookup gives a deterministic per‑member offset. If our GetProTxHash() were ever missing from the list at pIndex, this would silently fall back to offset 0, which is benign but potentially surprising.

If you expect GetProTxHash() to always be present here in masternode mode, consider adding a debug assertion or log in the “not found” path to ease future diagnosis without changing runtime behaviour.


109-167: Encrypted contribution handling mirrors prior QGETDATA/QDATA logic with clear error paths

The unified ProcessEncryptedContribs implementation:

  • Validates quorum membership and presence of encrypted contributions for QGETDATA, setting the appropriate CQuorumDataRequest::Errors and only misbehaving when the caller has exceeded the request limit.
  • For QDATA, requires a full quorum vvec, validates membership again, then decrypts and aggregates contributions before calling SetSecretKeyShare, with precise MisbehavingError reasons for decryption or key‑share failures.

This keeps behaviour aligned with previous code while making the masternode‑only side explicit.

You could optionally route both the QDATA aggregation path and SetQuorumSecretKeyShare() through a single helper to avoid duplication of the AggregateSecretKeys+SetSecretKeyShare pattern.

src/Makefile.am (1)

154-155: New active/llmq quorum sources and headers are wired correctly; add to non-backported list.

The additions of active/quorums.{h,cpp}, llmq/quorumsman.{h,cpp}, and llmq/observer/quorums.{h,cpp} to BITCOIN_CORE_H and libbitcoin_node_a_SOURCES look consistent with how other public llmq/masternode modules are exposed.

One follow-up: since these are new, non‑Bitcoin backport files, they should also be listed in test/util/data/non-backported.txt to keep the non‑backported tracking accurate. Based on learnings, this is expected for newly added Dash‑only code.

Also applies to: 281-283, 291-292, 485-486, 555-557, 562-564

src/init.cpp (1)

2209-2212: Quorum observer/participant wiring and NetInstantSend/NetSigning integration are consistent.

  • The new quorums_recovery, quorums_watch, and sync_map values are computed once in AppInitMain() and passed consistently into ActiveContext (masternode) or ObserverContext (watch‑only). This matches the earlier parameter‑interaction validation using GetEnabledQuorumVvecSyncEntries(args).
  • Exactly one of active_ctx or observer_ctx is constructed (mn_activeman vs quorums_watch), so CQuorumManager::ConnectManagers() is only called from a single context.
  • The added NetInstantSend and NetSigning handlers hang off peerman and are created after llmq_ctx is initialized and before peerman->StartHandlers(), and are torn down when peerman is reset, so their references to isman, qman, and sigman stay valid.
  • In Step 10a, observer_ctx->Start() is invoked after llmq_ctx->Start() and before scheduler shutdown, symmetric with PrepareShutdown()’s observer_ctx->Stop().

Overall the lifecycle and dependency ordering here looks correct.

Also applies to: 2223-2227, 2231-2233, 2341-2342

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d69043 and 25c87e4.

📒 Files selected for processing (39)
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/chainlock/chainlock.cpp
  • src/chainlock/chainlock.h
  • src/dsnotificationinterface.cpp
  • src/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.cpp
  • src/init.cpp
  • src/instantsend/net_instantsend.cpp
  • src/instantsend/signing.cpp
  • src/llmq/context.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/options.h
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
  • src/llmq/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/msg_result.h
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/node/chainstate.h
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
  • test/util/data/non-backported.txt
💤 Files with no reviewable changes (1)
  • src/node/chainstate.h
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/evo/assetlocktx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/chainlock/chainlock.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.h
  • src/msg_result.h
  • src/llmq/options.h
  • src/net_processing.cpp
  • src/test/util/setup_common.cpp
  • src/llmq/context.h
  • src/init.cpp
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
src/{masternode,evo}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/evo/smldiff.cpp
  • src/masternode/active/context.cpp
  • src/evo/specialtxman.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/assetlocktx.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/options.h
  • src/llmq/context.h
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.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/assetlocktx.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/options.h
  • src/llmq/context.h
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
src/{masternode,llmq}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

BLS integration must be used for cryptographic foundation of advanced masternode features

Files:

  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/options.h
  • src/llmq/context.h
  • src/llmq/observer/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/options.h
  • src/llmq/context.h
  • src/llmq/observer/quorums.h
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
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/util/setup_common.cpp
src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/node/chainstate.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 (34)
📓 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: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
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.
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
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.
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.
📚 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/evo/assetlocktx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.h
  • src/llmq/options.h
  • src/net_processing.cpp
  • src/test/util/setup_common.cpp
  • src/llmq/context.h
  • src/init.cpp
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

  • src/evo/assetlocktx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.h
  • src/llmq/options.h
  • src/net_processing.cpp
  • src/test/util/setup_common.cpp
  • src/llmq/context.h
  • src/init.cpp
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/evo/assetlocktx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/context.h
  • src/chainlock/chainlock.h
  • src/net_processing.cpp
  • src/llmq/context.h
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/evo/smldiff.cpp
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/observer/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/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

  • src/evo/assetlocktx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • test/util/data/non-backported.txt
  • src/evo/smldiff.cpp
  • src/instantsend/signing.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/assetlocktx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/chainlock/chainlock.cpp
  • src/evo/mnhftx.cpp
  • src/masternode/active/notificationinterface.cpp
  • test/util/data/non-backported.txt
  • src/evo/smldiff.cpp
  • src/instantsend/signing.cpp
  • src/evo/specialtxman.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/evo/assetlocktx.cpp
  • src/llmq/signing_shares.cpp
  • src/Makefile.am
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/evo/cbtx.cpp
  • src/Makefile.am
  • src/instantsend/net_instantsend.cpp
  • src/evo/mnhftx.cpp
  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/net_processing.cpp
  • src/llmq/context.h
  • src/init.cpp
  • src/evo/smldiff.cpp
  • src/llmq/observer/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/evo/specialtxman.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/cbtx.cpp
  • src/Makefile.am
  • src/evo/mnhftx.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
  • src/init.cpp
  • src/evo/smldiff.cpp
  • src/llmq/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
📚 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
  • test/util/data/non-backported.txt
📚 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-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/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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

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/{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/chainlock/chainlock.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/evo/smldiff.cpp
  • src/instantsend/signing.cpp
  • src/evo/specialtxman.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: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.

Applied to files:

  • src/chainlock/chainlock.cpp
  • src/instantsend/signing.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/instantsend/net_instantsend.cpp
  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/llmq/context.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.h
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.

Applied to files:

  • src/dsnotificationinterface.cpp
  • src/masternode/active/context.h
  • src/llmq/context.h
  • src/llmq/observer/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/llmq/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
📚 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/masternode/active/context.h
  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/masternode/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/masternode/active/context.h
  • src/chainlock/chainlock.h
  • src/msg_result.h
  • src/llmq/options.h
  • src/llmq/context.h
  • src/llmq/observer/quorums.h
  • src/llmq/quorums.h
  • src/active/quorums.h
  • src/llmq/observer/context.h
  • src/llmq/quorumsman.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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

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

Applied to files:

  • src/test/util/setup_common.cpp
  • src/llmq/quorums.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/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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/test/util/setup_common.cpp
  • src/instantsend/signing.cpp
  • src/node/chainstate.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Applied to files:

  • src/test/util/setup_common.cpp
  • src/init.cpp
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.

Applied to files:

  • test/util/data/non-backported.txt
📚 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/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

  • test/util/data/non-backported.txt
📚 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:

  • test/util/data/non-backported.txt
📚 Learning: 2025-08-10T13:52:46.289Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6666
File: test/functional/rpc_netinfo.py:73-81
Timestamp: 2025-08-10T13:52:46.289Z
Learning: In the Bitcoin/Dash test framework, `self.node.extra_args` contains the original startup arguments for a TestNode and is not modified by `test.restart_node()`. Each restart with new `extra_args` doesn't update the stored `self.node.extra_args`, so copying from it always provides a clean slate without accumulated modifications from previous restarts.

Applied to files:

  • src/init.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/init.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/masternode/active/context.cpp
🧬 Code graph analysis (10)
src/masternode/active/context.h (1)
src/active/quorums.cpp (2)
  • QuorumParticipant (27-37)
  • QuorumParticipant (39-39)
src/masternode/active/notificationinterface.cpp (1)
src/net_processing.cpp (1)
  • pindexNew (607-607)
src/test/util/setup_common.cpp (1)
src/llmq/options.h (1)
  • DEFAULT_BLSCHECK_THREADS (33-33)
src/llmq/observer/quorums.h (3)
src/llmq/observer/context.h (2)
  • llmq (22-29)
  • llmq (34-64)
src/llmq/observer/quorums.cpp (24)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
  • UpdatedBlockTip (61-75)
  • UpdatedBlockTip (61-61)
  • SetQuorumSecretKeyShare (127-131)
  • SetQuorumSecretKeyShare (127-127)
  • ProcessEncryptedContribs (133-151)
  • ProcessEncryptedContribs (133-137)
  • IsMasternode (153-157)
  • IsMasternode (153-153)
  • IsWatching (159-163)
  • IsWatching (159-159)
  • CheckQuorumConnections (106-125)
  • CheckQuorumConnections (106-107)
  • TriggerQuorumDataRecoveryThreads (286-299)
  • TriggerQuorumDataRecoveryThreads (286-286)
  • DataRecoveryThread (165-270)
  • DataRecoveryThread (165-166)
  • StartCleanupOldQuorumDataThread (319-372)
  • StartCleanupOldQuorumDataThread (319-319)
src/llmq/observer/context.cpp (6)
  • Start (35-38)
  • Start (35-35)
  • Stop (40-43)
  • Stop (40-40)
  • UpdatedBlockTip (45-52)
  • UpdatedBlockTip (45-45)
src/llmq/observer/context.cpp (1)
src/llmq/observer/quorums.cpp (4)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
src/llmq/quorums.h (4)
src/active/quorums.h (1)
  • llmq (30-35)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/context.h (1)
  • llmq (22-29)
src/evo/mnhftx.h (1)
  • llmq (30-32)
src/llmq/observer/quorums.cpp (3)
src/llmq/quorums.h (1)
  • nDataMask (87-87)
src/llmq/observer/quorums.h (1)
  • m_quorums_recovery (76-127)
src/llmq/quorums.cpp (2)
  • MakeQuorumKey (18-27)
  • MakeQuorumKey (18-18)
src/llmq/observer/context.h (2)
src/llmq/observer/quorums.cpp (8)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
  • UpdatedBlockTip (61-75)
  • UpdatedBlockTip (61-61)
src/llmq/observer/context.cpp (8)
  • ObserverContext (13-28)
  • ObserverContext (30-33)
  • Start (35-38)
  • Start (35-35)
  • Stop (40-43)
  • Stop (40-40)
  • UpdatedBlockTip (45-52)
  • UpdatedBlockTip (45-45)
src/llmq/quorumsman.cpp (4)
src/llmq/quorums.h (2)
  • llmq (25-30)
  • llmq (226-226)
src/active/quorums.h (1)
  • llmq (30-35)
src/llmq/observer/context.h (2)
  • llmq (22-29)
  • llmq (34-64)
src/llmq/quorums.cpp (5)
  • DataCleanupHelper (29-74)
  • DataCleanupHelper (29-29)
  • batch (33-33)
  • s (196-196)
  • s (211-211)
src/llmq/quorumsman.h (4)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/quorums.h (2)
  • llmq (25-30)
  • llmq (226-226)
src/llmq/quorumsman.cpp (20)
  • CQuorumManager (32-46)
  • CQuorumManager (48-54)
  • GetQuorum (368-389)
  • GetQuorum (368-368)
  • GetQuorum (391-407)
  • GetQuorum (391-391)
  • ScanQuorums (193-197)
  • ScanQuorums (193-193)
  • ScanQuorums (199-307)
  • ScanQuorums (199-201)
  • IsMasternode (309-315)
  • IsMasternode (309-309)
  • IsWatching (317-323)
  • IsWatching (317-317)
  • BuildQuorumFromCommitment (66-112)
  • BuildQuorumFromCommitment (66-66)
  • SelectQuorumForSigning (675-735)
  • SelectQuorumForSigning (675-676)
  • VerifyRecoveredSig (737-751)
  • VerifyRecoveredSig (737-739)
src/llmq/params.h (1)
  • LLMQType (14-125)
🪛 Cppcheck (2.19.0)
src/masternode/active/context.cpp

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

(preprocessorErrorDirective)

src/llmq/observer/context.cpp

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

(syntaxError)

src/llmq/quorums.cpp

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

(syntaxError)

src/active/quorums.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/quorums.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/context.cpp

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

(preprocessorErrorDirective)

src/llmq/quorumsman.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)

⏰ 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). (1)
  • GitHub Check: linux64_tsan-test / Test source

PastaPastaPasta added a commit that referenced this pull request Dec 31, 2025
41b9283 refactor: inline error handler in `CQuorumManager::ProcessMessage()` (Kittywhiskers Van Gogh)
caa5400 refactor: move `mapQuorumDataRequests` to `CQuorumManager`, use `Mutex` (Kittywhiskers Van Gogh)

Pull request description:

  ## Issue being fixed or feature implemented

  ## What was done?
  extracted two commits from: #7063

  ## How Has This Been Tested?

  ## Breaking Changes
  None

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] 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
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 41b9283

Tree-SHA512: 1ec93540031be1006ceefd4a2f1445f2edc089958bc8dd6dbefda689bd16ff21804b7bc1a7bbe7bae228a12824c898d43c38a8664100634469bf117349c02b05
@github-actions
Copy link

This pull request has conflicts, please rebase.

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 (3)
src/llmq/quorums.cpp (1)

14-32: DB key helpers centralize quorum contribution storage; minor batch nit

Moving DB_QUORUM_SK_SHARE / DB_QUORUM_QUORUM_VVEC into llmq and exposing MakeQuorumKey(const CQuorum&) keeps all contribution keying logic in one place and makes it usable from CQuorumManager/migration code. The key now hashes {llmqType, quorumHash, members[i].proTxHash} which should be stable per quorum and per node.

DataCleanupHelper() correctly walks both prefixes, skips the provided skip_list, and erases the rest in batches with an optional CompactFull() pass for migrations. One very small improvement you might consider (not urgent):

  • Clear batch after the final db.WriteBatch(batch); per prefix to avoid re‑emitting already‑applied erase ops when processing the second prefix.

Functionally everything is fine; this would just trim a bit of redundant I/O.

Also applies to: 29-75

src/llmq/quorumsman.cpp (2)

484-496: The const_cast is documented but warrants attention.

The TODO comment on line 486 acknowledges that const_cast on pQuorum is a code smell. While the current implementation doesn't actually write to the quorum object in ProcessEncryptedContribs when handling QGETDATA, this design could lead to accidental modifications.

Consider returning a mutable quorum reference from the cache lookup or refactoring ProcessEncryptedContribs to take a const reference where possible.


612-673: Migration code is properly marked for removal in v23.

The TODO on line 612 clearly indicates this migration code should be removed in v23. The migration:

  1. Checks if new DB is empty before migrating
  2. Copies both verification vectors and secret key shares
  3. Uses batch writes for efficiency
  4. Compacts the database after migration
  5. Cleans up the old EvoDB entries

Would you like me to open an issue to track the removal of this migration code in v23?

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25c87e4 and c7b5e7d.

📒 Files selected for processing (39)
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/chainlock/chainlock.cpp
  • src/chainlock/chainlock.h
  • src/dsnotificationinterface.cpp
  • src/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.cpp
  • src/init.cpp
  • src/instantsend/net_instantsend.cpp
  • src/instantsend/signing.cpp
  • src/llmq/context.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/options.h
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
  • src/llmq/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/msg_result.h
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/node/chainstate.h
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
  • test/util/data/non-backported.txt
💤 Files with no reviewable changes (1)
  • src/node/chainstate.h
🚧 Files skipped from review as they are similar to previous changes (11)
  • src/evo/specialtxman.cpp
  • src/evo/mnhftx.cpp
  • test/util/data/non-backported.txt
  • src/llmq/signing_shares.cpp
  • src/rpc/quorums.cpp
  • src/llmq/options.h
  • src/dsnotificationinterface.cpp
  • src/msg_result.h
  • src/instantsend/net_instantsend.cpp
  • src/llmq/context.h
  • src/evo/smldiff.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/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/test/util/setup_common.cpp
  • src/chainlock/chainlock.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/chainlock/chainlock.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/init.cpp
  • src/active/quorums.h
  • src/active/quorums.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
src/{masternode,evo}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/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/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.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/cbtx.cpp
  • src/evo/assetlocktx.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/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
src/{masternode,llmq}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

BLS integration must be used for cryptographic foundation of advanced masternode features

Files:

  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.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/observer/context.h
  • src/llmq/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
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/util/setup_common.cpp
src/node/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/node/chainstate.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 (29)
📓 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: 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/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: 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: 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)
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.
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.
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
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
📚 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/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/test/util/setup_common.cpp
  • src/chainlock/chainlock.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/chainlock/chainlock.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.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} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

  • src/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/test/util/setup_common.cpp
  • src/chainlock/chainlock.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/chainlock/chainlock.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.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/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.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/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/test/util/setup_common.cpp
  • src/chainlock/chainlock.cpp
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/chainlock/chainlock.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

  • src/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.cpp
  • 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/{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/instantsend/signing.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.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: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.

Applied to files:

  • src/instantsend/signing.cpp
  • src/chainlock/chainlock.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/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • src/evo/assetlocktx.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/chainlock/chainlock.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • 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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

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

Applied to files:

  • src/instantsend/signing.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/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,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/cbtx.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/init.cpp
  • src/Makefile.am
  • src/node/chainstate.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/evo/cbtx.cpp
  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/init.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.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/assetlocktx.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/evo/assetlocktx.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.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/observer/context.h
  • src/masternode/active/context.h
  • src/llmq/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorums.h
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.

Applied to files:

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

Applied to files:

  • src/llmq/observer/context.h
  • src/masternode/active/context.h
  • src/chainlock/chainlock.h
  • src/llmq/quorums.h
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/llmq/observer/quorums.h
📚 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/masternode/active/context.h
  • src/test/util/setup_common.cpp
  • src/masternode/active/context.cpp
  • src/init.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/test/util/setup_common.cpp
  • src/llmq/signing.cpp
  • src/llmq/quorums.h
  • src/llmq/quorums.cpp
  • src/active/quorums.h
  • src/active/quorums.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.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/test/util/setup_common.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/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/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Applied to files:

  • src/test/util/setup_common.cpp
  • src/init.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/masternode/active/context.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/masternode/active/context.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/chainlock/chainlock.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/llmq/quorums.cpp
  • 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-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
🧬 Code graph analysis (11)
src/llmq/observer/context.h (3)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/observer/quorums.cpp (8)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
  • UpdatedBlockTip (61-75)
  • UpdatedBlockTip (61-61)
src/llmq/observer/context.cpp (8)
  • ObserverContext (13-28)
  • ObserverContext (30-33)
  • Start (35-38)
  • Start (35-35)
  • Stop (40-43)
  • Stop (40-40)
  • UpdatedBlockTip (45-52)
  • UpdatedBlockTip (45-45)
src/masternode/active/context.h (1)
src/active/quorums.cpp (2)
  • QuorumParticipant (27-37)
  • QuorumParticipant (39-39)
src/masternode/active/notificationinterface.cpp (1)
src/net_processing.cpp (1)
  • pindexNew (607-607)
src/test/util/setup_common.cpp (1)
src/llmq/options.h (1)
  • DEFAULT_BLSCHECK_THREADS (33-33)
src/llmq/observer/context.cpp (1)
src/llmq/observer/quorums.cpp (4)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
src/llmq/quorums.h (8)
src/active/quorums.h (1)
  • llmq (30-35)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/context.h (1)
  • llmq (22-29)
src/llmq/blockprocessor.h (1)
  • llmq (36-103)
src/evo/mnhftx.h (1)
  • llmq (30-32)
src/evo/creditpool.h (1)
  • llmq (33-35)
src/llmq/types.h (1)
  • llmq (10-17)
src/llmq/quorums.cpp (7)
  • CQuorum (101-103)
  • MakeQuorumKey (18-27)
  • MakeQuorumKey (18-18)
  • DataCleanupHelper (29-74)
  • DataCleanupHelper (29-29)
  • SetSecretKeyShare (125-133)
  • SetSecretKeyShare (125-125)
src/init.cpp (2)
src/init.h (1)
  • node (23-25)
src/llmq/options.h (1)
  • DEFAULT_WATCH_QUORUMS (37-51)
src/active/quorums.h (5)
src/llmq/quorums.h (2)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/quorums.cpp (1)
  • CQuorum (101-103)
src/active/quorums.cpp (10)
  • QuorumParticipant (27-37)
  • QuorumParticipant (39-39)
  • IsMasternode (169-173)
  • IsMasternode (169-169)
  • IsWatching (175-179)
  • IsWatching (175-175)
  • SetQuorumSecretKeyShare (82-85)
  • SetQuorumSecretKeyShare (82-82)
  • ProcessEncryptedContribs (109-167)
  • ProcessEncryptedContribs (109-113)
src/llmq/observer/quorums.cpp (10)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • IsMasternode (153-157)
  • IsMasternode (153-153)
  • IsWatching (159-163)
  • IsWatching (159-159)
  • SetQuorumSecretKeyShare (127-131)
  • SetQuorumSecretKeyShare (127-127)
  • ProcessEncryptedContribs (133-151)
  • ProcessEncryptedContribs (133-137)
src/llmq/quorumsman.h (5)
src/llmq/observer/context.h (4)
  • util (30-32)
  • llmq (22-29)
  • llmq (34-64)
  • final (35-63)
src/llmq/quorums.h (2)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/quorumsman.cpp (20)
  • CQuorumManager (32-46)
  • CQuorumManager (48-54)
  • HasQuorum (145-148)
  • HasQuorum (145-145)
  • GetQuorum (368-389)
  • GetQuorum (368-368)
  • GetQuorum (391-407)
  • GetQuorum (391-391)
  • ScanQuorums (193-197)
  • ScanQuorums (193-193)
  • ScanQuorums (199-307)
  • ScanQuorums (199-201)
  • IsMasternode (309-315)
  • IsMasternode (309-309)
  • IsWatching (317-323)
  • IsWatching (317-317)
  • CacheWarmingThreadMain (566-602)
  • CacheWarmingThreadMain (566-566)
  • MigrateOldQuorumDB (613-673)
  • MigrateOldQuorumDB (613-613)
src/llmq/params.h (1)
  • LLMQType (14-125)
src/llmq/quorumsman.cpp (5)
src/llmq/quorums.h (3)
  • nDataMask (88-88)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/observer/context.h (2)
  • llmq (22-29)
  • llmq (34-64)
src/llmq/quorums.cpp (4)
  • DataCleanupHelper (29-74)
  • DataCleanupHelper (29-29)
  • s (196-196)
  • s (211-211)
src/msg_result.h (1)
  • MisbehavingError (19-72)
src/llmq/observer/quorums.h (2)
src/llmq/observer/context.h (2)
  • llmq (22-29)
  • llmq (34-64)
src/llmq/observer/quorums.cpp (30)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
  • UpdatedBlockTip (61-75)
  • UpdatedBlockTip (61-61)
  • SetQuorumSecretKeyShare (127-131)
  • SetQuorumSecretKeyShare (127-127)
  • ProcessEncryptedContribs (133-151)
  • ProcessEncryptedContribs (133-137)
  • IsMasternode (153-157)
  • IsMasternode (153-153)
  • IsWatching (159-163)
  • IsWatching (159-159)
  • CheckQuorumConnections (106-125)
  • CheckQuorumConnections (106-107)
  • TriggerQuorumDataRecoveryThreads (286-299)
  • TriggerQuorumDataRecoveryThreads (286-286)
  • GetQuorumsToDelete (77-104)
  • GetQuorumsToDelete (77-78)
  • DataRecoveryThread (165-270)
  • DataRecoveryThread (165-166)
  • StartVvecSyncThread (272-284)
  • StartVvecSyncThread (272-272)
  • TryStartVvecSyncThread (301-317)
  • TryStartVvecSyncThread (301-302)
  • StartCleanupOldQuorumDataThread (319-372)
  • StartCleanupOldQuorumDataThread (319-319)
🪛 Cppcheck (2.19.0)
src/llmq/context.cpp

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

(preprocessorErrorDirective)

src/masternode/active/context.cpp

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

(preprocessorErrorDirective)

src/llmq/observer/quorums.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] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/llmq/quorums.cpp

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

(syntaxError)

src/active/quorums.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/quorumsman.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)

⏰ 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: arm-linux-build / Build source
  • 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: mac-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: Lint / Run linters
🔇 Additional comments (62)
src/test/util/setup_common.cpp (1)

142-152: LGTM! Good simplification of test configuration.

The updated parameter list replaces explicit quorum configuration (sync maps, data recovery/watch booleans) with streamlined defaults (llmq::DEFAULT_BLSCHECK_THREADS, llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE). Using DEFAULT_BLSCHECK_THREADS = 0 is appropriate for test scenarios, enabling synchronous BLS processing and reducing threading complexity. This aligns well with the PR's refactoring objectives.

src/chainlock/chainlock.h (1)

37-42: Forward declaration now pins underlying type

Specifying : uint8_t here is fine and should match the definition in llmq/quorumsman.h; just keep these in sync if the enum ever changes.

src/Makefile.am (1)

154-155: Build wiring for new quorum components looks consistent

The additions of active/quorums.{h,cpp}, llmq/quorumsman.{h,cpp}, and llmq/observer/quorums.{h,cpp} to BITCOIN_CORE_H and libbitcoin_node_a_SOURCES match how other LLMQ/masternode modules are exposed; the build side looks correct.

One process note: if not already done elsewhere in this PR, please also add these new non‑backported files to test/util/data/non-backported.txt to comply with the Dash convention for newly introduced (non‑Bitcoin) sources.

Also applies to: 281-283, 291-292, 485-486, 555-557, 563-563

src/init.cpp (2)

275-299: Observer context lifecycle integrates cleanly into startup/shutdown

Adding observer_ctx->Stop() in PrepareShutdown() and observer_ctx->Start() alongside active_ctx keeps the LLMQ observer lifecycle consistent with other Dash services, and the Start/Stop ordering (llmq_ctx → peerman → active/observer on start, and the reverse on shutdown) looks sound.

One thing to double‑check: PrepareShutdown() will call observer_ctx->Stop() whenever the pointer is non‑null, even on early‑failure paths where Start() might not have been called yet. Please ensure llmq::ObserverContext::Stop() is safe and idempotent in that scenario.

Also applies to: 2338-2342


2209-2219: Quorum sync/recovery flags and handlers are wired coherently

Deriving quorums_recovery, quorums_watch, and the shared llmq::QvvecSyncModeMap sync_map once in AppInitMain() and passing them into:

  • ActiveContext(..., sync_map, dash_db_params, quorums_recovery, quorums_watch) when mn_activeman is present, and
  • ObserverContext(..., sync_map, dash_db_params, quorums_recovery) for non‑MN watch‑only nodes,

keeps masternode and observer modes clearly separated while sharing the same sync configuration. Moving NetInstantSend and NetSigning handler registration after this setup also ensures they see a fully initialized LLMQ stack.

Behaviorally this matches the PR description (participant vs watch‑only split); just confirm that “masternode + watch” is intentionally handled inside ActiveContext/QuorumParticipant and not via a separate observer_ctx as the else if (quorums_watch) branch implies.

Also applies to: 2223-2225, 2231-2233

src/llmq/quorums.cpp (1)

125-133: Secret key share validation via proTx hash is sound

Updating CQuorum::SetSecretKeyShare to:

  • take const uint256& protx_hash, and
  • verify secretKeyShare.GetPublicKey() == GetPubKeyShare(GetMemberIndex(protx_hash))

keeps the membership check local to CQuorum and removes the dependency on CActiveMasternodeManager. Given GetPubKeyShare guards against out‑of‑range indices, passing a non‑member protx_hash simply causes the check to fail and returns false, which is the desired behavior.

WriteContributions/ReadContributions using MakeQuorumKey(*this) and the new DB prefixes stays consistent with the helpers above, so contribution persistence remains straightforward.

Also applies to: 190-205, 208-231

src/chainlock/chainlock.cpp (1)

19-21: Include switched to new quorum manager header

Switching from <llmq/quorums.h> to <llmq/quorumsman.h> aligns this file with the new CQuorumManager/VerifyRecoveredSig home and keeps the ChainLocks dependency surface tight. No issues spotted.

src/instantsend/signing.cpp (1)

14-16: InstantSend signer now includes the correct quorum manager header

Updating the include to llmq/quorumsman.h matches the new location of CQuorumManager and SelectQuorumForSigning. The rest of InstantSendSigner remains unchanged, so this is a clean dependency update.

src/evo/cbtx.cpp (1)

11-11: LGTM! Include path updated consistently with refactoring.

The header change from llmq/quorums.h to llmq/quorumsman.h maintains access to llmq::CQuorumBlockProcessor used throughout this file for quorum commitment processing and merkle root calculation.

src/llmq/ehf_signals.cpp (1)

16-16: LGTM! Header refactor completed consistently.

The include update from llmq/quorums.h to llmq/quorumsman.h aligns with the PR's quorum management restructuring. The file's usage of CQuorumManager and SelectQuorumForSigning remains unchanged.

src/evo/assetlocktx.cpp (1)

9-9: Include refactoring is correct. The change from llmq/quorums.h to llmq/quorumsman.h properly addresses symbol requirements: assetlocktx.cpp uses llmq::CQuorumManager as a const reference parameter in function signatures, which requires the full class definition available in quorumsman.h (not just the forward declaration in quorums.h).

src/net_processing.cpp (1)

58-58: Include switch to llmq/quorumsman.h aligns with quorum-manager usage

Using the more specific manager header here matches this file’s direct use of m_llmq_ctx->qman and related APIs, and keeps the dependency consistent with the new refactor. No further changes needed in this unit.

src/llmq/signing.cpp (1)

9-9: LGTM!

The include update from llmq/quorums.h to llmq/quorumsman.h correctly aligns with the PR's refactoring that moves CQuorumManager and related APIs to the new header. The file's usage of CQuorumManager and IsQuorumActive is properly supported by the new include.

src/masternode/active/notificationinterface.cpp (2)

7-7: LGTM!

The include addition enables access to the QuorumParticipant type needed for the new qman_handler invocation.


33-33: LGTM!

The new qman_handler->UpdatedBlockTip call follows the established notification pattern in this file and correctly propagates block tip updates to the QuorumParticipant. This integrates the masternode-specific quorum handling into the notification chain.

src/llmq/observer/context.h (5)

8-8: LGTM!

The llmq/options.h include is needed for the QvvecSyncModeMap type used in the constructor signature.


16-20: LGTM!

Forward declarations for CConnman and CMasternodeSync are properly added to support the expanded constructor signature without introducing unnecessary header dependencies.


28-28: LGTM!

Forward declaration for QuorumObserver is correctly placed within the llmq namespace.


43-51: LGTM!

The expanded constructor signature properly accepts all dependencies needed for QuorumObserver construction. The Start() and Stop() lifecycle methods follow the established pattern in the codebase for managing thread lifecycles.


60-62: LGTM!

The qman_handler member is appropriately declared as private since it's an internal implementation detail. Using std::unique_ptr ensures proper RAII-based lifetime management.

test/lint/lint-circular-dependencies.py (3)

24-24: LGTM!

The circular dependency path update correctly reflects the header rename from llmq/quorums to llmq/quorumsman.


30-30: LGTM!

Consistent with the header refactoring.


38-38: LGTM!

Consistent with the header refactoring.

src/masternode/active/context.h (4)

8-8: LGTM!

The llmq/options.h include provides the QvvecSyncModeMap type needed in the constructor signature.


37-37: LGTM!

Forward declaration for QuorumParticipant is correctly placed within the llmq namespace.


56-57: LGTM!

The expanded constructor signature properly includes sync_map, db_params, quorums_recovery, and quorums_watch parameters needed for QuorumParticipant initialization.


75-75: LGTM!

The qman_handler public member follows the same pattern as other similar members (gov_signer, dkgdbgman, qdkgsman, etc.) in this context. Using std::unique_ptr ensures proper lifetime management.

src/llmq/context.cpp (3)

11-11: LGTM!

The include update from llmq/quorums.h to llmq/quorumsman.h aligns with the header refactoring.


16-24: LGTM!

The simplified constructor correctly removes masternode-specific parameters (mn_activeman, sync_map, quorums_recovery, quorums_watch) that are now handled by QuorumParticipant in ActiveContext. The CQuorumManager initialization is appropriately streamlined to only include dependencies needed for its core quorum data management functionality.


39-47: LGTM!

The removal of qman->Start() and qman->Stop() calls is correct. Lifecycle management for quorum-related threads is now handled by QuorumObserver::Start/Stop() (for observer mode) and QuorumParticipant::Start/Stop() (for masternode mode) in their respective contexts, aligning with the PR's separation of observer and participant logic.

src/llmq/quorums.h (5)

14-22: LGTM!

The includes are properly organized. The <set> include was correctly added to support std::set<uint256> used in DataCleanupHelper. This addresses the previous review feedback.


26-31: LGTM!

Forward declarations for CQuorum, CQuorumManager, QuorumObserver, and QuorumParticipant are properly placed to minimize header dependencies.


33-38: LGTM!

The public API declarations for DB_QUORUM_SK_SHARE, DB_QUORUM_QUORUM_VVEC, MakeQuorumKey, and DataCleanupHelper are appropriately exposed in this header. These are used across observer and participant contexts for quorum data management.


162-164: LGTM!

Adding QuorumObserver and QuorumParticipant as friends of CQuorum is appropriate since they need access to private members for quorum data recovery and management operations (e.g., WriteContributions, ReadContributions, SetVerificationVector).


194-195: All call sites correctly updated with protx_hash parameter.

Verification confirms both SetSecretKeyShare call sites (src/active/quorums.cpp:84 and :160) properly pass protx_hash directly via m_mn_activeman.GetProTxHash(). The signature decoupling from CActiveMasternodeManager is complete and correct.

src/masternode/active/context.cpp (8)

7-7: LGTM!

The include for active/quorums.h provides the QuorumParticipant class definition needed for qman_handler.


18-18: LGTM!

The include update from llmq/quorums.h to llmq/quorumsman.h aligns with the header refactoring.


26-27: LGTM!

The expanded constructor parameters (sync_map, db_params, quorums_recovery, quorums_watch) provide the dependencies needed for QuorumParticipant initialization.


39-41: LGTM!

The QuorumParticipant is properly constructed with all required dependencies. The initialization order places it after ehf_sighandler and before cl_signer/is_signer, which is appropriate given it doesn't depend on those signers.


50-50: LGTM!

The ConnectManagers call correctly registers both qman_handler (QuorumParticipant) and qdkgsman with the quorum manager, enabling coordinated lifecycle and notification handling.


55-55: LGTM!

The DisconnectManagers call properly cleans up the manager connections during destruction.


67-67: LGTM!

Starting qman_handler first ensures the quorum participant's threads are running before dependent components (qdkgsman, shareman, etc.) begin their work.


82-82: LGTM!

Stopping qman_handler last (after qdkgsman) follows proper shutdown order—dependent components are stopped before the quorum participant threads are terminated.

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

45-52: LGTM!

The UpdatedBlockTip correctly propagates the block tip update to both qdkgsman and qman_handler, maintaining the expected notification order. The early returns for IBD and no-change cases are appropriate.


30-43: No action needed—destructor follows established patterns in the codebase.

The current implementation is consistent with other context destructors in the codebase (LLMQContext, ActiveContext), which call specific disconnect/cleanup methods rather than their full Stop() methods during destruction. The caller is responsible for invoking Stop() if needed before the destructor runs. No changes required.

Likely an incorrect or invalid review comment.

src/node/chainstate.cpp (2)

91-93: LGTM!

The simplified call to DashChainstateSetup correctly removes the sync_map, quorums_recovery, and quorums_watch parameters, aligning with the refactoring that moves these concerns to the observer/participant layer.


244-246: LGTM!

The streamlined LLMQContext construction with reduced parameters is cleaner and matches the updated function signature in DashChainstateSetup.

src/llmq/observer/quorums.cpp (3)

165-270: DataRecoveryThread is well-structured with proper interrupt handling.

The thread correctly:

  • Waits for blockchain sync before proceeding
  • Uses quorumThreadInterrupt for graceful shutdown
  • Applies load balancing via start_offset
  • Handles request timeouts and retries
  • Uses atomic flag fQuorumDataRecoveryThreadRunning to prevent duplicate threads

The logic for requesting data from quorum members and handling various response statuses is sound.


319-372: Cleanup thread runs at specific block heights to avoid DKG interference.

The choice of height % 576 == 58 is well-documented in the comments. The implementation correctly:

  • Uses a timer for performance monitoring
  • Runs cleanup in the worker pool to avoid blocking
  • Respects thread interrupts
  • Uses proper locking for the cleanup cache

46-59: The original review comment's concern about guarding against double-start or stop-before-start is incorrect. The ctpl::thread_pool::stop(true) method has built-in guards that make it safe to call multiple times:

  1. Internal idempotency check: ctpl::thread_pool::stop(true) checks if (this->isDone || this->isStop) return; before proceeding, preventing re-entry on subsequent calls.
  2. Safe on unstarted pool: If Start() was never called, workerPool.resize() simply resizes to 0 threads, and stop(true) safely handles an empty thread list via a guarded for loop with joinable() checks.
  3. Consistent with codebase patterns: Other components (e.g., CSigSharesManager) use identical patterns without additional guards, relying on ctpl's internal safety mechanisms.

No changes needed to the code.

src/active/quorums.h (1)

37-81: Well-designed class hierarchy with clear separation of concerns.

QuorumParticipant correctly:

  • Derives from QuorumObserver with final specifier
  • Overrides the necessary virtual methods for masternode participation
  • Keeps masternode-specific state (m_bls_worker, m_mn_activeman, m_quorums_watch) as private members
  • Documents the purpose of GetQuorumRecoveryStartOffset in the comment

The deleted default and copy constructors enforce proper initialization.

src/active/quorums.cpp (3)

41-80: CheckQuorumConnections correctly adds inter-quorum connections for InstantSend.

The logic for watchOtherISQuorums ensures that masternodes participating in InstantSend quorums maintain connections to other IS quorums they're not members of. This is important for efficient message relay. The use of CalcDeterministicWatchConnections provides deterministic peer selection.


109-167: ProcessEncryptedContribs correctly handles encryption/decryption for masternodes.

For QGETDATA:

  • Validates membership before providing encrypted contributions
  • Properly retrieves and serializes encrypted contributions

For QDATA:

  • Validates verification vector availability
  • Decrypts contributions using the active masternode's keys
  • Aggregates secret keys and sets the quorum's secret key share

The error handling with MisbehavingError scoring is appropriate.


196-230: TriggerQuorumDataRecoveryThreads has proper recovery logic for masternodes.

The method correctly:

  • Distinguishes between quorums where the masternode is a valid member vs. observer
  • Requests both verification vector and encrypted contributions for member quorums
  • Falls back to TryStartVvecSyncThread for non-member quorums

Note: The std::move(pQuorum) on lines 220 and 226 is intentional to transfer ownership to the spawned threads, and the loop variable is not used after the move.

src/llmq/observer/quorums.h (2)

46-64: QuorumHandlerParent provides a clean abstraction for handler callbacks.

The interface correctly declares pure virtual methods that CQuorumManager needs to call on the handler. This enables the observer/participant pattern while maintaining loose coupling.


66-127: QuorumObserver base class is well-designed for the observer pattern.

Key design decisions:

  • Protected members allow derived classes direct access to dependencies
  • Mutable members (cs_cleanup, cleanupQuorumsCache, workerPool, quorumThreadInterrupt) correctly support const methods that modify cache/thread state
  • Virtual methods at appropriate visibility levels (public for API, protected for customization)
  • Thread pool and interrupt mechanism enable background processing

The m_sync_map is stored by value (copy) which is appropriate since it's configuration that shouldn't change.

src/llmq/quorumsman.h (3)

64-68: Friend declarations enable tight integration with observer/participant.

The friend class declarations for QuorumObserver and QuorumParticipant allow these classes to access private members like db and caches. While this creates coupling, it's intentional for this refactoring where the observer/participant are tightly integrated components of the quorum management system.


86-103: Multiple mutexes with clear thread-safety annotations.

The class uses fine-grained locking with separate mutexes for different concerns:

  • cs_db for database access
  • cs_data_requests for request tracking
  • cs_map_quorums for quorum cache
  • cs_scan_quorums for scan cache
  • cs_quorumBaseBlockIndexCache for block index cache
  • m_cache_cs for warming queue

The TODO on line 89 about merging cs_map_quorums and cs_scan_quorums is noted for future cleanup.


113-125: ConnectManagers/DisconnectManagers provide safe lifecycle management.

The ConnectManagers method uses assertions to prevent double initialization, which is appropriate for catching programmer errors during development. Based on learnings, this fail-fast approach is preferred for invariant enforcement.

src/llmq/quorumsman.cpp (3)

31-46: Constructor properly initializes cache, starts warming thread, and migrates legacy data.

The initialization sequence is correct:

  1. Initialize member references and create database
  2. Initialize quorums cache
  3. Reset interrupt and start cache warming thread
  4. Migrate old quorum data from EvoDB

The use of util::TraceThread enables proper thread naming for debugging.


48-54: Destructor correctly joins the cache warming thread.

The joinable() check before join() is good practice, and calling m_cache_interrupt() before join ensures the thread exits promptly.


675-751: Quorum selection and signature verification functions are correctly implemented.

SelectQuorumForSigning:

  • Handles both rotation and non-rotation quorum types
  • Uses deterministic selection based on selectionHash
  • Properly handles the sign offset for chain tip verification

VerifyRecoveredSig:

  • Correctly constructs the sign hash
  • Uses VerifyInsecure which is appropriate for recovered signatures where the public key is already validated by the quorum

// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <llmq/quorums.h>
#include <active/quorums.h>
Copy link
Member

Choose a reason for hiding this comment

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

please rename to activemn or something that indicates what is active

Copy link
Collaborator Author

@kwvg kwvg Jan 2, 2026

Choose a reason for hiding this comment

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

As discussed earlier, the suggestion of active_mn or something to that effect can be considered down the road but IMO shouldn't be a blocker now.

These subroutines do nothing unless we are in masternode mode so we need
to extract them in order to isolate it from regular mode.
kwvg added 2 commits January 2, 2026 03:01
Also:
- Drop avoidable `CActiveMasternodeManager&` arg by taking ref of hash
  we care about
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
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

♻️ Duplicate comments (1)
src/active/quorums.cpp (1)

1-5: Consider renaming directory to clarify masternode context.

A previous reviewer suggested renaming from active to activemn or similar to indicate that this relates to active masternode functionality, not just generic "active" behavior.

🧹 Nitpick comments (2)
src/init.cpp (1)

1415-1422: Early validation and warning for -llmq-qvvec-sync vs -llmq-data-recovery is well scoped

Catching std::invalid_argument from GetEnabledQuorumVvecSyncEntries(args) and converting it to InitError, plus warning when qvvec sync is configured but recovery is disabled, gives clear feedback while leaving behavior unchanged if the user insists on this combo. One small optional refinement is that -llmq-qvvec-sync is parsed twice (here and when building sync_map in AppInitMain); if this ever becomes more complex, you could cache the parsed map or thread it via NodeContext, but current cost is negligible.

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

318-371: Cleanup thread scheduling avoids DKG windows.

The logic at line 327 (pIndex->nHeight % 576 != 58) is designed to avoid CPU-heavy cleanup during DKG mining windows. The comment explains the rationale well. This is a thoughtful optimization.

Note: The magic number 576 appears to be derived from quorum cycle calculations (24 blocks × 24 cycles). Consider defining this as a named constant for clarity in future refactoring.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7b5e7d and db0729b.

📒 Files selected for processing (39)
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/chainlock/chainlock.cpp
  • src/chainlock/chainlock.h
  • src/dsnotificationinterface.cpp
  • src/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.cpp
  • src/init.cpp
  • src/instantsend/net_instantsend.cpp
  • src/instantsend/signing.cpp
  • src/llmq/context.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/options.h
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
  • src/llmq/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/msg_result.h
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/node/chainstate.h
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
  • test/util/data/non-backported.txt
💤 Files with no reviewable changes (1)
  • src/node/chainstate.h
✅ Files skipped from review due to trivial changes (1)
  • src/evo/mnhftx.cpp
🚧 Files skipped from review as they are similar to previous changes (13)
  • src/chainlock/chainlock.cpp
  • src/chainlock/chainlock.h
  • src/test/util/setup_common.cpp
  • src/instantsend/signing.cpp
  • src/evo/cbtx.cpp
  • test/util/data/non-backported.txt
  • src/instantsend/net_instantsend.cpp
  • src/Makefile.am
  • src/node/chainstate.cpp
  • test/lint/lint-circular-dependencies.py
  • src/evo/assetlocktx.cpp
  • src/net_processing.cpp
  • src/evo/specialtxman.cpp
🧰 Additional context used
📓 Path-based instructions (7)
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/masternode/active/notificationinterface.cpp
  • src/dsnotificationinterface.cpp
  • src/init.cpp
  • src/msg_result.h
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.h
  • src/evo/smldiff.cpp
  • src/masternode/active/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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/llmq/signing_shares.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/llmq/signing_shares.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/llmq/signing_shares.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/signing_shares.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/smldiff.cpp
🧠 Learnings (33)
📓 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: 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)
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
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.
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.
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
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
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
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.
📚 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/masternode/active/notificationinterface.cpp
  • src/evo/smldiff.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/masternode/active/notificationinterface.cpp
  • src/active/quorums.cpp
  • src/evo/smldiff.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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

  • src/masternode/active/notificationinterface.cpp
  • src/dsnotificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/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/masternode/active/notificationinterface.cpp
  • src/dsnotificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
  • src/llmq/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/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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/masternode/active/context.cpp
  • src/llmq/observer/quorums.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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.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/masternode/active/notificationinterface.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/signing.cpp
  • src/llmq/ehf_signals.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.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/dsnotificationinterface.cpp
  • src/init.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/context.cpp
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.

Applied to files:

  • src/dsnotificationinterface.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/llmq/observer/context.h
  • src/llmq/context.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/context.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/dsnotificationinterface.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/context.h
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
  • 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/init.cpp
  • src/masternode/active/context.h
  • src/masternode/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,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/init.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/evo/smldiff.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/init.cpp
📚 Learning: 2025-08-10T13:52:46.289Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6666
File: test/functional/rpc_netinfo.py:73-81
Timestamp: 2025-08-10T13:52:46.289Z
Learning: In the Bitcoin/Dash test framework, `self.node.extra_args` contains the original startup arguments for a TestNode and is not modified by `test.restart_node()`. Each restart with new `extra_args` doesn't update the stored `self.node.extra_args`, so copying from it always provides a clean slate without accumulated modifications from previous restarts.

Applied to files:

  • src/init.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/init.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/msg_result.h
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/llmq/observer/context.h
  • src/llmq/context.h
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/llmq/options.h
  • src/llmq/observer/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/masternode/active/context.h
  • src/rpc/quorums.cpp
  • src/llmq/signing_shares.cpp
  • src/active/quorums.cpp
  • src/llmq/observer/context.h
  • src/llmq/signing.cpp
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/evo/smldiff.cpp
  • src/active/quorums.h
  • src/masternode/active/context.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/context.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/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.cpp
  • src/llmq/signing.cpp
  • src/llmq/quorumsman.h
  • src/llmq/quorumsman.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/llmq/quorums.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/llmq/quorums.h
  • src/active/quorums.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/llmq/quorums.h
  • src/active/quorums.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/llmq/quorums.h
  • src/active/quorums.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/quorums.h
📚 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/quorums.h
  • src/active/quorums.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/quorums.h
📚 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/llmq/quorums.h
  • src/active/quorums.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/quorums.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/active/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/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/smldiff.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/masternode/active/context.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/masternode/active/context.cpp
🧬 Code graph analysis (10)
src/masternode/active/notificationinterface.cpp (1)
src/net_processing.cpp (1)
  • pindexNew (607-607)
src/init.cpp (4)
src/interfaces/node.h (2)
  • node (48-50)
  • node (481-481)
src/init.h (1)
  • node (23-25)
src/llmq/options.h (1)
  • DEFAULT_WATCH_QUORUMS (37-51)
src/llmq/options.cpp (2)
  • GetEnabledQuorumVvecSyncEntries (59-104)
  • GetEnabledQuorumVvecSyncEntries (59-59)
src/llmq/quorums.h (5)
src/active/quorums.h (1)
  • llmq (30-35)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/context.h (1)
  • llmq (22-29)
src/evo/mnhftx.h (1)
  • llmq (30-32)
src/llmq/types.h (1)
  • llmq (10-17)
src/masternode/active/context.h (1)
src/active/quorums.cpp (2)
  • QuorumParticipant (27-37)
  • QuorumParticipant (39-39)
src/active/quorums.cpp (3)
src/llmq/dkgsessionmgr.h (1)
  • m_quorums_watch (60-126)
src/llmq/quorumsman.cpp (4)
  • IsMasternode (309-315)
  • IsMasternode (309-309)
  • IsWatching (317-323)
  • IsWatching (317-317)
src/llmq/observer/quorums.h (1)
  • m_quorums_recovery (76-127)
src/llmq/observer/context.h (4)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/net_processing.h (2)
  • llmq (34-36)
  • bool (93-93)
src/node/context.h (1)
  • llmq (38-40)
src/llmq/observer/context.cpp (2)
  • ObserverContext (13-28)
  • ObserverContext (30-33)
src/llmq/observer/quorums.cpp (4)
src/llmq/quorumsman.cpp (4)
  • IsMasternode (309-315)
  • IsMasternode (309-309)
  • IsWatching (317-323)
  • IsWatching (317-317)
src/llmq/quorums.h (1)
  • nDataMask (88-88)
src/llmq/observer/quorums.h (1)
  • m_quorums_recovery (76-127)
src/llmq/quorums.cpp (2)
  • MakeQuorumKey (18-27)
  • MakeQuorumKey (18-18)
src/llmq/observer/context.cpp (3)
src/llmq/observer/quorums.cpp (4)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
src/llmq/context.cpp (4)
  • Start (39-42)
  • Start (39-39)
  • Stop (44-47)
  • Stop (44-44)
src/net_processing.cpp (1)
  • pindexNew (607-607)
src/llmq/quorumsman.h (4)
src/llmq/observer/quorums.h (1)
  • llmq (34-37)
src/llmq/quorumsman.cpp (8)
  • CQuorumManager (32-46)
  • CQuorumManager (48-54)
  • HasQuorum (145-148)
  • HasQuorum (145-145)
  • SelectQuorumForSigning (670-730)
  • SelectQuorumForSigning (670-671)
  • VerifyRecoveredSig (732-746)
  • VerifyRecoveredSig (732-734)
src/llmq/params.h (1)
  • LLMQType (14-125)
src/llmq/quorums.cpp (1)
  • CQuorum (101-103)
src/llmq/observer/quorums.h (3)
src/llmq/observer/context.h (2)
  • llmq (22-29)
  • llmq (34-64)
src/llmq/observer/quorums.cpp (8)
  • QuorumObserver (24-39)
  • QuorumObserver (41-44)
  • Start (46-52)
  • Start (46-46)
  • Stop (54-59)
  • Stop (54-54)
  • UpdatedBlockTip (61-75)
  • UpdatedBlockTip (61-61)
src/llmq/observer/context.cpp (6)
  • Start (35-38)
  • Start (35-35)
  • Stop (40-43)
  • Stop (40-40)
  • UpdatedBlockTip (45-52)
  • UpdatedBlockTip (45-45)
🪛 Cppcheck (2.19.0)
src/llmq/quorums.cpp

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

(syntaxError)

src/active/quorums.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/quorums.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/masternode/active/context.cpp

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

(preprocessorErrorDirective)

src/llmq/observer/context.cpp

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

(syntaxError)

src/llmq/quorumsman.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/context.cpp

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

(preprocessorErrorDirective)

⏰ 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_ubsan-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • 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: Lint / Run linters
🔇 Additional comments (55)
src/llmq/options.h (1)

24-28: Explicit int8_t underlying type for QvvecSyncMode is safe and clearer

Enumerators stay within [-1,1], and existing parsing/usage via int32_t casts remains valid, so this narrows the type without changing behavior while making ABI/size explicit.

src/llmq/context.h (1)

39-64: Constructor reshaping keeps LLMQContext invariants intact

The reduced, ordered parameter list still provides everything needed to initialize bls_worker, qman, and the other managers, preserving the “all members valid when LLMQContext is non-null” guarantee relied on elsewhere (e.g. net_processing). Based on learnings, this fits the existing initialization model without introducing new lifetime risks.

src/init.cpp (2)

295-299: ObserverContext shutdown/teardown order looks correct

Stopping observer_ctx alongside active_ctx and llmq_ctx, unregistering it from validation, and then resetting both contexts before DashChainstateSetupClose receives node.llmq_ctx keeps the lifetimes aligned with NodeContext teardown patterns from earlier PRs, avoiding use-after-free on LLMQ components. Based on learnings, this matches the intended shutdown sequencing.

Also applies to: 352-373, 398-410


2209-2212: New sync_map / (active vs observer) wiring is consistent with the masternode/watch-only split

Deriving quorums_recovery, quorums_watch, and the QvvecSyncModeMap sync_map once and passing them into ActiveContext (for masternode mode) or llmq::ObserverContext (for watch-only -watchquorums nodes) cleanly centralizes quorum vvec/recovery configuration. Starting llmq_ctx, then peerman handlers, then active_ctx (if present), and finally observer_ctx in Step 10a, with matching Stop()/reset() paths in PrepareShutdown, keeps lifetimes and thread startup/shutdown aligned and avoids extra quorum threads on nodes that neither run a masternode nor watch quorums.

Also applies to: 2218-2227, 2231-2233, 2338-2342

src/llmq/quorums.h (1)

14-22: Quorum header surface refactor (helpers, friends, and SetSecretKeyShare) is coherent

Localizing the core includes plus <set>/<string>/<vector>, adding the DB_QUORUM_SK_SHARE / DB_QUORUM_QUORUM_VVEC externs, and exposing MakeQuorumKey / DataCleanupHelper(CDBWrapper&, const std::set<uint256>&, ...) gives a clean, minimal API and avoids the previous by-value std::set copies. Extending CQuorum’s friends to QuorumObserver and QuorumParticipant and adding the protx_hash parameter to SetSecretKeyShare fits the new observer/participant split; assuming the definition and all call sites in llmq/quorums.cpp and the new managers were updated accordingly, there are no correctness or consensus-behavior concerns here. Based on learnings, this stays within refactor scope.

Also applies to: 26-39, 161-164, 194-195

src/rpc/quorums.cpp (1)

26-26: Including llmq/quorumsman.h here matches the new CQuorumManager API

Switching the include from llmq/quorums.h to llmq/quorumsman.h is appropriate given this file’s heavy use of llmq_ctx.qman. As long as quorumsman.h (directly or transitively) provides the full llmq::CQuorum definition needed by BuildQuorumInfo and related RPCs, this keeps the dependency surface aligned with the refactor without altering behavior.

src/llmq/signing_shares.cpp (1)

9-9: LGTM! Clean header migration.

The include change from llmq/quorums.h to llmq/quorumsman.h aligns with the PR's broader refactoring to introduce the new quorum management architecture. No functional changes in this file.

src/llmq/ehf_signals.cpp (1)

16-16: LGTM! Header migration is consistent.

The switch to llmq/quorumsman.h maintains compatibility with the existing CQuorumManager usage throughout this file.

src/evo/smldiff.cpp (1)

16-16: LGTM! Straightforward header update.

The header migration maintains compatibility with the CQuorumManager usage in this file's quorum diff building logic.

src/llmq/signing.cpp (1)

9-9: LGTM! Header migration complete.

The switch to llmq/quorumsman.h is consistent with the PR's architectural refactoring while maintaining all existing CQuorumManager functionality used throughout this file.

src/dsnotificationinterface.cpp (1)

20-20: Include change is correct; UpdatedBlockTip handling properly delegated to observer/handler pathway.

The include migration to llmq/quorumsman.h is appropriate. The UpdatedBlockTip functionality previously called directly on qman is now properly routed through the qman_handler (QuorumObserver) via the ObserverContext::UpdatedBlockTip method. The observer implementation handles all necessary quorum operations: checking quorum connections, cleaning up expired data requests, triggering quorum data recovery, and managing old quorum data cleanup. This architectural refactoring successfully delegates the responsibility without losing functionality.

src/msg_result.h (1)

70-71: LGTM!

The empty() method is well-implemented, correctly checking all relevant state members. The naming follows STL conventions and the const-correctness is appropriate.

src/masternode/active/notificationinterface.cpp (2)

7-7: LGTM!

Include added correctly for the new QuorumParticipant handler.


33-33: LGTM!

The qman_handler->UpdatedBlockTip() call follows the same pattern as other handlers in this method and correctly forwards the block tip update to the new quorum participant component.

src/llmq/observer/context.h (3)

8-9: LGTM!

Include for llmq/options.h is needed for the QvvecSyncModeMap type used in the constructor.


43-51: LGTM!

The expanded constructor signature and added Start()/Stop() lifecycle methods align with the component lifecycle patterns used elsewhere in the codebase. The dependency injection approach is clean and testable.


60-62: LGTM!

Good encapsulation moving qman_handler to the private section while keeping dkgdbgman and qdkgsman public for external access. This appropriately hides the observer implementation details.

src/masternode/active/context.h (3)

8-9: LGTM!

Include for llmq/options.h provides the QvvecSyncModeMap type needed in the constructor signature.


52-57: LGTM!

The constructor extension with sync_map, db_params, quorums_recovery, and quorums_watch parameters properly wires the dependencies for the new QuorumParticipant component.


75-75: LGTM!

The qman_handler member follows the same pattern as other public handlers (gov_signer, qdkgsman, shareman, ehf_sighandler) with const std::unique_ptr for clear ownership semantics.

src/llmq/context.cpp (3)

11-11: LGTM!

Header change from llmq/quorums.h to llmq/quorumsman.h reflects the architectural split where CQuorumManager is now defined in the new header.


16-24: LGTM!

The simplified constructor reflects the observer/watch-only mode: masternode-specific parameters (mn_activeman, sync_map, quorums_recovery, quorums_watch) have been moved to ActiveContext and ObserverContext. The core LLMQContext now only handles shared infrastructure components.


39-47: LGTM!

Start() and Stop() now only manage clhandler lifecycle. The quorum manager start/stop is appropriately delegated to the observer/participant handlers that own those responsibilities.

src/masternode/active/context.cpp (6)

7-7: LGTM!

Include for active/quorums.h provides the QuorumParticipant class definition.


39-41: LGTM!

The QuorumParticipant initialization properly wires all required dependencies, passing through the sync mode map, recovery flag, and watch flag to configure masternode quorum participation behavior.


48-50: LGTM!

The ConnectManagers() call correctly registers both the qman_handler (QuorumParticipant) and qdkgsman with the quorum manager, establishing the multi-manager architecture described in the PR objectives.


53-58: LGTM!

The destructor correctly disconnects managers before disconnecting signers, maintaining proper teardown order.


75-83: LGTM!

The Stop() method correctly stops qman_handler last (Line 82), maintaining symmetric ordering with Start(). This ensures dependent components are stopped before the underlying quorum infrastructure.


65-72: The code correctly maintains symmetric start/stop ordering. qman_handler->Start() is called first in Start() and qman_handler->Stop() is called last in Stop(). All other components (qdkgsman, shareman, and recovery interfaces) follow proper LIFO cleanup order.

src/llmq/quorums.cpp (4)

14-16: LGTM!

The database key constants are now properly scoped within the llmq namespace, making them accessible for the new modular quorum architecture while maintaining clear ownership.


18-27: LGTM!

The MakeQuorumKey function is now properly scoped in the llmq namespace and takes a const CQuorum& reference. The key construction includes quorum type, hash, and all member proTxHashes, providing a deterministic and unique key.


29-74: LGTM!

The DataCleanupHelper function is well-implemented:

  • Iterates both prefixes (vvec, sk_share) for comprehensive cleanup
  • Uses batch operations with 16MB threshold to avoid memory pressure
  • Properly resets cursor before optional compaction
  • Logging correctly identifies the calling context

One minor note: the function handles an empty skip_list correctly—it will simply delete all entries for the prefixes, which appears to be the intended behavior for full cleanup scenarios.


125-133: LGTM!

The updated SetSecretKeyShare signature with the protx_hash parameter strengthens validation by ensuring:

  1. The proTxHash is not null
  2. The secret key is valid
  3. The derived public key matches the expected pubkey share for the member index

This prevents storing invalid or mismatched secret key shares.

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

13-28: LGTM! Constructor wiring is consistent with the multi-manager pattern.

The constructor properly initializes the QuorumObserver and wires it to the CQuorumManager via ConnectManagers. The parameter forwarding and initialization order are correct.


35-43: Lifecycle delegation looks correct.

Start() and Stop() properly delegate to qman_handler, which manages the worker pool lifecycle. The destructor calling DisconnectManagers() ensures proper cleanup ordering.

src/llmq/observer/quorums.cpp (4)

41-44: Destructor calls Stop() which may be called twice.

If Stop() is called explicitly before destruction, the destructor will call it again. Verify that Stop() is idempotent. Looking at the implementation, workerPool.stop(true) should be safe to call multiple times, but consider adding a guard or documenting this expectation.


46-52: Thread pool sizing is reasonable.

Worker count is clamped to [1, 4] based on half of hardware concurrency. This is a sensible default for background data recovery work that shouldn't monopolize CPU resources.


77-104: GetQuorumsToDelete correctly preserves in-progress DKG quorums.

The logic properly handles both rotated and non-rotated quorum types, ensuring that quorums involved in ongoing DKG rounds are not deleted. The logging provides good visibility into which quorums are being preserved.


164-269: DataRecoveryThread implementation is thorough but complex.

The recovery logic handles:

  • Blockchain sync waiting
  • Member list building and sorting for deterministic order
  • Timeout-based retry with member rotation
  • Proper interrupt handling

The complexity is warranted for robust data recovery. The printLog lambda provides consistent logging throughout.

src/active/quorums.cpp (3)

41-80: CheckQuorumConnections correctly handles masternode-specific connection logic.

The implementation properly:

  1. Uses the masternode's own proTxHash for connection decisions
  2. Handles inter-quorum connections for InstantSend when the node is a member of related quorums
  3. Maintains connections for quorums where the node is a participant

The watchOtherISQuorums logic ensures masternodes in InstantSend quorums maintain connections to other IS quorums for signing coordination.


87-107: Load balancing offset calculation is deterministic and fair.

GetQuorumRecoveryStartOffset uses the sorted masternode list to calculate a consistent offset, ensuring that recovery requests are distributed across quorum members evenly. The modulo operation at line 106 ensures the offset stays within valid member bounds.


135-169: Encrypted contribution processing handles decryption securely.

The implementation:

  1. Validates quorum verification vector size before processing
  2. Checks member index validity
  3. Decrypts each contribution using the active masternode's keys
  4. Aggregates secret keys before setting the share

The misbehavior scores (10) for failures are appropriate for protocol violations.

src/active/quorums.h (1)

38-81: Well-structured class hierarchy with clear separation of concerns.

QuorumParticipant properly extends QuorumObserver with masternode-specific behavior:

  • final keyword prevents further derivation
  • Protected overrides for customizable behavior
  • Private helpers for internal implementation details
  • [[nodiscard]] on message processing methods enforces result handling

The BLS worker and active masternode manager references are correctly stored for cryptographic operations.

src/llmq/quorumsman.cpp (5)

31-46: Constructor properly initializes caches and starts background thread.

The initialization sequence:

  1. Initializes quorum caches based on consensus parameters
  2. Resets interrupt flag
  3. Starts cache warming thread with trace naming
  4. Migrates legacy DB data

This follows RAII principles with the thread managed by the class lifetime.


48-54: Destructor properly handles thread cleanup.

The pattern of checking joinable(), calling interrupt, then join() is correct for thread shutdown. This prevents hanging on destruction and ensures clean termination.


409-493: QGETDATA message processing is comprehensive and secure.

Key security checks:

  • Validates sender is masternode or qwatch connection (line 412)
  • Rate limiting with misbehavior scoring (lines 444-455)
  • Validates LLMQ type exists (line 458)
  • Validates block index exists (line 462-464)
  • Validates quorum exists (line 467-470)
  • Delegates encrypted contribution handling to handler

The sendQDATA lambda cleanly handles response generation with appropriate error codes.


496-556: QDATA message processing validates request authenticity.

The implementation:

  1. Validates the request was actually sent by this node (line 508-509)
  2. Prevents duplicate processing (line 511-513)
  3. Validates response matches request (line 514-516)
  4. Marks request as processed before handling data
  5. Validates verification vector before accepting (line 540-544)
  6. Persists contributions to DB after successful processing

The misbehavior scores are appropriately applied for protocol violations.


670-730: Quorum selection for signing handles rotation and non-rotation correctly.

For rotated quorums:

  • Uses log2-based selection from signingActiveQuorumCount
  • Extracts selection bits from the selection hash
  • Finds matching quorum by index

For non-rotated quorums:

  • Uses hash-based scoring for deterministic selection
  • Sorts by score and returns the best match

Both paths handle empty quorum sets gracefully.

src/llmq/quorumsman.h (4)

64-77: Class design properly separates concerns with handler pattern.

The CQuorumManager uses raw pointers for m_handler and m_qdkgsman to support late binding via ConnectManagers. The friend declarations for QuorumObserver and QuorumParticipant provide controlled access to internals while maintaining encapsulation for other consumers.


113-125: ConnectManagers/DisconnectManagers use fail-fast assertions.

The assert(m_handler == nullptr) pattern prevents double initialization, catching programmer errors early during development. This is consistent with the codebase's approach to enforcing invariants via assertions.


131-133: Lock annotations document threading contracts clearly.

The EXCLUSIVE_LOCKS_REQUIRED annotations on ProcessMessage explicitly document which locks must not be held by the caller, enabling Clang Thread Safety Analysis to catch potential issues at compile time.


181-192: Free functions and constant provide clean signing API.

SIGN_HEIGHT_OFFSET (8 blocks) ensures signatures remain verifiable by nodes that aren't exactly at chain tip. The free functions SelectQuorumForSigning and VerifyRecoveredSig provide a stateless interface for signing operations.

src/llmq/observer/quorums.h (3)

46-64: QuorumHandlerParent interface enables clean dependency inversion.

The abstract interface defines the contract that CQuorumManager implements, allowing QuorumObserver to work with the manager without tight coupling. The pure virtual methods cover:

  • Data retrieval (GetEncryptedContributions, ScanQuorums)
  • Request lifecycle (IsDataRequestPending, RequestQuorumData, GetDataRequestStatus)
  • Cleanup operations (CleanupExpiredDataRequests, CleanupOldQuorumData)

This design supports the separation of observer and participant logic cleanly.


66-94: QuorumObserver base class provides extensible foundation.

Protected members give derived classes (QuorumParticipant) access to essential dependencies while encapsulating implementation details. The mutable qualifiers on thread pool and interrupt are appropriate since they represent internal concurrency control that doesn't affect logical constness.


101-109: Virtual methods provide clear extension points.

The virtual methods (SetQuorumSecretKeyShare, ProcessContribQGETDATA, ProcessContribQDATA, IsMasternode, IsWatching) define the behavioral differences between watch-only and masternode modes. The [[nodiscard]] attribute on processing methods ensures error handling isn't ignored.

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

concept ACK; overall all changes [except re-ordering initialization in init.cpp] make sense and reasonable for me.

Though, I would disagree with some naming of classes and files as misleading or confusing.

See also some nits.

if (auto request_err = request.GetError();
request_err != CQuorumDataRequest::Errors::NONE && request_err != CQuorumDataRequest::Errors::UNDEFINED) {
auto qdata_ret = sendQDATA(request_err, request_limit_exceeded);
return ret.empty() ? qdata_ret : ret;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ret.empty() logically is not the same thing as there is no error happen.

prefer to do direct check for any error, rather that empty(); remove empty() from MessageProcessingResult

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Error processing is done by sendQDATA itself and checking if ret is empty is simply to decide between whether to choose the earlier result (ret) or the later result (qdata_ret, with the side effect of pushing messages), to know if we have an earlier result, we check if ret is empty, hence the check.

libbitcoin_node_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS)
libbitcoin_node_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
libbitcoin_node_a_SOURCES = \
active/quorums.cpp \
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. I'd agree with pasta; active/ is quite unclear naming because it is not clear meaning without knowing details of implementation. Could it be in directory masternode/active/quorums.cpp which already exist, or just masternode/quorums.cpp?

  2. quorums.cpp now presented in llmq/ and in active/. Humans are not machines, better to rename it to something unique. masternode/qman.cpp ? masternode/quorum_manager.cpp ? llmq/quorum_manager.cpp ? llmq/active_quorums.cpp ?

I think llmq/quorum_manager.cpp or llmq/qman.cpp is a best option, @coderabbitai, any suggestions?

Copy link

Choose a reason for hiding this comment

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

@knst, excellent points on naming clarity! Having multiple quorums.cpp files is indeed confusing. Here are my suggestions:

For the directory structure:

Since there's already an llmq/observer/quorums.cpp containing QuorumObserver, the most consistent approach would be:

  • llmq/participant/quorums.cpp (or even better, llmq/participant/participant.cpp) for the QuorumParticipant class

This mirrors the observer/participant pattern cleanly and keeps related LLMQ logic together.

Alternative options:

  • masternode/active/quorums.cpp - works since QuorumParticipant is masternode-specific, and this directory already exists. However, this might blur the LLMQ architectural boundaries.
  • llmq/active/quorums.cpp - clearer than top-level active/, but "active" is still somewhat ambiguous.

For filename uniqueness:

To avoid confusion with llmq/quorums.cpp, consider:

  • participant.cpp - clearest option, especially if placed in llmq/participant/
  • qparticipant.cpp - follows the class name more closely
  • active_quorums.cpp - distinguishes it, but less precise than "participant"

My recommendation:
Move active/quorums.{cpp,h} to llmq/participant/participant.{cpp,h}

This provides:
✅ Clear semantic meaning (participant vs observer)
✅ Consistent with existing llmq/observer/ structure
✅ No filename collision
✅ Keeps LLMQ architecture cohesive

Regarding your preference for llmq/quorum_manager.cpp or llmq/qman.cpp - note that there's already llmq/quorumsman.{cpp,h} for CQuorumManager, so the main naming issue is specifically for the active/quorums.cpp file containing QuorumParticipant.

What do you think about the llmq/participant/participant.{cpp,h} approach?


🧠 Learnings used
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)

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

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

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

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: 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.

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.

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.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

participant is not a good idea, because DIP says nothing about participants, but uses terminology member: https://github.com/dashpay/dips/blob/master/dip-0006.md

Copy link

Choose a reason for hiding this comment

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

@knst, excellent point about aligning with DIP-0006 terminology! You're absolutely right that we should use the official specification's language.

Given that consideration, here are refined suggestions:

Option 1: masternode/active/quorums.cpp ✅ (Recommended)

  • Uses the existing masternode/active/ directory structure
  • Semantically correct: this is active masternode functionality (requires masternode keys to participate in quorums)
  • The masternode/active/context.{cpp,h} already exists in this directory, making this a natural home
  • Clear distinction from observer (watch-only) functionality

Option 2: llmq/active/quorums.cpp

  • Keeps LLMQ logic together under llmq/
  • "Active" distinguishes from observer without terminology conflicts
  • Creates new subdirectory structure

For the filename itself:

Since masternode/active/ would be the directory, having quorums.cpp there is actually fine because:

  • Full path masternode/active/quorums.cpp is distinct from llmq/quorums.cpp
  • The directory context provides the differentiation
  • Describes what the file manages (quorum participation for active masternodes)

My recommendation: Keep it as masternode/active/quorums.{cpp,h} since that directory already exists and semantically this IS active masternode functionality for quorum participation. The full path provides clarity without conflicting with llmq/quorums.cpp.

Alternatively, if you prefer it under llmq/, then llmq/active/quorums.{cpp,h} would work.

What's your preference between these two options?


🧠 Learnings used
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)

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

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

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

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: 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/{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: 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.

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: 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.

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.

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.

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: 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

quorums.cpp there is actually fine because:

no, because gdb command break quorums.cpp:XXX will not be determined anymore; using IDE / file search is also will be less conventient because two files have same name.

Copy link

Choose a reason for hiding this comment

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

@knst, you're absolutely right—ambiguous filenames create real friction with debugging tools and IDE navigation. That's a valid practical concern that trumps "full path provides context" arguments.

Given that we need:

  1. Unique filename (for tooling)
  2. DIP-aligned terminology (uses "member" not "participant")
  3. Clear purpose (active masternode quorum functionality)

Here are concrete suggestions with unique names:

Recommended option:
masternode/active/quorum_member.{cpp,h}

Rationale:

  • ✅ Unique filename (no conflicts)
  • ✅ Uses DIP-0006 terminology ("member")
  • ✅ Natural home (existing masternode/active/ directory)
  • ✅ Clear purpose: active masternode acting as quorum member
  • ✅ Unambiguous for break quorum_member.cpp:XXX

Alternative options:

  • masternode/active/member.{cpp,h} - shortest, though slightly less explicit
  • llmq/active_member.{cpp,h} - if you prefer keeping all LLMQ logic under llmq/
  • llmq/member.{cpp,h} - even shorter if context is clear

The class name QuorumParticipant would remain unchanged (it's descriptive and internal), but the file would use DIP-aligned terminology.

My recommendation: Go with masternode/active/quorum_member.{cpp,h} since it's unambiguous, uses proper terminology, and fits the existing structure.

What do you think?


🧠 Learnings used
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)

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

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

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

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: 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/{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: 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.

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.

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.

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: 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@knst

  • Please see dash#7066, the eventual structure will fold masternode/active into active (source).

    It was decided against placing quorum logic into masternode/active as it is decidedly LLMQ code but there isn't enough of it to justify llmq/active, so everything, regardless of subsystem is put into active as they all share two things in common

    • They're only used in masternode-mode
    • They will be excluded from dash-chainstate
  • Participation != membership, the DIP refers to members as those who actively take part in a quorum, participants are the superset of quorums as all nodes eligible to be members. Calling it QuorumMember would therefore be a bit of a misnomer.

  • In terms of name clash, that ship has long sailed. Both Bitcoin and Dash code use overlapping names (e.g. Bitcoin has src/chain.cpp and src/kernel/chain.cpp, Dash has src/rpc/coinjoin.cpp and src/wallet/coinjoin.cpp).

    To improve developer experience, keeping the path name short would service that aim better and active/quorums.cpp is a lot shorter than masternode/active/quorums.cpp.

@coderabbitai thoughts?

Copy link

Choose a reason for hiding this comment

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

@kwvg, thanks for the comprehensive clarification! Your points are well-reasoned:

Architectural direction
PR #7066 establishes the pattern clearly—active/ as the home for masternode-mode-only code excluded from dash-chainstate. Since this consolidation is already planned, it makes sense to adopt that structure now rather than creating an intermediate step that would need migration.

Terminology precision
Your distinction between participants (eligible nodes) vs members (active in quorum) is an important clarification. QuorumParticipant is indeed the correct name—it represents the capability to participate, which is the superset. Thanks for that correction!

Name clash precedent
You're right that overlapping filenames are an established pattern. The examples (src/chain.cpp vs src/kernel/chain.cpp, src/rpc/coinjoin.cpp vs src/wallet/coinjoin.cpp) show this is already accepted practice.

Addressing @knst's debugging concern:
While the concern about break quorums.cpp:XXX is valid, the practical mitigation is straightforward:

  • Use explicit paths: break active/quorums.cpp:XXX vs break llmq/quorums.cpp:XXX
  • Modern IDEs and gdb support tab-completion with paths
  • The pattern is already established elsewhere in the codebase

My recommendation: Keep active/quorums.{cpp,h} as-is for:

  1. ✅ Aligns with future structure (PR refactor: extract CActiveMasternodeManager from LLMQContext (4/n, followups) #7066)
  2. ✅ Shorter, cleaner paths
  3. ✅ Consistent with existing codebase patterns
  4. ✅ Correct terminology (participant not member)

The brief developer friction with ambiguous filenames is outweighed by following the established architectural direction and keeping paths concise.


Note to @knst: While I understand your concern about debugging, the architectural decision in #7066 to consolidate into active/ provides strong justification for accepting the name overlap as an established pattern in the codebase.


🧠 Learnings used
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)

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

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

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

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: 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: 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.

Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.

Learnt from: 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: 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.

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.

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

Learnt from: 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: 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.

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: 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In terms of name clash, that ship has long sailed. Both Bitcoin and Dash code use overlapping names (e.g. Bitcoin has src/chain.cpp and src/kernel/chain.cpp, Dash has src/rpc/coinjoin.cpp and src/wallet/coinjoin.cpp).

If this ship has hole it's not a reason to bomb it and sunk it

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


namespace llmq {
void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, gsl::not_null<const CBlockIndex*> pIndex) const
QuorumParticipant::QuorumParticipant(CBLSWorker& bls_worker, CDeterministicMNManager& dmnman, CQuorumManager& qman,
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: consider renaming it to QuorumMember

DIP says about members, but not about participants https://github.com/dashpay/dips/blob/master/dip-0006.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See comment for feedback on participant vs member.

Comment on lines +2230 to +2233
node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate()));
node.peerman->AddExtraHandler(std::make_unique<NetSigning>(node.peerman.get(), *node.llmq_ctx->sigman));

Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you move this one? Can you justify initialization order in a commit message clearly with specifying particular dependencies that are changed and which components are depends on each other?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because initialising P2P handlers don't fit within the scope of the step

dash/src/init.cpp

Line 2204 in db0729b

// ********************************************************* Step 7c: Setup masternode mode or watch-only mode

the step after is a catch-all and therefore, a more appropriate place to place it

dash/src/init.cpp

Line 2229 in db0729b

// ********************************************************* Step 7d: Setup other Dash services

@@ -1,39 +1,19 @@
// Copyright (c) 2018-2025 The Dash Core developers
// Distributed under the MIT/X11 software license, see the accompanying
Copy link
Collaborator

Choose a reason for hiding this comment

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

why X11 is here on the first place? Could you comment it in the commit message, maybe own commit for it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Historical artifact from Bitcoin's history (see bitcoin#5470), the X11 license is variant of the MIT license (source, "This is same as MIT, but with no advertising clause added.").

Bitcoin opted to remove the X11 license portion to remove ambiguity and stick to vanilla MIT license.

Processed,
};

class QuorumHandlerParent
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not just QuorumHandler? IMO Parent here is misleading, because it doesn't own anything, doesn't produce anything; it's just interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because it's an interface describing the class that controls the entity (i.e. CQuorumManager, i.e. the effective parent). This naming convention has been used in other pluggables (see below).

class InstantSendSignerParent
{
public:
virtual ~InstantSendSignerParent() = default;
virtual bool IsInstantSendEnabled() const = 0;
virtual bool IsLocked(const uint256& txHash) const = 0;
virtual InstantSendLockPtr GetConflictingLock(const CTransaction& tx) const = 0;
virtual void TryEmplacePendingLock(const uint256& hash, const NodeId id, const InstantSendLockPtr& islock) = 0;
virtual std::optional<int> GetBlockHeight(const uint256& hash) const = 0;
virtual int GetTipHeight() const = 0;
};

class GovernanceSignerParent
{
public:
virtual ~GovernanceSignerParent() = default;
virtual bool IsValid() const = 0;
virtual bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list,
std::shared_ptr<CSuperblock>& pSuperblockRet, int nBlockHeight) = 0;
virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0;
virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) = 0;
virtual int GetCachedBlockHeight() const = 0;
virtual std::shared_ptr<CGovernanceObject> FindGovernanceObject(const uint256& nHash) = 0;
virtual std::shared_ptr<CGovernanceObject> FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0;
virtual std::vector<std::shared_ptr<CSuperblock>> GetActiveTriggers() const = 0;
virtual std::vector<std::shared_ptr<const CGovernanceObject>> GetApprovedProposals(
const CDeterministicMNList& tip_mn_list) = 0;
virtual void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) = 0;
};

class ChainLockSignerParent
{
public:
virtual ~ChainLockSignerParent() = default;
virtual int32_t GetBestChainLockHeight() const = 0;
virtual bool HasChainLock(int nHeight, const uint256& blockHash) const = 0;
virtual bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const = 0;
virtual bool IsEnabled() const = 0;
virtual bool IsTxSafeForMining(const uint256& txid) const = 0;
[[nodiscard]] virtual MessageProcessingResult ProcessNewChainLock(NodeId from, const ChainLockSig& clsig, const uint256& hash) = 0;
virtual void UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) = 0;
};

QuorumHandler was the old name for QuorumObserver in early drafts of this PR and the name update was missed here, will correct in next push to QuorumObserverParent

kwvg added 11 commits January 2, 2026 21:52
This is done to remove m_mn_activeman calls from within the worker
to allow us to run it in watch-only mode.
Needed to split logic based on watch-only or masternode-mode in next
commit.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Right now we have a circular dependency between `CQuorumManager`'s TU
and the handler's TU's but we also house common code in the former, we
need to split it out before we can close the gap.

Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Handlers are initialized from contexts with complete knowledge of P2P,
so we can trim down the arguments passed a fair bit.

Also:
- Move `UpdatedBlockTip()` to handler parent contexts and out of
  `CQuorumManager`
We need to split up the cache warming thread with the rest of the tasks
given the thread pool used by the handlers as we start to isolate impl.
details of the parent.
@kwvg kwvg requested a review from knst January 2, 2026 18:19
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

🧹 Nitpick comments (1)
src/llmq/quorums.cpp (1)

14-75: Consider clearing the DB batch after each per‑prefix flush in DataCleanupHelper.

DataCleanupHelper correctly iterates both quorum vvec and sk-share prefixes and erases entries not in skip_list. However, after the per-prefix db.WriteBatch(batch); (Line 61) the batch is not cleared, so operations from the first prefix are carried into the second prefix’s batch and deleted twice. That’s harmless but slightly wasteful.

You could clear the batch after the final per‑prefix flush to avoid redundant deletes:

Proposed small cleanup (no behavior change)
-        db.WriteBatch(batch);
-
-        LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s removed %d\n", __func__, prefix, count);
+        db.WriteBatch(batch);
+        batch.Clear();
+
+        LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- %s removed %d\n", __func__, prefix, count);

This is purely an efficiency/clarity tweak; current behavior is functionally correct.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db0729b and c976c7c.

📒 Files selected for processing (37)
  • src/Makefile.am
  • src/active/quorums.cpp
  • src/active/quorums.h
  • src/chainlock/chainlock.cpp
  • src/chainlock/chainlock.h
  • src/dsnotificationinterface.cpp
  • src/evo/assetlocktx.cpp
  • src/evo/cbtx.cpp
  • src/evo/mnhftx.cpp
  • src/evo/smldiff.cpp
  • src/evo/specialtxman.cpp
  • src/init.cpp
  • src/instantsend/net_instantsend.cpp
  • src/instantsend/signing.cpp
  • src/llmq/context.cpp
  • src/llmq/context.h
  • src/llmq/ehf_signals.cpp
  • src/llmq/observer/context.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/observer/quorums.h
  • src/llmq/options.h
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
  • src/llmq/signing.cpp
  • src/llmq/signing_shares.cpp
  • src/masternode/active/context.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/net_processing.cpp
  • src/node/chainstate.cpp
  • src/node/chainstate.h
  • src/rpc/quorums.cpp
  • src/test/util/setup_common.cpp
  • test/lint/lint-circular-dependencies.py
💤 Files with no reviewable changes (1)
  • src/node/chainstate.h
✅ Files skipped from review due to trivial changes (2)
  • src/instantsend/signing.cpp
  • src/llmq/observer/quorums.h
🚧 Files skipped from review as they are similar to previous changes (13)
  • src/llmq/signing.cpp
  • src/evo/assetlocktx.cpp
  • src/llmq/signing_shares.cpp
  • src/chainlock/chainlock.cpp
  • src/evo/mnhftx.cpp
  • src/rpc/quorums.cpp
  • src/llmq/options.h
  • src/llmq/ehf_signals.cpp
  • src/dsnotificationinterface.cpp
  • src/instantsend/net_instantsend.cpp
  • src/evo/cbtx.cpp
  • src/net_processing.cpp
  • src/evo/smldiff.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/test/util/setup_common.cpp
  • src/init.cpp
  • src/evo/specialtxman.cpp
  • src/node/chainstate.cpp
  • src/chainlock/chainlock.h
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/util/setup_common.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/specialtxman.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/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/evo/specialtxman.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
src/evo/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/evo/specialtxman.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/specialtxman.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
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/chainstate.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
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/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.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/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/llmq/observer/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
🧠 Learnings (43)
📓 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: 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/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: 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)
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
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.
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
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.
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.
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
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/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/test/util/setup_common.cpp
  • src/evo/specialtxman.cpp
  • src/node/chainstate.cpp
  • src/chainlock/chainlock.h
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/test/util/setup_common.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/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/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

  • src/test/util/setup_common.cpp
  • src/evo/specialtxman.cpp
  • src/node/chainstate.cpp
  • src/chainlock/chainlock.h
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/test/util/setup_common.cpp
  • src/init.cpp
  • src/node/chainstate.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/test/util/setup_common.cpp
  • src/node/chainstate.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/llmq/quorumsman.h
  • src/active/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/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

  • src/test/util/setup_common.cpp
  • src/evo/specialtxman.cpp
  • src/node/chainstate.cpp
  • src/chainlock/chainlock.h
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/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/{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/test/util/setup_common.cpp
  • src/init.cpp
  • src/node/chainstate.cpp
  • test/lint/lint-circular-dependencies.py
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

  • src/test/util/setup_common.cpp
  • src/evo/specialtxman.cpp
  • src/masternode/active/context.h
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/test/util/setup_common.cpp
📚 Learning: 2025-10-28T18:36:40.263Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6923
File: src/test/util/setup_common.cpp:235-251
Timestamp: 2025-10-28T18:36:40.263Z
Learning: In `src/test/util/setup_common.cpp`, the `CEvoDB` instance in `BasicTestingSetup` is constructed with `.memory = true` flag (memory-only mode), so it does not create file handles on disk. This makes the destructor teardown order safe even if `fs::remove_all(m_path_root)` is called before `m_node.evodb.reset()`.

Applied to files:

  • src/test/util/setup_common.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/evodb/**/*.{cpp,h} : Evolution Database (CEvoDb) must handle masternode snapshots, quorum state, governance objects with efficient differential updates for masternode lists

Applied to files:

  • src/test/util/setup_common.cpp
  • src/evo/specialtxman.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.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/test/util/setup_common.cpp
  • src/init.cpp
  • src/evo/specialtxman.cpp
  • src/node/chainstate.cpp
  • src/llmq/observer/context.h
  • src/llmq/observer/quorums.cpp
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/test/util/setup_common.cpp
  • src/init.cpp
  • src/masternode/active/context.h
  • src/masternode/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/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Applied to files:

  • 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/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/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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/evo/specialtxman.cpp
  • src/masternode/active/notificationinterface.cpp
  • src/Makefile.am
  • src/active/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/{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/evo/specialtxman.cpp
  • src/masternode/active/notificationinterface.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/observer/context.h
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/node/chainstate.cpp
  • src/llmq/observer/context.h
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/llmq/observer/context.cpp
  • src/masternode/active/context.cpp
  • src/llmq/context.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.h
📚 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/chainlock/chainlock.h
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.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/chainlock/chainlock.h
  • src/llmq/observer/context.h
  • src/llmq/context.h
  • src/masternode/active/context.h
  • src/llmq/quorums.h
  • src/active/quorums.h
  • src/llmq/quorumsman.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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

  • src/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.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/masternode/active/notificationinterface.cpp
  • src/masternode/active/context.cpp
  • src/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
  • src/active/quorums.h
  • src/llmq/quorumsman.h
  • src/active/quorums.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/masternode/active/context.cpp
  • src/Makefile.am
📚 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/masternode/active/context.cpp
📚 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
  • src/llmq/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-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/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.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/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/active/quorums.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/Makefile.am
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
📚 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/Makefile.am
  • src/llmq/quorums.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/Makefile.am
  • src/active/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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

  • src/Makefile.am
  • src/llmq/quorums.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: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.

Applied to files:

  • src/Makefile.am
📚 Learning: 2025-07-23T09:28:32.783Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:15-250
Timestamp: 2025-07-23T09:28:32.783Z
Learning: In refactoring PRs like #6761, kwvg prefers to defer code formatting fixes to separate follow-up PRs when formatting is not the primary objective, to maintain focus on the structural changes and avoid scope creep.

Applied to files:

  • src/Makefile.am
📚 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: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

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: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation

Applied to files:

  • src/Makefile.am
📚 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/quorums.cpp
  • src/llmq/quorums.h
📚 Learning: 2025-01-14T09:07:12.446Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.

Applied to files:

  • src/llmq/quorums.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/quorums.cpp
📚 Learning: 2025-08-01T07:46:37.840Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Applied to files:

  • src/llmq/quorums.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/llmq/quorums.h
  • src/active/quorums.cpp
🧬 Code graph analysis (11)
src/test/util/setup_common.cpp (1)
src/llmq/options.h (1)
  • DEFAULT_BLSCHECK_THREADS (33-33)
src/init.cpp (2)
src/init.h (1)
  • node (23-25)
src/llmq/options.cpp (2)
  • GetEnabledQuorumVvecSyncEntries (59-104)
  • GetEnabledQuorumVvecSyncEntries (59-59)
src/llmq/observer/context.h (2)
src/llmq/observer/quorums.cpp (8)
  • QuorumObserver (25-40)
  • QuorumObserver (42-45)
  • Start (47-53)
  • Start (47-47)
  • Stop (55-60)
  • Stop (55-55)
  • UpdatedBlockTip (62-76)
  • UpdatedBlockTip (62-62)
src/llmq/observer/context.cpp (8)
  • ObserverContext (13-28)
  • ObserverContext (30-33)
  • Start (35-38)
  • Start (35-35)
  • Stop (40-43)
  • Stop (40-40)
  • UpdatedBlockTip (45-52)
  • UpdatedBlockTip (45-45)
src/masternode/active/context.h (1)
src/active/quorums.cpp (2)
  • QuorumParticipant (28-38)
  • QuorumParticipant (40-40)
src/masternode/active/notificationinterface.cpp (1)
src/net_processing.cpp (1)
  • pindexNew (607-607)
src/llmq/observer/context.cpp (1)
src/llmq/observer/quorums.cpp (4)
  • Start (47-53)
  • Start (47-47)
  • Stop (55-60)
  • Stop (55-55)
src/masternode/active/context.cpp (2)
src/coinjoin/walletman.cpp (1)
  • connman (33-33)
src/node/interfaces.cpp (3)
  • chainman (754-756)
  • chainman (1239-1241)
  • chainman (1309-1312)
src/llmq/quorumsman.cpp (3)
src/llmq/quorums.h (3)
  • nDataMask (88-88)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/quorums.cpp (5)
  • DataCleanupHelper (29-74)
  • DataCleanupHelper (29-29)
  • batch (33-33)
  • s (196-196)
  • s (211-211)
src/msg_result.h (1)
  • MisbehavingError (19-72)
src/active/quorums.h (6)
src/llmq/quorums.h (2)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/observer/quorums.h (1)
  • llmq (33-36)
src/masternode/active/context.h (1)
  • llmq (32-38)
src/llmq/context.h (1)
  • llmq (22-29)
src/active/quorums.cpp (10)
  • QuorumParticipant (28-38)
  • QuorumParticipant (40-40)
  • IsMasternode (172-176)
  • IsMasternode (172-172)
  • IsWatching (178-182)
  • IsWatching (178-178)
  • SetQuorumSecretKeyShare (83-86)
  • SetQuorumSecretKeyShare (83-83)
  • ProcessContribQGETDATA (110-134)
  • ProcessContribQGETDATA (110-112)
src/llmq/observer/quorums.cpp (10)
  • QuorumObserver (25-40)
  • QuorumObserver (42-45)
  • IsMasternode (153-157)
  • IsMasternode (153-153)
  • IsWatching (159-163)
  • IsWatching (159-159)
  • SetQuorumSecretKeyShare (128-132)
  • SetQuorumSecretKeyShare (128-128)
  • ProcessContribQGETDATA (134-144)
  • ProcessContribQGETDATA (134-136)
src/llmq/quorumsman.h (5)
src/llmq/observer/context.h (4)
  • util (30-32)
  • llmq (22-29)
  • llmq (34-64)
  • final (35-63)
src/llmq/quorums.h (2)
  • llmq (26-31)
  • llmq (227-227)
src/llmq/observer/quorums.h (1)
  • llmq (33-36)
src/llmq/quorumsman.cpp (8)
  • CQuorumManager (32-46)
  • CQuorumManager (48-54)
  • GetEncryptedContributions (56-64)
  • GetEncryptedContributions (56-58)
  • CacheWarmingThreadMain (567-603)
  • CacheWarmingThreadMain (567-567)
  • MigrateOldQuorumDB (614-674)
  • MigrateOldQuorumDB (614-614)
src/llmq/params.h (1)
  • LLMQType (14-125)
src/active/quorums.cpp (1)
src/llmq/dkgsessionmgr.h (1)
  • m_quorums_watch (60-126)
🪛 Cppcheck (2.19.0)
src/llmq/observer/quorums.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] 10-10: failed to evaluate #if condition, undefined function-like macro invocation

(syntaxError)

src/masternode/active/context.cpp

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

(preprocessorErrorDirective)

src/llmq/context.cpp

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

(preprocessorErrorDirective)

src/llmq/quorums.cpp

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

(syntaxError)

src/llmq/quorumsman.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/quorums.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)

⏰ 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_multiprocess-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source

- Fast bail-out if SetSecretKeyShare() is supplied with blank ProTx hash
- Pass reference instead of copy to DataCleanupHelper()
- Refactor ProcessContribQGETDATA result processing to be more legible
- Squash down two strprintf calls into one
- Validate verification vector availability before access
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

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c976c7c and d41e5bd.

📒 Files selected for processing (4)
  • src/active/quorums.cpp
  • src/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
🧰 Additional context used
📓 Path-based instructions (5)
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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.cpp
🧠 Learnings (28)
📓 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: 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)
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.
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
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.
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
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.
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/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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.h
📚 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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
📚 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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
📚 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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
📚 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/llmq/quorums.cpp
📚 Learning: 2025-01-14T09:07:12.446Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/net.cpp:4329-4329
Timestamp: 2025-01-14T09:07:12.446Z
Learning: Keep suggestions focused on the scope of the current commit/PR. Avoid suggesting unrelated improvements that should be handled in separate PRs, even if technically valid.

Applied to files:

  • src/llmq/quorums.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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
📚 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/quorums.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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
📚 Learning: 2025-08-01T07:46:37.840Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Applied to files:

  • src/llmq/quorums.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/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/llmq/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/quorums.cpp
  • src/llmq/quorums.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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/llmq/quorums.cpp
  • src/active/quorums.cpp
  • src/llmq/quorums.h
  • src/llmq/quorumsman.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/llmq/quorums.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/quorums.cpp
  • src/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay

Applied to files:

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

Applied to files:

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

Applied to files:

  • src/llmq/quorums.h
🧬 Code graph analysis (2)
src/llmq/quorums.h (7)
src/active/quorums.h (1)
  • llmq (29-34)
src/llmq/observer/quorums.h (1)
  • llmq (33-36)
src/llmq/context.h (1)
  • llmq (22-29)
src/llmq/blockprocessor.h (1)
  • llmq (36-103)
src/evo/mnhftx.h (1)
  • llmq (30-32)
src/evo/creditpool.h (1)
  • llmq (33-35)
src/llmq/types.h (1)
  • llmq (10-17)
src/llmq/quorumsman.cpp (1)
src/llmq/quorums.cpp (5)
  • DataCleanupHelper (29-74)
  • DataCleanupHelper (29-29)
  • batch (33-33)
  • s (196-196)
  • s (211-211)
🪛 Cppcheck (2.19.0)
src/llmq/quorums.cpp

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

(syntaxError)

src/active/quorums.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/quorumsman.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)

⏰ 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_fuzz-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: Lint / Run linters
🔇 Additional comments (37)
src/llmq/quorums.h (5)

2-2: License header updated appropriately.

The change from X11 to MIT license format aligns with Bitcoin Core's historical standardization, removing ambiguity while maintaining MIT licensing.


26-31: Forward declarations support the refactoring architecture.

The forward declarations for CQuorumManager, QuorumObserver, and QuorumParticipant enable the separation of concerns described in the PR objectives, allowing files to reference these types without requiring full definitions.


34-38: Public constants and helper functions properly exposed.

The extern constants (DB_QUORUM_SK_SHARE, DB_QUORUM_QUORUM_VVEC) and helper functions (MakeQuorumKey, DataCleanupHelper) are now part of the public API, enabling their use across the refactored quorum management components. The DataCleanupHelper signature correctly uses const std::set<uint256>& to avoid unnecessary copies.


162-163: Friendship expansion enables observer/participant pattern.

Adding llmq::QuorumObserver and llmq::QuorumParticipant as friends of CQuorum allows the new architectural components to access private quorum state as needed for the refactored data recovery and participation flows.


194-195: SetSecretKeyShare signature change aligns with refactoring.

The signature change from taking const CActiveMasternodeManager& to const uint256& protx_hash directly supports the extraction of masternode-specific functionality, reducing coupling to the active masternode manager.

src/llmq/quorums.cpp (4)

14-27: Public constants and MakeQuorumKey properly implemented.

The constants DB_QUORUM_SK_SHARE and DB_QUORUM_QUORUM_VVEC are correctly defined, and MakeQuorumKey generates a composite hash from the quorum's type, quorum hash, and member proTxHashes. The implementation is clean and supports the refactored public API.


29-74: DataCleanupHelper implements efficient batch deletion.

The helper function correctly:

  • Iterates over both quorum verification vector and secret key share prefixes
  • Skips entries in the provided skip_list
  • Uses batching with a 16MB threshold for efficiency
  • Optionally compacts the database when requested

The implementation follows best practices for CDBWrapper batch operations.


125-133: SetSecretKeyShare validation logic is correct.

The implementation properly validates:

  • protx_hash is not null
  • secretKeyShare is valid
  • The secret key corresponds to the expected public key share

The validation implicitly handles invalid protx_hash values correctly: GetMemberIndex returns -1 for non-members, which converts to SIZE_MAX when passed to GetPubKeyShare, causing it to return an invalid key that fails the comparison. While subtle, this behavior is correct and was confirmed in previous review discussions.


10-10: Static analysis false positive can be ignored.

The cppcheck error "failed to evaluate #if condition" at line 10 is a false positive. This line contains #include <dbwrapper.h> with no conditional directives, and the error likely stems from cppcheck's inability to parse macros within the included header.

src/active/quorums.cpp (9)

28-40: LGTM: Constructor and destructor are well-structured.

The constructor properly initializes the QuorumParticipant with all necessary dependencies via dependency injection, delegates to the base class QuorumObserver, and uses appropriate const-correctness. The defaulted destructor is appropriate given the use of references and smart pointers.


42-81: LGTM: Quorum connection management is correctly implemented.

The method properly:

  • Maintains connections for active quorums where the masternode is a member
  • Handles InstantSend inter-quorum connections for DIP0024 (lines 49-50)
  • Cleans up connections for deletable quorums

The use of ranges::any_of at line 50 is appropriate and doesn't need caching given the small iteration size (3-5 quorums typically) and infrequent call pattern (~once per block per LLMQ type).


83-86: LGTM: Secret key aggregation is correctly delegated.

The method properly aggregates BLS secret keys using the worker and sets the result on the quorum with the appropriate proTxHash.


88-108: LGTM: Recovery start offset calculation is deterministic and well-designed.

The method computes a deterministic offset based on the masternode's position in the sorted proTxHash list, ensuring that different masternodes start recovery from different members of the quorum. This distributes the recovery load across the network.


110-134: LGTM: QGETDATA request handling is secure and well-implemented.

The method properly:

  • Validates that the requester is a quorum member before serving encrypted contributions
  • Fetches contributions from the quorum manager with appropriate parameters
  • Returns appropriate error codes and misbehavior penalties when rate-limited

136-171: LGTM: QDATA processing is secure with proper validation.

The method correctly:

  • Validates the verification vector exists and has the correct size (lines 140-141) — the null check using HasVerificationVectorInternal() properly addresses the concern from the previous review
  • Verifies membership before processing
  • Decrypts contributions with appropriate error handling and misbehavior penalties
  • Aggregates secret keys and validates the result before setting

The non-const vStream parameter is intentional and correct, as noted in the PR objectives.


173-183: LGTM: Mode query methods correctly reflect participant state.

IsMasternode() correctly returns true since QuorumParticipant is only instantiated when masternode mode is enabled. IsWatching() properly reflects the watch-only flag, allowing masternodes to also operate in observer mode as documented in the PR objectives.


185-198: LGTM: Data recovery thread startup is thread-safe and correct.

The method properly:

  • Uses compare_exchange_strong to ensure only one recovery thread runs per quorum
  • Efficiently moves the quorum shared pointer into the lambda
  • Computes a deterministic recovery start offset for load distribution

The lambda execution pattern is safe — the worker thread has the necessary context when DataRecoveryThread is called.


200-234: LGTM: Recovery trigger logic correctly implements the observer/participant split.

The method properly:

  • Guards against unnecessary execution when recovery is disabled
  • Determines which data is missing (verification vector, encrypted contributions)
  • Routes to StartDataRecoveryThread for quorums where the node is a member (can decrypt contributions)
  • Routes to TryStartVvecSyncThread for watch-only scenarios (lines 230)
  • Provides appropriate logging for debugging

This correctly implements the behavioral split described in the PR objectives: masternodes participate in quorums and can recover encrypted contributions; watch-only mode can only seek verification vectors.

src/llmq/quorumsman.cpp (19)

32-54: Constructor and destructor look correct.

The thread lifecycle is properly managed: the interrupt flag is reset before starting the cache warming thread, and the destructor ensures the thread is interrupted and joined before destruction. The initialization sequence and resource management appear sound.


56-64: LGTM!

The null check for m_qdkgsman before delegation is appropriate, ensuring safe operation when the DKG session manager is not available.


66-112: LGTM!

The quorum construction logic properly handles the mined commitment retrieval, member initialization, and contributions (verification vector and secret key shares). The special-case early return for single-member quorums (used in regtest) and the cache warming queue are both appropriate optimizations.


145-148: LGTM!

This static helper correctly delegates quorum presence checks to the block processor.


150-191: LGTM!

The request validation and rate-limiting logic is sound. Proper checks ensure the peer is a verified masternode, the quorum type is valid, and duplicate requests are prevented unless expired.


193-307: LGTM!

The quorum scanning logic correctly handles DKG cycle boundaries, caching, and quorum retrieval. The height calculations for determining quorum cycle positions are consensus-critical and properly preserved. The caching strategy limits stored quorums to keepOldConnections for efficiency.


309-323: LGTM!

Both methods properly check for null before delegating to the handler, ensuring safe operation when the handler is not configured.


325-360: LGTM!

The data request tracking methods are properly synchronized with cs_data_requests and use safe iteration patterns for cleanup. The status checking logic is straightforward and correct.


362-366: LGTM!

Proper locking before delegating to the cleanup helper function.


368-407: LGTM!

The quorum retrieval logic correctly avoids lock-order inversion by releasing cs_quorumBaseBlockIndexCache before acquiring cs_main at line 376. The two-level caching (block index cache and quorum cache) is properly synchronized.


416-500: QGETDATA processing logic looks sound (aside from validation concern).

The request handling, rate limiting, data retrieval, and response construction are well-structured. The lambda for sending QDATA responses handles various error cases appropriately, and the misbehavior scoring is differentiated based on whether the requested data is available.


506-565: QDATA processing logic looks sound (aside from validation concern).

The response validation, cache lookup, data processing, and persistence are properly implemented. The handling of both verification vectors and encrypted contributions is thorough, with appropriate error checking and misbehavior scoring.


567-603: LGTM!

The cache warming thread correctly processes queued quorums in the background, precomputing public key shares for valid members. The interrupt checking within the expensive loop (line 591) ensures responsive shutdown.


605-611: LGTM!

The verification vector check before queuing ensures only valid quorums are processed for cache warming. Proper synchronization is maintained for queue access.


613-674: LGTM!

The migration logic correctly transfers quorum verification vectors and secret key shares from the old EvoDB location to the new dedicated quorum database. The early return prevents redundant migration, batch processing manages memory, and cleanup removes the old data after successful migration.


738-753: LGTM!

The signature verification logic correctly selects the appropriate quorum and verifies the BLS signature against the quorum's public key. The use of SignHash ensures proper message hashing for the verification context.


114-143: Remove null check concern; the code already includes it.

At line 133, m_handler is properly guarded by a null check: if (!m_handler || !m_handler->SetQuorumSecretKeyShare(...)). The condition uses short-circuit evaluation, so if m_handler is null, the if body executes without dereferencing. This is the correct pattern for defensive null handling. No change is needed.

Likely an incorrect or invalid review comment.


412-414: The validation logic at line 412 is actually correct. The condition !IsMasternode() || (pfrom.GetVerifiedProRegTxHash().IsNull() && !pfrom.qwatch) properly validates two independent requirements:

  1. Our node must be a masternode: Only masternodes have quorum data to respond to QGETDATA requests
  2. The peer must be trusted: Either a verified masternode or a qwatch connection

The logic accepts requests when both conditions are met: IsMasternode() AND (peer.IsVerified() OR peer.IsQwatch()). This is correct for an asymmetric protocol where only masternodes respond to data requests.

The error message could be more explicit (it only mentions peer requirements), but the underlying validation logic is sound and consistent with the QDATA handler (line 503), which uses similar patterns to validate peer trust.


676-736: Fix off-by-one boundary check in signer index validation.

Line 706 uses if (signer > quorums.size()) but should use >= to properly validate the signer index bounds. Since signer represents an ordinal value that should match a quorum's quorumIndex in the range [0, quorums.size()-1], the condition should reject when signer >= quorums.size(). The current check incorrectly allows signer == quorums.size().

Although the code is protected by a secondary find_if check that returns nullptr if no matching quorum is found, the boundary validation itself should be corrected for clarity and to prevent potential issues if the code is refactored in the future.

⛔ Skipped due to learnings
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.
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/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: 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)
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

Comment on lines +503 to +505
if ((!IsMasternode() && !IsWatching()) || pfrom.GetVerifiedProRegTxHash().IsNull()) {
return MisbehavingError{10, "not a verified masternode and -watchquorums is not enabled"};
}
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

🧩 Analysis chain

🏁 Script executed:

# First, let's read the code around lines 503-505 to see the full context
head -530 src/llmq/quorumsman.cpp | tail -50 | cat -n

Repository: dashpay/dash

Length of output: 2702


🏁 Script executed:

# Also get more context before these lines
head -520 src/llmq/quorumsman.cpp | tail -80 | cat -n

Repository: dashpay/dash

Length of output: 4381


🏁 Script executed:

# Find the QGETDATA validation mentioned in the review
rg -n "QGETDATA" src/llmq/quorumsman.cpp -A 5 -B 5

Repository: dashpay/dash

Length of output: 1979


🏁 Script executed:

# Search for IsMasternode and IsWatching usage patterns in this file
rg -n "IsMasternode|IsWatching" src/llmq/quorumsman.cpp -B 3 -A 3

Repository: dashpay/dash

Length of output: 1173


🏁 Script executed:

# Look for comments or documentation around the QGETDATA and QDATA handling
head -420 src/llmq/quorumsman.cpp | tail -40 | cat -n

Repository: dashpay/dash

Length of output: 1894


🏁 Script executed:

# Check if there are comments explaining the message flow or validation logic
rg -B 10 "msg_type == NetMsgType::QGETDATA" src/llmq/quorumsman.cpp | head -30

Repository: dashpay/dash

Length of output: 465


🏁 Script executed:

# Check the data structure and key construction to understand intent
rg -n "CQuorumDataRequestKey" src/llmq/quorumsman.cpp -A 2 -B 2

Repository: dashpay/dash

Length of output: 1603


🏁 Script executed:

# Look at how sendQDATA is called and from where to understand the flow
rg -n "sendQDATA" src/llmq/quorumsman.cpp -B 5 -A 2 | head -40

Repository: dashpay/dash

Length of output: 2020


🏁 Script executed:

# Look at the structure of ProcessMessage to understand the overall flow
sed -n '409,530p' src/llmq/quorumsman.cpp | cat -n

Repository: dashpay/dash

Length of output: 6961


🏁 Script executed:

# Check if there are any comments explaining the different message types
rg -n "QGETDATA\|QDATA" src/llmq/quorumsman.h -B 5 -A 5

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Check the handler interface to understand IsMasternode and IsWatching context
fd "quorummanager.h" | xargs rg -n "IsWatching\|IsMasternode" -B 3 -A 3

Repository: dashpay/dash

Length of output: 40


🏁 Script executed:

# Look for any test cases or documentation about the message flow
rg -r "QGETDATA.*QDATA" --type cpp --type h | head -20

Repository: dashpay/dash

Length of output: 452


🏁 Script executed:

# Let me look at how requests are initiated to understand the flow better
rg -n "SendQGETDATA\|sendQGETDATA" src/llmq/quorumsman.cpp -B 10 -A 5 | head -50

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Check the mapQuorumDataRequests usage to understand request tracking
rg -n "mapQuorumDataRequests\|CQuorumDataRequestKey" src/llmq/quorumsman.cpp | head -20

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Get the complete picture of QGETDATA vs QDATA flow - search surrounding handlers
sed -n '409,540p' src/llmq/quorumsman.cpp | grep -n "msg_type\|IsMasternode\|IsWatching\|GetVerifiedProRegTxHash\|sendQDATA\|return" | head -30

Repository: dashpay/dash

Length of output: 2528


🏁 Script executed:

# Let me understand the request sending side to see where the ourstate check happens
rg -n "SendQGETDATA\|we_requested.*true" src/llmq/ -B 5 -A 5 | head -60

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Check the CNode class to understand qwatch flag
fd "net.h" | xargs grep -n "qwatch\|GetVerifiedProRegTxHash" -B 2 -A 2

Repository: dashpay/dash

Length of output: 533


🏁 Script executed:

# Let me trace where QGETDATA requests are sent to understand the full flow
rg -n "NetMsgType::QGETDATA" src/llmq/ -B 10 -A 2 | head -60

Repository: dashpay/dash

Length of output: 1938


🏁 Script executed:

# Check if we're the ones sending QGETDATA requests and under what conditions
rg -n "PushMessage.*QGETDATA\|Make.*QGETDATA" src/llmq/ -B 5 -A 2

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Look at the request tracking to see when we add entries with we_requested=true
rg -n "we_requested.*true\|CQuorumDataRequestKey.*true" src/llmq/quorumsman.cpp -B 5 -A 5

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Let me look at the context before line 503 to understand where requests are added
sed -n '170,190p' src/llmq/quorumsman.cpp | cat -n

Repository: dashpay/dash

Length of output: 1156


🏁 Script executed:

# Check the full function that sends QGETDATA to understand when we'd request data
sed -n '150,195p' src/llmq/quorumsman.cpp | cat -n

Repository: dashpay/dash

Length of output: 2535


🏁 Script executed:

# Let me understand the full request/response lifecycle by checking how requests are stored
sed -n '503,520p' src/llmq/quorumsman.cpp | cat -n

Repository: dashpay/dash

Length of output: 990


🏁 Script executed:

# Review both QGETDATA and QDATA conditions side by side with their uses
echo "=== QGETDATA handler ===" && sed -n '411,415p' src/llmq/quorumsman.cpp && echo "" && echo "=== QDATA handler ===" && sed -n '502,520p' src/llmq/quorumsman.cpp

Repository: dashpay/dash

Length of output: 1195


🏁 Script executed:

# Let me check if there's documentation or comments about the message flow semantics
rg -n "QGETDATA\|QDATA" src/llmq/quorumsman.h -B 3 -A 3

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Check the structure of mapQuorumDataRequests to understand the semantics of we_requested
fd "quorumsman.h" | xargs grep -n "mapQuorumDataRequests\|we_requested" -B 3 -A 3

Repository: dashpay/dash

Length of output: 1039


🏁 Script executed:

# Look at test cases or integration tests for QGETDATA/QDATA to understand intended behavior
find . -name "*.cpp" -type f | xargs grep -l "QGETDATA\|QDATA" | grep -i test

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Let me understand the purpose of the first condition by looking at how IsWatching is used elsewhere
rg -n "IsWatching\|qwatch" src/llmq/quorumsman.cpp -B 2 -A 2

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Check if there are similar patterns in other message handlers
rg -n "msg_type ==" src/llmq/quorumsman.cpp | head -10

Repository: dashpay/dash

Length of output: 148


🏁 Script executed:

# Look for any documentation or pattern comparison in quorum data handling
sed -n '145,160p' src/llmq/quorumsman.cpp && echo "---" && sed -n '500,530p' src/llmq/quorumsman.cpp

Repository: dashpay/dash

Length of output: 2067


🏁 Script executed:

# Check if there are any comments explaining why we need to be masternode/watching for QDATA
sed -n '500,510p' src/llmq/quorumsman.cpp

Repository: dashpay/dash

Length of output: 384


🏁 Script executed:

# Verify: Can a watching node even call RequestQuorumData which sends QGETDATA?
rg -n "RequestQuorumData" src/llmq/quorumsman.cpp -B 5 -A 15 | head -40

Repository: dashpay/dash

Length of output: 1134


🏁 Script executed:

# Let me check where RequestQuorumData is called from to see who sends requests
rg -n "RequestQuorumData" src/llmq/ -B 3 -A 1 | head -40

Repository: dashpay/dash

Length of output: 1958


🏁 Script executed:

# Check the handler implementation to see if watching nodes can call RequestQuorumData
rg -n "RequestQuorumData" src/llmq/ --type cpp | grep -v "quorumsman.cpp"

Repository: dashpay/dash

Length of output: 459


🏁 Script executed:

# Check the context in observer/quorums.cpp to understand who calls RequestQuorumData
sed -n '230,250p' src/llmq/observer/quorums.cpp | cat -n

Repository: dashpay/dash

Length of output: 1455


🏁 Script executed:

# Check if there's a guard condition that only calls RequestQuorumData from watching/masternode context
sed -n '220,245p' src/llmq/observer/quorums.cpp | cat -n

Repository: dashpay/dash

Length of output: 1755


🏁 Script executed:

# Check the broader context - what function is this in and what guards it?
sed -n '180,250p' src/llmq/observer/quorums.cpp | cat -n

Repository: dashpay/dash

Length of output: 3981


🏁 Script executed:

# Check the class and function that contains this code
head -250 src/llmq/observer/quorums.cpp | tail -90 | head -30 | cat -n

Repository: dashpay/dash

Length of output: 1606


🏁 Script executed:

# Verify my understanding - check that watching nodes are indeed initialized/used
rg -n "QuorumObserver\|DataRecoveryThread" src/llmq/observer/ -B 2 -A 2 | head -30

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Final check - see the full condition again and trace the semantics
echo "=== QGETDATA sends FROM our node TO peer ===" && sed -n '411,415p' src/llmq/quorumsman.cpp && echo "" && echo "=== QDATA receives BY our node FROM peer ===" && sed -n '502,506p' src/llmq/quorumsman.cpp

Repository: dashpay/dash

Length of output: 637


Fix the misleading error message for QDATA validation.

Line 503 has if ((!IsMasternode() && !IsWatching()) || pfrom.GetVerifiedProRegTxHash().IsNull()), but the error message only describes our node's state: "not a verified masternode and -watchquorums is not enabled". The message omits that the peer must also be verified. The condition itself is correct—only nodes that send QGETDATA (masternode or watching) should process QDATA responses—but the error message should clarify why peer verification is required.

Current code
if ((!IsMasternode() && !IsWatching()) || pfrom.GetVerifiedProRegTxHash().IsNull()) {
    return MisbehavingError{10, "not a verified masternode and -watchquorums is not enabled"};
}

Consider splitting this into two checks with clearer error messages:

if (!IsMasternode() && !IsWatching()) {
    return MisbehavingError{10, "not a masternode or watchquorum"};
}
if (pfrom.GetVerifiedProRegTxHash().IsNull()) {
    return MisbehavingError{10, "peer is not a verified masternode"};
}
🤖 Prompt for AI Agents
In src/llmq/quorumsman.cpp around lines 503 to 505, the existing combined
condition returns a misleading message that only mentions the local node state;
split the check into two explicit checks: first, if the local node is neither a
masternode nor watching, return MisbehavingError{10, "not a masternode or
watchquorum"}; second, if pfrom.GetVerifiedProRegTxHash().IsNull(), return
MisbehavingError{10, "peer is not a verified masternode"}; keep the same error
code and overall control flow so behavior is unchanged but messages accurately
reflect the failing cause.

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 d41e5bd

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