refactor: extract CActiveMasternodeManager from LLMQContext (4/n, followups)

[kwvg]

Additional Information

• Depends on refactor: extract CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation) #7065

• llmq::DashParams was introduced to implement a review suggestion made in dash#7056 (comment)

• Most (m_observer_ctx && m_observer_ctx->Fn() || m_active_ctx && m_active_ctx->Fn()) pattern uses were eliminated in b59baac, vastly simplifying the implementation of the review suggestion made in dash#7056 (comment) through just isolating them with functions and avoiding the creation of a new interface.

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 (note: N/A)
• 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]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• refactor: consolidate Dash-specific utils between C++23 polyfills (util/std23.h) and helpers (util/helpers.h) #7098 - refactor: consolidate Dash-specific utils between C++23 polyfills (util/std23.h) and helpers (util/helpers.h)

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

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[kwvg]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

This pull request introduces a significant refactoring focused on worker count parameterization and configuration consolidation across the LLMQ and DKG subsystems. A new DashParams structure is introduced to encapsulate BLS thread count, worker count, maximum recovered signature age, and database parameters. Worker initialization is made explicit by adding worker_count parameters to Start() methods across multiple components (ActiveContext, ObserverContext, CBLSWorker, CSigSharesManager). Container types are updated throughout the codebase, replacing std::set<uint256> with Uint256HashSet and std::set<NodeId>/std::set<size_t> with unordered variants for potential performance benefits. DKG session message handling is refactored with a new ReceiveMessagePreamble template to centralize preprocessing, and debug status management is restructured with dependency injection. Additionally, locking mechanisms are consolidated in CQuorumManager by replacing separate mutexes with a unified m_cs_maps mutex.

Sequence Diagram(s)

sequenceDiagram
    participant Init as init.cpp
    participant LLMQCtx as LLMQContext
    participant ObsCtx as ObserverContext
    participant ActCtx as ActiveContext
    participant BLSWorker as CBLSWorker
    participant SigMgr as CSigSharesManager

    Init->>Init: Create DashParams with worker_count
    Init->>LLMQCtx: Constructor(dash_params)
    LLMQCtx->>BLSWorker: Start(dash_params.worker_count)
    BLSWorker->>BLSWorker: Resize workerPool
    LLMQCtx->>SigMgr: Start(dash_params.worker_count)
    SigMgr->>SigMgr: Resize worker pool
    Init->>ObsCtx: Constructor(dmnman, qsnapman, chainman)
    Init->>ObsCtx: Start(worker_count)
    ObsCtx->>ObsCtx: Initialize with explicit worker count
    Init->>ActCtx: Constructor(dmnman, qsnapman, chainman)
    Init->>ActCtx: Start(connman, peerman, worker_count)
    ActCtx->>ActCtx: Initialize with explicit worker count

Loading

sequenceDiagram
    participant PeerMgr as PeerManager
    participant NetProc as net_processing
    participant DKGSession as DKGSession
    participant Logger as CDKGLogger

    PeerMgr->>NetProc: ProcessMessage(DKG message)
    NetProc->>NetProc: DKGSessionAlreadyHave(inv)
    alt Has inventory
        NetProc->>PeerMgr: Return (skip processing)
    else New message
        NetProc->>DKGSession: DKGSessionProcessMessage()
        DKGSession->>DKGSession: ReceiveMessagePreamble(msg)
        DKGSession->>Logger: Create logger after member check
        DKGSession->>DKGSession: Hash message, insert into container
        DKGSession->>DKGSession: Update debug status
        alt should_process=true
            DKGSession->>DKGSession: Full message verification
        else should_process=false
            DKGSession->>PeerMgr: Return early, skip verification
        end
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• refactor: introduce new helpers Uint256HashMap, Uint256HashSet, Uint256LruHashMap #6851: Introduces Uint256HashSet and related type aliases, aligning with the container type refactoring in this PR that replaces std::set with Uint256HashSet throughout the codebase.

Suggested reviewers

• UdjinM6

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.42% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main objective: extracting CActiveMasternodeManager from LLMQContext as part 4 of a refactor series, which aligns with the primary changes throughout the codebase.
Description check	✅ Passed	The description explains the PR's purpose, dependency on PR 7065, introduces llmq::DashParams, and notes simplification through commit b59baac. It directly relates to the changeset's refactoring objective.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/llmq/signing.cpp (1)

182-189: Harden writeTime deserialization to avoid crashes on corrupted/unexpected DB data (keep happy-path behavior unchanged).

db->ReadDataStream(k2, writeTimeDs) succeeding doesn’t guarantee the stream contains at least 4 bytes (or is in the expected format). writeTimeDs >> writeTime; without a size/exception guard can crash cleanup/removal on DB corruption.

Proposed fix (minimal, behavior-preserving for valid entries)

     if (deleteTimeKey) {
         CDataStream writeTimeDs(SER_DISK, CLIENT_VERSION);
         if (db->ReadDataStream(k2, writeTimeDs)) {
-            uint32_t writeTime;
-            writeTimeDs >> writeTime;
-            auto k5 = std::make_tuple(std::string("rs_t"), (uint32_t) htobe32_internal(writeTime), recSig.getLlmqType(), recSig.getId());
-            batch.Erase(k5);
+            // Defensive: DB value may be corrupted or from an unexpected legacy format
+            if (writeTimeDs.size() >= sizeof(uint32_t)) {
+                try {
+                    uint32_t writeTime;
+                    writeTimeDs >> writeTime;
+                    auto k5 = std::make_tuple(std::string("rs_t"), (uint32_t)htobe32_internal(writeTime), recSig.getLlmqType(), recSig.getId());
+                    batch.Erase(k5);
+                } catch (const std::exception&) {
+                    // ignore; leave rs_t cleanup to CleanupOldRecoveredSigs()
+                }
+            }
         }
     }

src/llmq/dkgsession.cpp (1)

1-1: Fix clang-format issues before merging.

The CI pipeline reports clang-format differences. Run clang-format-diff.py to fix formatting in this file.

🤖 Fix all issues with AI agents

In @src/llmq/utils.h:
- Around line 65-67: The header declares std::unordered_set in the public API
(CalcDeterministicWatchConnections) but does not include <unordered_set>; add an
explicit #include <unordered_set> to the top of the header (next to the other
standard includes) so the declaration does not rely on transitive includes and
consumers can compile independently.

🧹 Nitpick comments (4)

src/llmq/ehf_signals.h (1)

8-13: Include addition is fine; consider dropping <set> if it’s now unused in this header.

src/llmq/utils.cpp (1)

761-768: Thread-safe lazy initialization for qwatch connection seed is functionally correct but could be improved.

The current pattern checks the atomic flag outside the lock but doesn't re-check inside. Multiple threads could sequentially generate seeds. While this is acceptable for random seed generation (any random value is valid), a cleaner double-checked locking pattern would be:

if (!qwatchConnectionSeedGenerated) {
    LOCK(qwatchConnectionSeedCs);
    if (!qwatchConnectionSeedGenerated) {  // Re-check after acquiring lock
        qwatchConnectionSeed = GetRandHash();
        qwatchConnectionSeedGenerated = true;
    }
}

This is a minor improvement and the current code is functionally correct for its purpose. Based on learnings, this refinement could be deferred to a follow-up.

src/llmq/dkgsession.cpp (2)

620-636: Consider adding a defensive default case inside the switch.

The switch statement covers all current MsgPhase values, and the assert(false) after the lambda provides a runtime check. However, for improved maintainability and compiler diagnostics, consider either:

1. Adding default: assert(false); inside the switch, or
2. Using std::unreachable() (C++23) or compiler-specific __builtin_unreachable() to help the compiler with control flow analysis

This would allow compilers to warn if a new enum value is added but not handled.

♻️ Optional improvement

     auto& member_set = [&]() -> Uint256HashSet& {
         switch (phase) {
         case MsgPhase::Contribution:
             inv_type = MSG_QUORUM_CONTRIB;
             msg_name = "contribution";
             return member->contributions;
         case MsgPhase::Complaint:
             inv_type = MSG_QUORUM_COMPLAINT;
             msg_name = "complaint";
             return member->complaints;
         case MsgPhase::Justification:
             inv_type = MSG_QUORUM_JUSTIFICATION;
             msg_name = "justification";
             return member->justifications;
+        default:
+            assert(false);
         }
-        assert(false);
     }();

---

640-671: Logic flow is correct but comment could be clearer.

The two size checks serve different purposes:

1. Line 640: size >= 2 - Rejects the 3rd+ message entirely (before emplacing)
2. Line 665: size > 1 - After emplacing, if size is now >1, marks member as bad but still relays

This correctly allows up to 2 messages to be relayed while marking the sender as bad after the 2nd. The comment on line 641 mentions "justifications" but this template handles all three message types. Consider updating the comment to be generic.

📝 Minor comment fix

     if (member_set.size() >= 2) {
-        // only relay up to 2 justifications, that's enough to let the other members know about his bad behavior
+        // only relay up to 2 messages per member, that's enough to let the other members know about their bad behavior
         return std::nullopt;
     }

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7482956 and 2262e22.

📒 Files selected for processing (32)

• src/active/context.cpp
• src/active/dkgsession.cpp
• src/active/dkgsession.h
• src/active/dkgsessionhandler.cpp
• src/llmq/core_write.cpp
• src/llmq/debug.cpp
• src/llmq/debug.h
• src/llmq/dkgsession.cpp
• src/llmq/dkgsession.h
• src/llmq/dkgsessionhandler.h
• src/llmq/ehf_signals.h
• src/llmq/observer/context.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/observer/quorums.h
• src/llmq/quorums.cpp
• src/llmq/quorums.h
• src/llmq/quorumsman.cpp
• src/llmq/quorumsman.h
• src/llmq/signing.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/net.cpp
• src/net.h
• src/rpc/client.cpp
• src/rpc/evo.cpp
• src/rpc/governance.cpp
• src/rpc/masternode.cpp
• src/rpc/quorums.cpp
• src/rpc/util.cpp
• src/rpc/util.h
• src/test/rpc_tests.cpp
• test/functional/feature_llmq_dkgerrors.py

💤 Files with no reviewable changes (2)

• src/rpc/util.cpp
• src/rpc/util.h

🧰 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/llmq/dkgsessionhandler.h
• src/rpc/masternode.cpp
• src/net.cpp
• src/rpc/evo.cpp
• src/active/dkgsession.h
• src/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/net.h
• src/llmq/quorumsman.cpp
• src/rpc/client.cpp
• src/llmq/observer/quorums.h
• src/llmq/signing.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/rpc/governance.cpp
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/active/dkgsession.cpp
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/active/context.cpp
• src/test/rpc_tests.cpp
• src/llmq/ehf_signals.h
• src/rpc/quorums.cpp
• src/llmq/debug.h
• src/llmq/debug.cpp
• src/active/dkgsessionhandler.cpp
• src/llmq/core_write.cpp

src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/llmq/dkgsessionhandler.h
• src/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/quorumsman.cpp
• src/llmq/observer/quorums.h
• src/llmq/signing.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/llmq/debug.h
• src/llmq/debug.cpp
• src/llmq/core_write.cpp

src/{masternode,llmq}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/llmq/dkgsessionhandler.h
• src/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/quorumsman.cpp
• src/llmq/observer/quorums.h
• src/llmq/signing.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/llmq/debug.h
• src/llmq/debug.cpp
• src/llmq/core_write.cpp

src/llmq/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Files:

• src/llmq/dkgsessionhandler.h
• src/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/quorumsman.cpp
• src/llmq/observer/quorums.h
• src/llmq/signing.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/llmq/debug.h
• src/llmq/debug.cpp
• src/llmq/core_write.cpp

src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Files:

• src/llmq/dkgsessionhandler.h
• src/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/quorumsman.cpp
• src/llmq/observer/quorums.h
• src/llmq/signing.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/llmq/debug.h
• src/llmq/debug.cpp
• src/llmq/core_write.cpp

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node

Files:

• test/functional/feature_llmq_dkgerrors.py

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/rpc_tests.cpp

🧠 Learnings (31)

📓 Common learnings

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

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

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

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

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

📚 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/dkgsessionhandler.h
• src/active/dkgsession.h
• src/net.h
• src/llmq/observer/quorums.h
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/quorums.h
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/llmq/debug.h

📚 Learning: 2025-10-03T11:29:36.358Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/governance/signing.cpp:84-85
Timestamp: 2025-10-03T11:29:36.358Z
Learning: After bitcoin#25153 (included in dash#6775), UniValue uses the templated getInt<T>() API instead of the legacy get_int() and get_int64() methods. For example, use jproposal["start_epoch"].getInt<int64_t>() to retrieve int64_t values from UniValue objects.

Applied to files:

• src/rpc/masternode.cpp
• src/rpc/evo.cpp
• src/rpc/governance.cpp

📚 Learning: 2025-08-08T07:01:47.332Z

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

Applied to files:

• src/rpc/evo.cpp
• src/rpc/client.cpp
• src/test/rpc_tests.cpp
• src/rpc/quorums.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

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

Applied to files:

• src/rpc/evo.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/quorumsman.cpp
• src/llmq/dkgsession.h

📚 Learning: 2025-12-02T12:59:28.247Z

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

Applied to files:

• src/rpc/evo.cpp
• src/rpc/quorums.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/llmq/observer/quorums.cpp
• src/net.h
• src/llmq/quorumsman.cpp
• src/llmq/observer/quorums.h
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/quorums.h
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/rpc/quorums.cpp
• src/llmq/debug.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/llmq/quorums.cpp
• src/llmq/observer/quorums.cpp
• src/net.h
• src/llmq/quorumsman.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/ehf_signals.h
• src/rpc/quorums.cpp
• src/llmq/debug.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/net.h
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/ehf_signals.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/net.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,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/net.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/net.h
• src/llmq/dkgsession.h
• src/active/dkgsession.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/net.h
• src/llmq/quorumsman.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/dkgsession.cpp
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/debug.h

📚 Learning: 2025-10-21T11:09:34.688Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.

Applied to files:

• src/net.h
• src/llmq/quorumsman.cpp
• src/llmq/quorumsman.h

📚 Learning: 2025-09-09T21:36:11.833Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:11.833Z
Learning: In RawSender class (src/stats/rawsender.cpp), cs_net is the appropriate mutex for protecting socket access (m_sock) and network operations, not additional custom locks. The implementation correctly uses cs_net with GUARDED_BY annotations and EXCLUSIVE_LOCKS_REQUIRED to synchronize socket access between SendDirectly() and ReconnectThread().

Applied to files:

• src/net.h
• src/llmq/quorumsman.h

📚 Learning: 2025-09-09T21:36:11.833Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:11.833Z
Learning: In RawSender class (src/stats/rawsender.cpp), cs_net is the appropriate mutex for protecting socket access (m_sock) and network operations, not additional custom locks.

Applied to files:

• src/net.h

📚 Learning: 2025-09-09T21:36:58.969Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:223-232
Timestamp: 2025-09-09T21:36:58.969Z
Learning: In RawSender class (src/stats/rawsender.cpp), socket operations are properly synchronized using the cs_net mutex. The m_sock and m_server members are GUARDED_BY(cs_net), and methods like Connect(), SendDirectly(), and ReconnectThread() use appropriate locking with cs_net to prevent race conditions on socket access.

Applied to files:

• src/net.h

📚 Learning: 2025-09-09T21:36:31.504Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:31.504Z
Learning: In RawSender class (src/stats/rawsender.cpp), m_sock access is protected by the cs_net mutex. This mutex coordinates between the TCP send path in SendDirectly() and the reconnection operations in Connect()/Reconnect() methods, ensuring proper synchronization of socket state.

Applied to files:

• src/net.h

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety

Applied to files:

• src/net.h
• src/llmq/dkgsession.h

📚 Learning: 2025-09-09T21:36:31.504Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:31.504Z
Learning: In RawSender class (src/stats/rawsender.cpp), m_sock access requires synchronization using the cs_net mutex. The race condition exists between SendDirectly() (callable from any thread) and Connect() (called by ReconnectThread), where both read and write m_sock concurrently. The cs_net mutex properly coordinates these network operations.

Applied to files:

• src/net.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/llmq/quorumsman.cpp
• src/llmq/quorumsman.h
• src/llmq/dkgsession.cpp
• src/llmq/observer/context.cpp
• src/llmq/utils.cpp
• src/active/context.cpp
• src/rpc/quorums.cpp
• src/llmq/debug.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/quorumsman.cpp
• src/llmq/observer/context.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:

• test/functional/feature_llmq_dkgerrors.py
• src/rpc/quorums.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

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

Applied to files:

• src/llmq/quorumsman.h
• src/llmq/dkgsession.h
• src/llmq/utils.h

📚 Learning: 2025-11-13T20:02:55.480Z

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

Applied to files:

• src/llmq/dkgsession.h
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/llmq/debug.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/llmq/utils.cpp
• src/llmq/utils.h
• src/rpc/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: 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/active/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/active/context.cpp

📚 Learning: 2025-01-14T09:06:19.717Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/test/fuzz/netaddress.cpp:83-84
Timestamp: 2025-01-14T09:06:19.717Z
Learning: In fuzzer harness tests, CServiceHash can be used with both default constructor (CServiceHash()) and parameterized constructor (CServiceHash(salt_k0, salt_k1)) to test different variants. The usage pattern CServiceHash()(service) and CServiceHash(0, 0)(service) is valid and intentional in such tests.

Applied to files:

• src/llmq/ehf_signals.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/rpc/quorums.cpp

📚 Learning: 2025-07-15T14:53:04.819Z

Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.

Applied to files:

• src/rpc/quorums.cpp

🧬 Code graph analysis (14)

src/rpc/evo.cpp (1)

src/evo/providertx.h (1)

• nReason (258-300)

src/net.h (1)

src/net.cpp (2)

• AddPendingProbeConnections (4458-4462)
• AddPendingProbeConnections (4458-4458)

test/functional/feature_llmq_dkgerrors.py (1)

test/functional/test_framework/test_framework.py (2)

• get_node (1223-1228)
• mine_quorum (2108-2196)

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

src/llmq/quorumsman.cpp (2)

• CleanupOldQuorumData (362-366)
• CleanupOldQuorumData (362-362)

src/llmq/quorumsman.h (1)

src/llmq/quorumsman.cpp (10)

• ScanQuorums (193-197)
• ScanQuorums (193-193)
• ScanQuorums (199-307)
• ScanQuorums (199-201)
• CleanupOldQuorumData (362-366)
• CleanupOldQuorumData (362-362)
• GetQuorum (368-389)
• GetQuorum (368-368)
• GetQuorum (391-407)
• GetQuorum (391-391)

src/llmq/dkgsession.h (2)

src/llmq/dkgsession.cpp (5)

• ReceiveMessagePreamble (612-675)
• ReceiveMessagePreamble (612-612)
• ReceiveMessagePreamble (677-677)
• ReceiveMessagePreamble (678-678)
• ReceiveMessagePreamble (679-679)

src/active/dkgsessionhandler.cpp (1)

• logger (336-336)

src/rpc/governance.cpp (4)

src/qt/rpcconsole.cpp (2)

• request (421-475)
• request (421-421)

src/rpc/request.cpp (1)

• request (31-31)

src/rpc/server.cpp (1)

• request (552-552)

src/governance/vote.h (1)

• nTime (69-69)

src/llmq/dkgsession.cpp (2)

src/llmq/debug.h (1)

• phase (60-60)

src/llmq/commitment.h (1)

• quorumIndex (56-56)

src/llmq/quorums.h (1)

src/llmq/quorums.cpp (2)

• DataCleanupHelper (29-74)
• DataCleanupHelper (29-29)

src/active/dkgsession.cpp (1)

src/active/dkgsessionhandler.cpp (1)

• logger (336-336)

src/llmq/utils.h (2)

src/llmq/utils.cpp (2)

• CalcDeterministicWatchConnections (757-777)
• CalcDeterministicWatchConnections (757-759)

src/llmq/params.h (1)

• LLMQType (14-125)

src/test/rpc_tests.cpp (5)

src/rpc/quorums.cpp (1)

• result (475-475)

src/rpc/mining.cpp (1)

• result (872-872)

src/rpc/blockchain.cpp (3)

• result (139-139)
• result (277-277)
• result (564-564)

src/bitcoin-cli.cpp (4)

• result (329-329)
• result (340-340)
• result (474-474)
• result (715-715)

src/rpc/node.cpp (7)

• result (429-429)
• result (503-503)
• result (590-590)
• result (678-678)
• result (743-743)
• result (1015-1015)
• result (1094-1094)

src/llmq/debug.h (2)

src/llmq/debug.cpp (4)

• CDKGDebugManager (110-116)
• CDKGDebugManager (118-118)
• GetSessionCount (120-123)
• GetSessionCount (120-120)

src/llmq/core_write.cpp (14)

• GetJsonHelp (20-78)
• GetJsonHelp (20-20)
• GetJsonHelp (81-95)
• GetJsonHelp (81-81)
• GetJsonHelp (97-114)
• GetJsonHelp (97-97)
• GetJsonHelp (134-142)
• GetJsonHelp (134-134)
• GetJsonHelp (153-178)
• GetJsonHelp (153-153)
• GetJsonHelp (222-234)
• GetJsonHelp (222-222)
• GetJsonHelp (255-266)
• GetJsonHelp (255-255)

src/llmq/debug.cpp (1)

src/llmq/debug.h (1)

• CDKGDebugManager (91-124)

🪛 Cppcheck (2.19.0)

src/llmq/core_write.cpp

[error] 80-80: Uninitialized variable

(legacyUninitvar)

🪛 GitHub Actions: Clang Diff Format Check

src/llmq/dkgsession.h

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

src/llmq/dkgsession.cpp

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

src/rpc/quorums.cpp

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

src/llmq/debug.h

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

src/active/dkgsessionhandler.cpp

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

🔇 Additional comments (59)

src/rpc/client.cpp (2)

258-258: LGTM: Clear documentation of index offset.

The comment accurately explains that compound RPC parameter indices are offset by one to account for the subcommand occupying position 0 in the parameter array.

---

259-313: All parameter indices in the RPC conversion entries are correct and match the actual RPC method signatures. No changes needed.

src/rpc/governance.cpp (4)

179-180: LGTM - Consistent use of getInt() API.

The migration from ParseInt32V/ParseInt64V to getInt<int>() and getInt<int64_t>() aligns with the UniValue templated API introduced in bitcoin#25153 (dash#6775). Based on learnings.

---

228-228: LGTM - Type consistency verified.

The collateralIndex is correctly parsed as int and subsequently cast to uint32_t on line 232, matching the existing pattern.

---

281-281: LGTM - Proper handling of optional parameter.

The ternary correctly handles the empty params case with a default of 10, and the negative check on line 282 provides appropriate validation.

---

364-365: LGTM - Consistent with gobject_prepare parsing.

The nRevision and nTime parsing matches the pattern used in gobject_prepare (lines 179-180), maintaining consistency across related RPC handlers.

src/rpc/evo.cpp (4)

716-716: LGTM - Proper negative value validation before cast.

The pattern of parsing as int, validating collateralIndex < 0, then casting to uint32_t on line 721 is correct and safe.

---

1269-1274: LGTM - Proper bounds validation for reason code.

The nReason is correctly validated against both lower bound (< 0) and upper bound (CProUpRevTx::REASON_LAST) before casting to uint16_t. This matches the uint16_t nReason field in CProUpRevTx as shown in the relevant code snippets from src/evo/providertx.h.

---

1455-1455: LGTM - Consistent height parsing with validation.

The height is properly validated on line 1456 against chain bounds before use.

---

1488-1488: LGTM - Consistent pattern with wallet list variant.

This matches the same parsing and validation pattern used on lines 1455-1458 for the wallet case, maintaining consistency within the function.

src/net.h (3)

36-38: LGTM - Required includes for type migration.

The saltedhasher.h include provides the Uint256HashSet type definition needed for the container type changes below.

---

1469-1469: LGTM - Consistent container type migration.

The parameter type change from std::set<uint256> to Uint256HashSet aligns with the member variable change on line 1786 and the implementation in net.cpp that uses range insert.

---

1786-1786: LGTM - Performance optimization for pending probes.

Changing from std::set<uint256> to Uint256HashSet provides O(1) average-case lookup/insert complexity instead of O(log n). Since probe connections don't require ordering, an unordered set is appropriate here. This aligns with the broader PR migration pattern across DKG and quorum-related components.

src/active/context.cpp (1)

38-41: DI construction for CDKGDebugManager looks consistent with the refactor.

No concerns with the new constructor wiring; it also keeps dkgdbgman initialized before qdkgsman/handlers that may use it.

src/llmq/ehf_signals.h (1)

32-37: Uint256HashSet for ids looks appropriate for the described “filter” usage.

Assuming callers don’t require ordered iteration, this is a good perf-oriented refactor.

src/test/rpc_tests.cpp (1)

604-616: Test expectation update to getInt<int>() is aligned with typed RPC conversion.

src/net.cpp (1)

4458-4462: Self-aliasing risk is theoretical but extremely unlikely; iterator invalidation concern is incorrect.

The function is called only once in the codebase (src/llmq/utils.cpp:873) with a locally-constructed Uint256HashSet probeConnections, making self-aliasing with masternodePendingProbes impossible in practice.

More importantly, the iterator invalidation risk described is not present: the code iterates over the source range (proTxHashes) while inserting into the destination (masternodePendingProbes). Rehashing of the destination set does not invalidate iterators to the source, so there is no UB here. Iterator invalidation would only occur if you iterated over the same unordered_set being rehashed during insertion.

The proposed hardening with reserve() and the self-aliasing check are defensive in the wrong direction—if self-aliasing did occur, you would still need to copy the source before inserting to avoid modifying the container mid-iteration. Given the single, fixed call site with a local variable, this mitigation adds complexity without addressing a realistic threat.

src/llmq/quorums.h (1)

38-38: LGTM! Performance optimization with hash-based container.

The migration from std::set<uint256> to Uint256HashSet is well-suited for the skip list use case, providing O(1) membership checks instead of O(log n). The implementation in src/llmq/quorums.cpp confirms this is used exclusively for lookups without requiring ordering.

src/rpc/masternode.cpp (1)

385-385: LGTM! Type-safe parameter extraction.

The migration to getInt<int64_t>() provides type-safe extraction with proper null-check handling via the ternary operator. This aligns with the modern UniValue API introduced in bitcoin#25153.

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

337-363: LGTM! Consistent container migration for performance.

The migration to Uint256HashSet for both dbKeysToSkip and quorum_keys is appropriate. The containers are used exclusively for membership tracking and merging, where hash-based operations provide O(1) performance vs O(log n) for ordered sets. The merge() operation is fully supported in C++17+ for std::unordered_set.

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

22-22: LGTM! Dependency injection for improved testability.

The change from default construction to parameterized construction CDKGDebugManager(dmnman, qsnapman, chainman) makes dependencies explicit and improves testability. The required references are already available from the constructor parameter list.

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

62-62: LGTM! Interface updated for container migration.

The pure virtual method signature correctly updated to match the Uint256HashSet migration. The implementation in src/llmq/quorumsman.cpp:361 confirms the interface change is consistently applied across all implementations.

test/functional/feature_llmq_dkgerrors.py (1)

40-40: LGTM! Numeric argument type alignment.

The conversion from string literals to integer literals for quorum command numeric arguments is consistent across all test cases and aligns with the RPC parameter parsing refactor (switching from ParseInt32V/ParseInt64V to direct .getInt<>() calls).

Also applies to: 45-46, 51-51, 59-60, 65-67, 72-73, 78-79

src/llmq/dkgsessionhandler.h (1)

70-70: LGTM! Performance optimization via hash-based container.

Replacing std::set<uint256> with Uint256HashSet provides O(1) lookup instead of O(log n) for message deduplication, which is appropriate since message ordering is not required for tracking seen messages.

src/llmq/quorums.cpp (1)

29-29: LGTM! Container type migration for DataCleanupHelper.

The parameter type change to Uint256HashSet is compatible with the existing usage at line 50 (skip_list.find(...)) and aligns with the broader performance optimization effort.

src/llmq/quorumsman.cpp (2)

85-85: LGTM! Mutex consolidation simplifies quorum cache locking.

The consolidation of cs_map_quorums and cs_scan_quorums into a single m_cs_maps mutex simplifies the locking logic and ensures consistent protection for both mapQuorumsCache and scanQuorumsCache.

Note: This consolidation may increase lock contention if map and scan operations were previously parallelizable, but it eliminates potential lock-ordering issues and reduces complexity.

Also applies to: 109-109, 233-233, 296-296, 402-402, 533-533

---

362-362: LGTM! Parameter type change aligns with container migration.

The signature change to accept const Uint256HashSet& instead of const std::set<uint256>& is consistent with the broader performance optimization effort across LLMQ code.

src/active/dkgsession.h (1)

46-46: LGTM! Consistent parameter type migration.

The SendJustification parameter type change from const std::set<uint256>& to const Uint256HashSet& aligns with the broader container migration for performance optimization. All call-sites have been properly updated to pass Uint256HashSet, as confirmed by the implementation at src/active/dkgsession.cpp:307-336 where the justifyFor variable is correctly declared as Uint256HashSet and passed to the method.

src/active/dkgsession.cpp (3)

46-47: LGTM: Logger initialization moved after membership check.

The logger is now created after the AreWeMember() check and threshold assertion, avoiding unnecessary construction when the early return paths are taken.

---

307-307: LGTM: Container type update to Uint256HashSet.

The change from std::set<uint256> to Uint256HashSet improves lookup performance for member justification tracking. The usage with emplace() and count() remains semantically equivalent.

---

340-341: LGTM: SendJustification signature updated consistently.

The parameter type change to const Uint256HashSet& aligns with the caller in VerifyAndJustify and the header declaration.

src/active/dkgsessionhandler.cpp (2)

253-260: LGTM: Container type changes for batch signature verification.

The function signature and internal containers are updated from ordered to unordered sets. This is appropriate since ordering is not required for tracking bad nodes or message hashes, and hash-based containers provide better average-case lookup performance.

---

266-266: LGTM: messageHashesSet uses Uint256HashSet.

Consistent with the PR-wide migration to hash-based containers for uint256 collections.

src/llmq/dkgsession.h (4)

9-9: LGTM: Added protocol.h include.

Required for CInv type used in the new ReceiveMessageState struct.

---

218-224: LGTM: CDKGMember container types updated to Uint256HashSet.

All member tracking containers (contributions, complaints, justifications, prematureCommitments, badMemberVotes, complaintsFromOthers) now use hash-based sets. This aligns with the coding guidelines for using efficient containers in LLMQ data structures.

---

284-296: LGTM: Internal helper types for message reception.

The MsgPhase enum and ReceiveMessageState struct support the new ReceiveMessagePreamble template, centralizing common message handling logic and reducing code duplication across ReceiveMessage overloads.

---

417-420: LGTM: ReceiveMessagePreamble template declaration.

The template with EXCLUSIVE_LOCKS_REQUIRED(invCs) annotation correctly documents the locking requirements for the centralized message preamble processing.

src/llmq/core_write.cpp (1)

80-81: LGTM: GetJsonHelp moved to CDKGDebugManager.

The function scope change from CDKGDebugStatus to CDKGDebugManager aligns with the refactor consolidating debug status reporting. The added inner_optional parameter provides flexibility for optional inner field documentation.

Note: The Cppcheck "uninitialized variable" warning at line 80 appears to be a false positive—this is a static method that returns a fully constructed RPCResult object.

src/llmq/utils.cpp (2)

757-770: LGTM: Return type and container updated to unordered_set.

The function returns connection indices where ordering is not required, making std::unordered_set<size_t> appropriate.

---

845-845: LGTM: probeConnections uses Uint256HashSet.

Consistent with PR-wide container type migration.

src/rpc/quorums.cpp (5)

72-72: LGTM: Direct UniValue parameter parsing.

Using request.params[0].getInt<int>() instead of ParseInt32V() is cleaner and leverages UniValue's built-in type conversion with proper error handling.

---

277-277: LGTM: llmqType parsing updated consistently.

The pattern static_cast<Consensus::LLMQType>(request.params[0].getInt<int>()) is used consistently across all RPC methods that accept llmqType.

---

300-300: LGTM: Updated to CDKGDebugManager::GetJsonHelp.

Reflects the API consolidation where JSON help generation moved to the debug manager.

---

349-358: LGTM: Debug status JSON generation refactored.

The refactor from constructing CDKGDebugStatus separately to using debugman->ToJson(detailLevel) simplifies the code and centralizes the JSON generation logic in the debug manager.

---

1006-1006: LGTM: Using GetSessionCount() accessor.

The new accessor method properly encapsulates the session count retrieval with appropriate locking.

src/llmq/debug.h (4)

48-48: LGTM: complaintsFromMembers uses std::unordered_set.

The container change from std::set<uint16_t> to std::unordered_set<uint16_t> is appropriate since member indices don't require ordering for complaint tracking.

---

86-89: LGTM: CDKGDebugStatus simplified to struct.

Changing from class to struct with only data members is appropriate since all methods were moved to CDKGDebugManager. The simpler data structure clearly conveys its role as a plain data container.

---

93-106: LGTM: CDKGDebugManager constructor with dependency injection.

The refactor introduces proper dependency injection, storing references to CDeterministicMNManager, CQuorumSnapshotManager, and ChainstateManager. This aligns with the broader refactoring goal of extracting components from LLMQContext and follows the project's initialization patterns seen in other files.

---

119-123: LGTM: New accessor methods with proper lock annotations.

• GetSessionCount() provides encapsulated access to session count
• GetJsonHelp() is appropriately static since it doesn't depend on instance state
• ToJson() has the correct EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus) annotation indicating it acquires the lock internally

The API is clean and well-documented.

src/llmq/quorumsman.h (3)

87-91: Mutex consolidation looks good.

Consolidating cs_map_quorums and cs_scan_quorums into a single m_cs_maps mutex simplifies the locking strategy. Looking at the relevant snippets from quorumsman.cpp, the ScanQuorums method accesses both caches within the same logical operation (lines 231-262 and 296-302), so having a single mutex is appropriate and reduces the risk of lock ordering issues.

---

161-161: Signature change to Uint256HashSet aligns with guidelines.

Switching from std::set<uint256> to Uint256HashSet provides O(1) lookup complexity, which is appropriate for membership checks during cleanup operations. As per coding guidelines, this follows the preference for unordered containers in Dash-specific data structures.

---

15-15: Include addition is necessary.

The saltedhasher.h include is required for Uint256HashSet type used in the CleanupOldQuorumData signature.

src/llmq/dkgsession.cpp (3)

206-221: Clean extraction of common logic to preamble.

The refactoring properly delegates pre-processing (hash computation, relay decision, status updates) to ReceiveMessagePreamble, and the contribution-specific logic (vvec storage, decryption, verification) remains in ReceiveMessage. The timer t2 is correctly placed to measure only the contribution-specific processing.

---

361-361: Good performance improvement.

Switching from std::set<size_t> to std::unordered_set<size_t> improves the duplicate detection check from O(log n) to O(1), which is appropriate for this DoS protection code path.

---

677-679: Explicit instantiations are complete.

The three explicit template instantiations correctly cover all message types that use ReceiveMessagePreamble. CDKGPrematureCommitment intentionally has different handling and doesn't use this template.

src/llmq/debug.cpp (4)

110-116: Dependency injection improves design.

The new constructor that accepts CDeterministicMNManager&, CQuorumSnapshotManager&, and const ChainstateManager& improves testability and makes dependencies explicit. This aligns with the broader refactoring goals of the PR.

---

120-123: Simple and thread-safe accessor.

The GetSessionCount() method correctly uses WITH_LOCK to access localStatus.sessions.size() under the mutex.

---

135-146: Session iteration correctly uses internal state and injected dependencies.

The loop properly iterates over localStatus.sessions and passes the stored member references (m_dmnman, m_qsnapman, m_chainman) to CDKGDebugSessionStatus::ToJson. This ensures consistent behavior regardless of where ToJson is called from.

---

125-150: Lock ordering is consistent and safe. The ToJson method acquires cs_lockStatus, then calls CDKGDebugSessionStatus::ToJson which uses WITH_LOCK to acquire cs_main. This establishes a consistent one-way lock order (cs_lockStatus → cs_main) with no reverse dependency. Public methods are properly annotated with EXCLUSIVE_LOCKS_REQUIRED(!cs_lockStatus), preventing misuse. No deadlock risk exists.

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/llmq/utils.cpp (1)

757-777: Critical: qwatchConnectionSeed “init once” isn’t actually init-once (can overwrite).

Two threads can both pass if (!qwatchConnectionSeedGenerated), then serialize on the mutex and both execute the initialization block, overwriting qwatchConnectionSeed and changing results.

Proposed diff (minimal change)

 std::unordered_set<size_t> CalcDeterministicWatchConnections(Consensus::LLMQType llmqType,
                                                              gsl::not_null<const CBlockIndex*> pQuorumBaseBlockIndex,
                                                              size_t memberCount, size_t connectionCount)
 {
     static uint256 qwatchConnectionSeed;
     static std::atomic<bool> qwatchConnectionSeedGenerated{false};
     static RecursiveMutex qwatchConnectionSeedCs;
-    if (!qwatchConnectionSeedGenerated) {
-        LOCK(qwatchConnectionSeedCs);
-        qwatchConnectionSeed = GetRandHash();
-        qwatchConnectionSeedGenerated = true;
-    }
+    if (!qwatchConnectionSeedGenerated.load()) {
+        LOCK(qwatchConnectionSeedCs);
+        if (!qwatchConnectionSeedGenerated.load()) {
+            qwatchConnectionSeed = GetRandHash();
+            qwatchConnectionSeedGenerated.store(true);
+        }
+    }
 
     std::unordered_set<size_t> result;
     uint256 rnd = qwatchConnectionSeed;

src/llmq/debug.cpp (1)

110-150: Potential deadlock: ToJson() and GetSessionCount() can deadlock with RPC code holding cs_main.

ToJson() locks cs_lockStatus then calls CDKGDebugSessionStatus::ToJson(), which takes cs_main when detailLevel == 2. Meanwhile, quorum_dkginfo() RPC calls GetSessionCount() (which locks cs_lockStatus) while already holding cs_main. This creates an ABBA deadlock if these code paths execute concurrently.

Mitigate by snapshotting localStatus.sessions under cs_lockStatus, unlocking, then building the JSON from the snapshot.

src/llmq/dkgsession.cpp (1)

361-381: Fix off-by-one: p.index == members.size() currently passes PreVerify and can crash later.
members[p.index] is used in ReceiveMessage(const CDKGJustification&) (Line 411), so PreVerify must reject index >= members.size().

Proposed diff

-    for (const auto& p : qj.contributions) {
-        if (p.index > members.size()) {
+    for (const auto& p : qj.contributions) {
+        if (p.index >= members.size()) {
             logger.Batch("invalid contribution index");
             retBan = true;
             return false;
         }

Also applies to: 410-418

🤖 Fix all issues with AI agents

In @src/bls/bls_worker.h:
- Around line 57-58: Add a defensive assertion in Start(int8_t worker_count)
before calling workerPool.resize() to ensure worker_count is non-negative (e.g.,
assert(worker_count >= 0)); then cast to size_t only after the check when
resizing the workerPool. Locate the Start function in bls_worker.cpp (the caller
of workerPool.resize) and add the assertion and the safe cast so negative int8_t
values cannot silently convert to a huge size_t.

In @src/init.cpp:
- Around line 2019-2037: The bls_threads lambda narrows user input into int8_t
before clamping, which can overflow; change the intermediate type to a wider
signed integer (e.g., int or int32_t) when calling args.GetIntArg and doing
arithmetic/GetNumCores, apply std::clamp with that wider type against 0 and
llmq::MAX_BLSCHECK_THREADS, then static_cast the final clamped value to int8_t
for the llmq::DashParams::bls_threads return; update references inside the
lambda (GetIntArg, GetNumCores, std::clamp, llmq::MAX_BLSCHECK_THREADS)
accordingly.

In @src/llmq/dkgsession.cpp:
- Around line 201-210: ReceiveMessage calls ReceiveMessagePreamble and then
dereferences the returned state's member unconditionally; modify ReceiveMessage
(and the other callers you noted) to guard against a nullptr member from
ReceiveMessagePreamble by checking the state's member before any member->...
access and handling the null case (e.g., return std::nullopt or the existing inv
as appropriate) to avoid null deref. Specifically, in
CDKGSession::ReceiveMessage (and the similar call sites around
ReceiveMessagePreamble for other phases), after obtaining state from
ReceiveMessagePreamble, verify state->member is non-null before using member or
member->contributions; if null, log/return early consistent with the function’s
existing error return (std::nullopt or inv) so callers remain well-behaved.

In @src/llmq/observer/context.h:
- Around line 50-51: Ensure QuorumObserver::Start(int8_t worker_count) validates
that worker_count > 0 before using it (e.g., before calling workerPool.resize);
add an assert or explicit check that either throws/returns on invalid values or
clamps to a minimum of 1, and include a brief comment referencing
DEFAULT_WORKER_COUNT behavior; target the QuorumObserver::Start implementation
and the call site to workerPool.resize() to prevent resizing with non-positive
counts.

In @src/llmq/options.h:
- Around line 8-16: DEFAULT_WORKER_COUNT currently uses std::clamp without
including <algorithm> and clamps using int8_t which can overflow when
hardware_concurrency()/2 > 127; add #include <algorithm>, change the clamp
expression to operate on a wider integer type (e.g., int) such as: compute auto
cores = static_cast<int>(std::thread::hardware_concurrency()) / 2; then use
std::clamp(cores, 1, 4) and only cast the final clamped value to int8_t if the
constant must be int8_t; update the same pattern wherever DEFAULT_WORKER_COUNT
is defined/used (e.g., around the other occurrences mentioned).

In @src/node/chainstate.h:
- Around line 30-33: The trailing comment after the namespace closing is wrong:
it closes namespace llmq but reads "// namespace util"; update the comment to
"// namespace llmq" (or remove it) to match the opened namespace llmq in this
header (locate the block starting with "namespace llmq {" and the closing brace
that currently has the incorrect comment).

🧹 Nitpick comments (9)

src/llmq/signing.cpp (1)

182-190: Consider adding error handling for stream deserialization.

The size check for writeTimeDs was removed, which means if the database contains corrupted or malformed data at k2, the deserialization on line 186 could throw an exception. This would abort the cleanup operation mid-way and leave the cache in an inconsistent state (lines 192-197 wouldn't execute).

While k2 should always contain a uint32_t based on line 137, adding defensive error handling would improve robustness against database corruption.

🛡️ Suggested error handling approach

 if (deleteTimeKey) {
     CDataStream writeTimeDs(SER_DISK, CLIENT_VERSION);
     if (db->ReadDataStream(k2, writeTimeDs)) {
-        uint32_t writeTime;
-        writeTimeDs >> writeTime;
-        auto k5 = std::make_tuple(std::string("rs_t"), (uint32_t) htobe32_internal(writeTime), recSig.getLlmqType(), recSig.getId());
-        batch.Erase(k5);
+        try {
+            uint32_t writeTime;
+            writeTimeDs >> writeTime;
+            auto k5 = std::make_tuple(std::string("rs_t"), (uint32_t) htobe32_internal(writeTime), recSig.getLlmqType(), recSig.getId());
+            batch.Erase(k5);
+        } catch (const std::exception& e) {
+            LogPrint(BCLog::LLMQ, "CRecoveredSigsDb::%s -- failed to deserialize writeTime: %s\n", __func__, e.what());
+        }
     }
 }

Alternatively, restore the size check if that was the previous defensive guard:

 if (deleteTimeKey) {
     CDataStream writeTimeDs(SER_DISK, CLIENT_VERSION);
-    if (db->ReadDataStream(k2, writeTimeDs)) {
+    if (db->ReadDataStream(k2, writeTimeDs) && writeTimeDs.size() == sizeof(uint32_t)) {
         uint32_t writeTime;
         writeTimeDs >> writeTime;
         auto k5 = std::make_tuple(std::string("rs_t"), (uint32_t) htobe32_internal(writeTime), recSig.getLlmqType(), recSig.getId());
         batch.Erase(k5);
     }
 }

src/bls/bls_worker.cpp (1)

59-63: Consider adding validation for the worker_count parameter.

The worker_count parameter is used directly to resize the workerPool without validation. Since worker_count is int8_t, it could theoretically be negative or zero, which might lead to unexpected behavior.

♻️ Suggested validation

 void CBLSWorker::Start(int8_t worker_count)
 {
+    assert(worker_count > 0);
     workerPool.resize(worker_count);
     RenameThreadPool(workerPool, "bls-work");
 }

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

47-51: Consider adding validation for the worker_count parameter.

Similar to other components, the worker_count parameter is used directly to resize the workerPool without validation.

♻️ Suggested validation

 void QuorumObserver::Start(int8_t worker_count)
 {
+    assert(worker_count > 0);
     workerPool.resize(worker_count);
     RenameThreadPool(workerPool, "q-mngr");
 }

src/llmq/signing_shares.cpp (1)

199-217: Consider adding validation for the worker_count parameter.

While the code correctly checks for already-running threads, the worker_count parameter is used without validation.

♻️ Suggested validation

 void CSigSharesManager::Start(int8_t worker_count)
 {
     // can't start if threads are already running
     if (housekeepingThread.joinable() || dispatcherThread.joinable()) {
         assert(false);
     }
 
+    assert(worker_count > 0);
     // Initialize worker pool
     workerPool.resize(worker_count);
     RenameThreadPool(workerPool, "sigsh-work");
 
     // Start housekeeping thread
     housekeepingThread = std::thread(&util::TraceThread, "sigsh-maint",
         [this] { HousekeepingThreadMain(); });
 
     // Start dispatcher thread
     dispatcherThread = std::thread(&util::TraceThread, "sigsh-dispat",
         [this] { WorkDispatcherThreadMain(); });
 }

src/active/dkgsession.cpp (1)

340-371: Signature change OK; consider using .contains() for consistency/readability (optional).

SendJustification(..., const Uint256HashSet& forMembers) matches the new container choice. Minor optional cleanup: prefer forMembers.contains(...) (if Uint256HashSet supports it) to match other code in this PR (proRegTxHashes.contains(h) usage elsewhere).

src/llmq/context.h (1)

8-9: Potential compile-time bloat: consider forward-decl-only in header (optional).

Since this header only needs llmq::DashParams by reference, you could avoid including <llmq/options.h> here and include it in src/llmq/context.cpp instead, relying on the forward declaration. This may reduce rebuild impact.

Also applies to: 29-30

src/node/chainstate.cpp (2)

37-63: Ensure llmq::DashParams is declared via a direct include (avoid transitive include fragility).
LoadChainstate(...) now exposes llmq::DashParams in the signature, so this TU should include the header that defines it (not rely on llmq/context.h transitively).

Proposed diff

 #include <llmq/context.h>
+// Prefer direct include for llmq::DashParams (avoid relying on transitive includes)
+#include <llmq/options.h>

---

71-73: Redundant evodb.reset() before immediate reassignment.
Not a problem, but the explicit reset is unnecessary if you immediately overwrite the unique_ptr.

Proposed diff

-    evodb.reset();
-    evodb = std::make_unique<CEvoDB>(dash_params.db_params);
+    evodb = std::make_unique<CEvoDB>(dash_params.db_params);

src/net_processing.cpp (1)

967-969: Assert/align the “active vs observer” routing invariant (and keep ordering consistent).

DKGSessionAlreadyHave() prefers observer first, while DKGSessionProcessMessage() prefers active first. If both contexts can never be set simultaneously, consider asserting that (in both helpers, or once centrally) and picking a single consistent preference/order to avoid latent surprises.

Also applies to: 2382-2390, 5501-5519

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2262e22 and 176f929.

📒 Files selected for processing (42)

• src/active/context.cpp
• src/active/context.h
• src/active/dkgsession.cpp
• src/active/dkgsession.h
• src/active/dkgsessionhandler.cpp
• src/bench/bls.cpp
• src/bench/bls_dkg.cpp
• src/bls/bls_worker.cpp
• src/bls/bls_worker.h
• src/init.cpp
• src/llmq/context.cpp
• src/llmq/context.h
• src/llmq/core_write.cpp
• src/llmq/debug.cpp
• src/llmq/debug.h
• src/llmq/dkgsession.cpp
• src/llmq/dkgsession.h
• src/llmq/dkgsessionhandler.h
• src/llmq/ehf_signals.h
• 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/llmq/signing_shares.h
• src/llmq/utils.cpp
• src/llmq/utils.h
• src/net.cpp
• src/net.h
• src/net_processing.cpp
• src/node/chainstate.cpp
• src/node/chainstate.h
• src/rpc/quorums.cpp
• src/test/util/setup_common.cpp
• src/test/util/setup_common.h
• src/test/validation_chainstatemanager_tests.cpp

💤 Files with no reviewable changes (1)

• src/test/util/setup_common.h

🚧 Files skipped from review as they are similar to previous changes (4)

• src/llmq/quorums.cpp
• src/llmq/quorums.h
• src/llmq/quorumsman.cpp
• src/llmq/dkgsession.h

🧰 Additional context used

📓 Path-based instructions (9)

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/active/context.h
• src/llmq/observer/context.h
• src/bench/bls.cpp
• src/llmq/signing.cpp
• src/llmq/signing_shares.cpp
• src/bls/bls_worker.h
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/active/context.cpp
• src/llmq/observer/quorums.cpp
• src/net.h
• src/net.cpp
• src/llmq/context.cpp
• src/bench/bls_dkg.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/test/validation_chainstatemanager_tests.cpp
• src/node/chainstate.cpp
• src/active/dkgsession.cpp
• src/active/dkgsession.h
• src/bls/bls_worker.cpp
• src/llmq/debug.cpp
• src/net_processing.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/active/dkgsessionhandler.cpp
• src/rpc/quorums.cpp
• src/test/util/setup_common.cpp
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/init.cpp
• src/node/chainstate.h
• src/llmq/core_write.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/observer/context.h
• src/llmq/signing.cpp
• src/llmq/signing_shares.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/context.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/llmq/debug.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/llmq/core_write.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/signing.cpp
• src/llmq/signing_shares.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/context.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/llmq/debug.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/llmq/core_write.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/observer/context.h
• src/llmq/signing.cpp
• src/llmq/signing_shares.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/context.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/llmq/debug.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/llmq/core_write.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/observer/context.h
• src/llmq/signing.cpp
• src/llmq/signing_shares.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/observer/quorums.cpp
• src/llmq/context.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/llmq/debug.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/llmq/core_write.cpp

src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Performance benchmarks in src/bench/ must use the nanobench library

Files:

• src/bench/bls.cpp
• src/bench/bls_dkg.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/validation_chainstatemanager_tests.cpp
• 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.h

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/node/chainstate.h

🧠 Learnings (45)

📓 Common learnings

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

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

Learnt from: kwvg
Repo: dashpay/dash PR: 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: 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: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.

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

📚 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/active/context.h
• src/active/context.cpp

📚 Learning: 2025-12-22T15:42:48.595Z

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

Applied to files:

• src/active/context.h
• src/llmq/observer/context.h
• src/bls/bls_worker.h
• src/llmq/utils.h
• src/llmq/dkgsessionhandler.h
• src/llmq/signing_shares.h
• src/llmq/context.h
• src/net.h
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/active/dkgsession.h
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/debug.h
• src/node/chainstate.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/bench/bls.cpp
• src/llmq/context.h
• src/net.h
• src/bench/bls_dkg.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/node/chainstate.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/node/chainstate.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/bench/bls.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/observer/quorums.cpp
• src/net.h
• src/llmq/context.cpp
• src/bench/bls_dkg.cpp
• src/llmq/observer/quorums.h
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/rpc/quorums.cpp
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/init.cpp
• src/node/chainstate.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/bench/bls.cpp
• src/llmq/signing.cpp
• src/llmq/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/net.h
• src/llmq/context.cpp
• src/bench/bls_dkg.cpp
• src/llmq/ehf_signals.h
• src/llmq/observer/quorums.h
• src/net_processing.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/debug.h
• src/node/chainstate.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/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

• src/bench/bls.cpp

📚 Learning: 2025-10-02T18:29:54.756Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.

Applied to files:

• src/llmq/dkgsession.cpp
• src/llmq/context.h
• src/llmq/utils.cpp
• src/active/context.cpp
• src/llmq/context.cpp
• src/node/chainstate.cpp
• src/net_processing.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/observer/context.cpp
• src/llmq/debug.h
• src/init.cpp
• src/node/chainstate.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/dkgsession.cpp
• src/llmq/utils.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/net.h
• src/llmq/context.cpp
• src/llmq/observer/quorums.h
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/llmq/debug.h
• src/node/chainstate.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/llmq/utils.h
• src/llmq/context.h
• src/node/chainstate.cpp
• src/llmq/options.h
• src/llmq/quorumsman.h
• src/test/util/setup_common.cpp
• src/init.cpp
• src/node/chainstate.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/utils.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/utils.h
• src/test/util/setup_common.cpp
• src/node/chainstate.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/utils.h
• src/test/util/setup_common.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/utils.h
• src/test/validation_chainstatemanager_tests.cpp
• src/test/util/setup_common.cpp
• src/llmq/debug.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/utils.h
• src/node/chainstate.cpp
• src/node/chainstate.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/llmq/utils.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/utils.h

📚 Learning: 2025-11-13T20:02:55.480Z

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

Applied to files:

• src/llmq/utils.h
• src/llmq/utils.cpp
• src/llmq/debug.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/llmq/utils.h
• src/llmq/context.h
• src/llmq/utils.cpp
• src/llmq/context.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/llmq/options.h
• 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/{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/context.h
• src/llmq/context.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/node/chainstate.cpp
• src/llmq/options.h
• src/test/util/setup_common.cpp
• src/init.cpp
• src/node/chainstate.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/llmq/context.h
• src/llmq/context.cpp
• src/test/util/setup_common.cpp
• src/node/chainstate.h

📚 Learning: 2025-11-24T16:41:22.457Z

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

Applied to files:

• src/llmq/context.h
• src/llmq/context.cpp
• src/test/util/setup_common.cpp
• src/node/chainstate.h

📚 Learning: 2025-07-23T09:30:34.631Z

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

Applied to files:

• src/llmq/context.h
• src/llmq/context.cpp
• src/node/chainstate.cpp
• src/llmq/options.h
• src/test/util/setup_common.cpp
• src/node/chainstate.h

📚 Learning: 2025-01-02T21:50:00.967Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.

Applied to files:

• src/llmq/context.h
• src/llmq/context.cpp
• src/node/chainstate.cpp

📚 Learning: 2024-12-29T17:43:41.755Z

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

Applied to files:

• src/llmq/context.h
• src/llmq/context.cpp
• src/node/chainstate.cpp
• src/llmq/observer/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/llmq/context.h
• src/llmq/context.cpp
• src/llmq/options.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/net.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,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/net.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/net.h
• src/active/dkgsession.cpp

📚 Learning: 2025-10-21T11:09:34.688Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.

Applied to files:

• src/net.h
• src/llmq/quorumsman.h

📚 Learning: 2025-09-09T21:36:11.833Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:11.833Z
Learning: In RawSender class (src/stats/rawsender.cpp), cs_net is the appropriate mutex for protecting socket access (m_sock) and network operations, not additional custom locks. The implementation correctly uses cs_net with GUARDED_BY annotations and EXCLUSIVE_LOCKS_REQUIRED to synchronize socket access between SendDirectly() and ReconnectThread().

Applied to files:

• src/net.h
• src/llmq/quorumsman.h

📚 Learning: 2025-09-09T21:36:11.833Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:11.833Z
Learning: In RawSender class (src/stats/rawsender.cpp), cs_net is the appropriate mutex for protecting socket access (m_sock) and network operations, not additional custom locks.

Applied to files:

• src/net.h

📚 Learning: 2025-09-09T21:36:58.969Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:223-232
Timestamp: 2025-09-09T21:36:58.969Z
Learning: In RawSender class (src/stats/rawsender.cpp), socket operations are properly synchronized using the cs_net mutex. The m_sock and m_server members are GUARDED_BY(cs_net), and methods like Connect(), SendDirectly(), and ReconnectThread() use appropriate locking with cs_net to prevent race conditions on socket access.

Applied to files:

• src/net.h

📚 Learning: 2025-09-09T21:36:31.504Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:31.504Z
Learning: In RawSender class (src/stats/rawsender.cpp), m_sock access is protected by the cs_net mutex. This mutex coordinates between the TCP send path in SendDirectly() and the reconnection operations in Connect()/Reconnect() methods, ensuring proper synchronization of socket state.

Applied to files:

• src/net.h

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety

Applied to files:

• src/net.h

📚 Learning: 2025-09-09T21:36:31.504Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:31.504Z
Learning: In RawSender class (src/stats/rawsender.cpp), m_sock access requires synchronization using the cs_net mutex. The race condition exists between SendDirectly() (callable from any thread) and Connect() (called by ReconnectThread), where both read and write m_sock concurrently. The cs_net mutex properly coordinates these network operations.

Applied to files:

• src/net.h

📚 Learning: 2025-01-14T09:06:19.717Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/test/fuzz/netaddress.cpp:83-84
Timestamp: 2025-01-14T09:06:19.717Z
Learning: In fuzzer harness tests, CServiceHash can be used with both default constructor (CServiceHash()) and parameterized constructor (CServiceHash(salt_k0, salt_k1)) to test different variants. The usage pattern CServiceHash()(service) and CServiceHash(0, 0)(service) is valid and intentional in such tests.

Applied to files:

• src/llmq/ehf_signals.h

📚 Learning: 2025-11-24T16:41:22.457Z

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

Applied to files:

• src/test/validation_chainstatemanager_tests.cpp
• src/node/chainstate.cpp
• src/test/util/setup_common.cpp
• src/init.cpp
• src/node/chainstate.h

📚 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/validation_chainstatemanager_tests.cpp
• src/node/chainstate.cpp
• src/test/util/setup_common.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/node/chainstate.cpp
• src/llmq/quorumsman.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/node/chainstate.cpp
• src/init.cpp

📚 Learning: 2025-08-13T16:10:32.972Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6815
File: src/chainlock/chainlock.h:68-69
Timestamp: 2025-08-13T16:10:32.972Z
Learning: The Dash codebase extensively uses std::chrono_literals throughout, including in header files. The pattern using namespace std::chrono_literals; is used globally in src/util/time.h, and chrono literals like 0s are commonly used for atomic member initialization in headers (e.g., src/net.h). Chrono literals like 0s, 1min, etc. are preferred over explicit constructors.

Applied to files:

• src/llmq/options.h

📚 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/llmq/options.h

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

Applied to files:

• src/node/chainstate.h

🧬 Code graph analysis (21)

src/active/context.h (6)

src/llmq/context.cpp (2)

• Start (39-42)
• Start (39-39)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

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

src/llmq/context.cpp (2)

• Start (39-42)
• Start (39-39)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

src/llmq/signing_shares.cpp (2)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/bls/bls_worker.h (5)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

src/llmq/dkgsession.cpp (2)

src/llmq/debug.h (1)

• phase (60-60)

src/llmq/commitment.h (1)

• quorumIndex (56-56)

src/llmq/utils.h (1)

src/llmq/utils.cpp (2)

• CalcDeterministicWatchConnections (757-777)
• CalcDeterministicWatchConnections (757-759)

src/llmq/signing_shares.h (2)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

src/llmq/context.h (1)

src/llmq/context.cpp (2)

• LLMQContext (17-32)
• LLMQContext (34-37)

src/active/context.cpp (5)

src/llmq/context.cpp (2)

• Start (39-42)
• Start (39-39)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

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

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (40-43)
• Start (40-40)

src/net.h (1)

src/net.cpp (2)

• AddPendingProbeConnections (4458-4462)
• AddPendingProbeConnections (4458-4458)

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

src/llmq/quorumsman.cpp (2)

• CleanupOldQuorumData (362-366)
• CleanupOldQuorumData (362-362)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/test/validation_chainstatemanager_tests.cpp (2)

src/node/chainstate.cpp (2)

• DashChainstateSetup (208-243)
• DashChainstateSetup (208-221)

src/test/util/setup_common.cpp (2)

• DashChainstateSetup (140-157)
• DashChainstateSetup (140-142)

src/active/dkgsession.cpp (1)

src/active/dkgsessionhandler.cpp (1)

• logger (336-336)

src/bls/bls_worker.cpp (4)

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

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

• Start (40-43)
• Start (40-40)

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

• Start (47-51)
• Start (47-47)

src/llmq/signing_shares.cpp (2)

• Start (199-217)
• Start (199-199)

src/llmq/debug.cpp (1)

src/llmq/debug.h (1)

• CDKGDebugManager (91-124)

src/test/util/setup_common.cpp (1)

src/llmq/options.h (1)

• DEFAULT_BLSCHECK_THREADS (45-45)

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

src/active/context.cpp (2)

• Start (75-83)
• Start (75-75)

src/bls/bls_worker.cpp (2)

• Start (59-63)
• Start (59-59)

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

• Start (47-51)
• Start (47-47)

src/llmq/debug.h (2)

src/llmq/debug.cpp (4)

• CDKGDebugManager (110-116)
• CDKGDebugManager (118-118)
• GetSessionCount (120-123)
• GetSessionCount (120-120)

src/llmq/core_write.cpp (24)

• GetJsonHelp (20-78)
• GetJsonHelp (20-20)
• GetJsonHelp (81-95)
• GetJsonHelp (81-81)
• GetJsonHelp (97-114)
• GetJsonHelp (97-97)
• GetJsonHelp (134-142)
• GetJsonHelp (134-134)
• GetJsonHelp (153-178)
• GetJsonHelp (153-153)
• GetJsonHelp (222-234)
• GetJsonHelp (222-222)
• GetJsonHelp (255-266)
• GetJsonHelp (255-255)
• ToJson (116-132)
• ToJson (116-116)
• ToJson (144-151)
• ToJson (144-144)
• ToJson (180-220)
• ToJson (180-180)
• ToJson (236-253)
• ToJson (236-236)
• ToJson (268-278)
• ToJson (268-268)

src/init.cpp (2)

src/llmq/options.h (2)

• DEFAULT_BLSCHECK_THREADS (45-45)
• MAX_BLSCHECK_THREADS (49-63)

src/util/system.cpp (2)

• GetNumCores (1455-1458)
• GetNumCores (1455-1455)

src/node/chainstate.h (1)

src/llmq/context.h (1)

• llmq (22-30)

🪛 Cppcheck (2.19.0)

src/llmq/context.cpp

[error] 16-16: #error Client version information missing

(preprocessorErrorDirective)

src/bench/bls_dkg.cpp

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

(syntaxError)

src/llmq/core_write.cpp

[error] 80-80: Uninitialized variable

(legacyUninitvar)

⏰ 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_multiprocess-test / Test source

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) Find all CBLSWorker::Start call sites
echo "=== Searching for CBLSWorker::Start() call sites ==="
rg -n --type=cpp --type=cc --type=h --type=hpp 'Start\s*\(' src/bls src/llmq 2>/dev/null | grep -i worker | head -30

echo -e "\n=== Searching for CBLSWorker in header files ==="
rg -n --type=h --type=hpp 'class CBLSWorker' src

echo -e "\n=== Searching for DEFAULT_WORKER_COUNT ==="
rg -n --type=cpp --type=cc --type=h --type=hpp 'DEFAULT_WORKER_COUNT' src

Repository: dashpay/dash

Length of output: 274

---

🏁 Script executed:

#!/bin/bash
# 2) Look at the actual implementation of Start() in bls_worker.cpp
echo "=== Finding bls_worker.cpp ==="
find src -name "bls_worker.cpp" -type f

echo -e "\n=== Checking if file exists and reading it ==="
if [ -f "src/bls/bls_worker.cpp" ]; then
  echo "File found. Reading Start() implementation:"
  rg -n -A 10 'void CBLSWorker::Start' src/bls/bls_worker.cpp
fi

Repository: dashpay/dash

Length of output: 451

---

🏁 Script executed:

#!/bin/bash
# 3) Check for any validation or constraints on worker_count parameter
echo "=== Searching for worker_count usage and constraints ==="
rg -n --type=cpp --type=h 'worker_count' src/bls src/llmq | head -40

Repository: dashpay/dash

Length of output: 1111

---

🏁 Script executed:

#!/bin/bash
# Fix the file type issue and search more broadly
echo "=== Searching for worker_count initialization ==="
rg -n 'worker_count\s*=' src --type c | head -30

echo -e "\n=== Searching for params initialization in context.cpp ==="
rg -n -B 5 'bls_worker->Start' src/bls/context.cpp

echo -e "\n=== Looking for LLMQParams or similar definition ==="
rg -n 'struct.*Params.*{' src/llmq | head -10

Repository: dashpay/dash

Length of output: 394

---

🏁 Script executed:

#!/bin/bash
# Check what params is and where worker_count comes from
echo "=== Examining src/bls/context.cpp around Start() call ==="
cat -n src/bls/context.cpp | head -50

echo -e "\n=== Checking llmq/options.h for LLMQParams ==="
cat -n src/llmq/options.h | grep -A 10 -B 10 'worker_count'

Repository: dashpay/dash

Length of output: 1009

---

🏁 Script executed:

#!/bin/bash
# Search for validation of worker_count anywhere
echo "=== Searching for any validation of worker_count ==="
rg -n 'worker_count.*>' src --type c
rg -n 'assert.*worker_count' src --type c
rg -n 'if.*worker_count' src --type c | head -20

Repository: dashpay/dash

Length of output: 108

---

🏁 Script executed:

#!/bin/bash
echo "=== Searching for DashParams initialization ==="
rg -n 'DashParams' src --type c | head -30

echo -e "\n=== Examining src/llmq/context.cpp ==="
cat -n src/llmq/context.cpp | head -60

Repository: dashpay/dash

Length of output: 2950

---

🏁 Script executed:

#!/bin/bash
echo "=== Searching for where worker_count/bls_threads are set ==="
rg -n 'bls_threads|worker_count' src --type c | grep -E '(=|DEFAULT|gArgs)' | head -30

echo -e "\n=== Searching for gArgs or parameter parsing related to bls/worker ==="
rg -n -i 'bls.thread|worker.count|llmq.*thread' src --type c | head -30

Repository: dashpay/dash

Length of output: 921

---

🏁 Script executed:

#!/bin/bash
echo "=== Looking for where DashParams gets populated ==="
rg -n 'bls_threads\s*=' src --type c -A 2 -B 2
rg -n 'worker_count\s*=' src --type c -A 2 -B 2

Repository: dashpay/dash

Length of output: 107

---

🏁 Script executed:

#!/bin/bash
echo "=== Searching for where DashParams is constructed ==="
rg -n 'DashParams\s*{' src --type c -A 3 | head -50

echo -e "\n=== Searching for any place where DashParams gets created ==="
rg -n 'llmq::DashParams' src --type c | head -30

Repository: dashpay/dash

Length of output: 632

---

🏁 Script executed:

#!/bin/bash
echo "=== Looking for parameter parsing or initialization related to worker_count ==="
rg -n -i 'worker|llmq' src --type c | grep -i -E 'parse|arg|init|create|param' | head -30

echo -e "\n=== Checking if there are command-line argument parsers ==="
find src -name "*args*" -o -name "*params*" | head -10

Repository: dashpay/dash

Length of output: 3380

---

🏁 Script executed:

#!/bin/bash
echo "=== Searching in chainstate or main initialization files ==="
find src -name "*.h" -o -name "*.cpp" | xargs grep -l 'DashParams' | head -10

Repository: dashpay/dash

Length of output: 265

---

🏁 Script executed:

#!/bin/bash
echo "=== Checking src/init.cpp for DashParams construction ==="
rg -n 'DashParams' src/init.cpp -A 5 -B 5

echo -e "\n=== Checking src/node/chainstate.cpp for DashParams ==="
rg -n 'DashParams' src/node/chainstate.cpp -A 5 -B 5

echo -e "\n=== Searching for llmq:: DashParams initialization ==="
rg -n -A 10 'llmq::DashParams' src --type c | head -60

Repository: dashpay/dash

Length of output: 5393

---

🏁 Script executed:

#!/bin/bash
echo "=== Looking for worker_count assignments or initialization ==="
rg -n 'worker_count' src/init.cpp src/node/chainstate.cpp -B 3 -A 3

echo -e "\n=== Checking test setup files ==="
cat src/test/util/setup_common.cpp | grep -n -A 10 -B 5 'DashParams\|worker_count' | head -50

Repository: dashpay/dash

Length of output: 3392

---

Add assertion to guard the int8_t to size_t conversion in workerPool.resize().

While worker_count is currently always positive (clamped to [1, 4]), the implicit int8_t to size_t conversion in resize() is a hazard if this parameter ever becomes user-configurable or dynamically set. Add an assertion to prevent negative values:

Proposed fix (in src/bls/bls_worker.cpp)

 void CBLSWorker::Start(int8_t worker_count)
 {
+    assert(worker_count > 0);
     workerPool.resize(worker_count);
     RenameThreadPool(workerPool, "bls-work");
 }

🤖 Prompt for AI Agents

In @src/bls/bls_worker.h around lines 57 - 58, Add a defensive assertion in
Start(int8_t worker_count) before calling workerPool.resize() to ensure
worker_count is non-negative (e.g., assert(worker_count >= 0)); then cast to
size_t only after the check when resizing the workerPool. Locate the Start
function in bls_worker.cpp (the caller of workerPool.resize) and add the
assertion and the safe cast so negative int8_t values cannot silently convert to
a huge size_t.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid int8_t narrowing before clamping when computing DashParams.bls_threads.

args.GetIntArg("-parbls", ...) is widened, but you immediately store into int8_t, so out-of-range user input can overflow before std::clamp<int8_t>(...) runs.

Proposed diff

                                               llmq::DashParams{
-                                                .bls_threads = [&args]() -> int8_t {
-                                                    int8_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS);
+                                                .bls_threads = [&args]() -> int8_t {
+                                                    int threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS);
                                                     if (threads <= 0) {
                                                         // -parbls=0 means autodetect (number of cores - 1 validator threads)
                                                         // -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads)
                                                         threads += GetNumCores();
                                                     }
                                                     // Subtract 1 because the main thread counts towards the par threads
-                                                    return std::clamp<int8_t>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS);
+                                                    threads = std::clamp<int>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS);
+                                                    return static_cast<int8_t>(threads);
                                                 }(),
                                                 .worker_count = llmq::DEFAULT_WORKER_COUNT,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	llmq::DashParams{
	.bls_threads = [&args]() -> int8_t {
	int8_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS);
	if (threads <= 0) {
	// -parbls=0 means autodetect (number of cores - 1 validator threads)
	// -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads)
	threads += GetNumCores();
	}
	// Subtract 1 because the main thread counts towards the par threads
	return std::clamp<int8_t>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS);
	}(),
	.worker_count = llmq::DEFAULT_WORKER_COUNT,
	.max_recsigs_age = args.GetIntArg("-maxrecsigsage", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE),
	.db_params = util::DbWrapperParams{
	.path = args.GetDataDirNet(),
	.memory = false,
	.wipe = fReset || fReindexChainState,
	},
	},
	llmq::DashParams{
	.bls_threads = [&args]() -> int8_t {
	int threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS);
	if (threads <= 0) {
	// -parbls=0 means autodetect (number of cores - 1 validator threads)
	// -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads)
	threads += GetNumCores();
	}
	// Subtract 1 because the main thread counts towards the par threads
	threads = std::clamp<int>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS);
	return static_cast<int8_t>(threads);
	}(),
	.worker_count = llmq::DEFAULT_WORKER_COUNT,
	.max_recsigs_age = args.GetIntArg("-maxrecsigsage", llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE),
	.db_params = util::DbWrapperParams{
	.path = args.GetDataDirNet(),
	.memory = false,
	.wipe = fReset || fReindexChainState,
	},
	},

🤖 Prompt for AI Agents

In @src/init.cpp around lines 2019 - 2037, The bls_threads lambda narrows user
input into int8_t before clamping, which can overflow; change the intermediate
type to a wider signed integer (e.g., int or int32_t) when calling
args.GetIntArg and doing arithmetic/GetNumCores, apply std::clamp with that
wider type against 0 and llmq::MAX_BLSCHECK_THREADS, then static_cast the final
clamped value to int8_t for the llmq::DashParams::bls_threads return; update
references inside the lambda (GetIntArg, GetNumCores, std::clamp,
llmq::MAX_BLSCHECK_THREADS) accordingly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Guard member against nullptr inside ReceiveMessagePreamble (avoid potential null deref).
Even if current call paths always pre-verify membership, ReceiveMessagePreamble dereferences member unconditionally (e.g., returning member->contributions), which is a brittle invariant for network-fed code.

Proposed diff

 template <typename MsgType>
 std::optional<CDKGSession::ReceiveMessageState> CDKGSession::ReceiveMessagePreamble(const MsgType& msg, MsgPhase phase, CDKGLogger& logger)
 {
     auto* member = GetMember(msg.proTxHash);
+    if (member == nullptr) {
+        logger.Batch("message from non-member %s", msg.proTxHash.ToString());
+        return std::nullopt;
+    }

     GetDataMsg inv_type{0};
     std::string msg_name;
@@
     const uint256 hash = ::SerializeHash(msg);
-    member_set.emplace(hash);
+    const bool inserted = member_set.emplace(hash).second;
@@
-    bool should_process{true};
+    bool should_process{true};
+    if (!inserted) {
+        // Already seen; avoid re-processing while keeping relay behavior consistent.
+        should_process = false;
+    }
     if (member_set.size() > 1) {
@@
     return ReceiveMessageState{member, hash, CInv{inv_type, hash}, should_process};
 }

Also applies to: 298-306, 395-403, 611-675

🤖 Prompt for AI Agents

In @src/llmq/dkgsession.cpp around lines 201 - 210, ReceiveMessage calls
ReceiveMessagePreamble and then dereferences the returned state's member
unconditionally; modify ReceiveMessage (and the other callers you noted) to
guard against a nullptr member from ReceiveMessagePreamble by checking the
state's member before any member->... access and handling the null case (e.g.,
return std::nullopt or the existing inv as appropriate) to avoid null deref.
Specifically, in CDKGSession::ReceiveMessage (and the similar call sites around
ReceiveMessagePreamble for other phases), after obtaining state from
ReceiveMessagePreamble, verify state->member is non-null before using member or
member->contributions; if null, log/return early consistent with the function’s
existing error return (std::nullopt or inv) so callers remain well-behaved.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find the actual context.h file and examine it
find src/llmq -name "context.h" -type f

Repository: dashpay/dash

Length of output: 102

---

🏁 Script executed:

# Search for ObserverContext and its Start() method calls
rg -n --type=cpp --type=cc --type=h --type=hpp '\bObserverContext::Start\b|ObserverContext\b.*\.Start\s*\(' src

Repository: dashpay/dash

Length of output: 82

---

🏁 Script executed:

# Also search for the Start method implementation in context.cpp or similar files
find src/llmq/observer -name "*.cpp" -o -name "*.cc" | head -20

Repository: dashpay/dash

Length of output: 115

---

🏁 Script executed:

# Let me read the context.h file to see the Start() signature
cat -n src/llmq/observer/context.h

Repository: dashpay/dash

Length of output: 2597

---

🏁 Script executed:

# Now read the context.cpp implementation
cat -n src/llmq/observer/context.cpp

Repository: dashpay/dash

Length of output: 2922

---

🏁 Script executed:

# Search for ObserverContext::Start callers with correct file types
rg -n --type=cpp --type=h -C2 'ObserverContext.*Start|\.Start\s*\(' src

Repository: dashpay/dash

Length of output: 3570

---

🏁 Script executed:

# Find QuorumObserver definition
cat -n src/llmq/observer/quorums.h | head -100

Repository: dashpay/dash

Length of output: 4466

---

🏁 Script executed:

# Search for QuorumObserver::Start implementation
rg -n --type=h 'class QuorumObserver' src/llmq/observer

Repository: dashpay/dash

Length of output: 218

---

🏁 Script executed:

# Find all actual calls to ObserverContext::Start
rg -n 'observer_context.*Start|ObserverContext.*Start\(' src

Repository: dashpay/dash

Length of output: 137

---

🏁 Script executed:

# Get QuorumObserver::Start implementation
rg -n 'void QuorumObserver::Start' src/llmq/observer -A20

Repository: dashpay/dash

Length of output: 4677

---

🏁 Script executed:

# Search for observer_context in all files
rg -n 'observer_context\|ObserverContext' src --type=h --type=cpp | grep -v "^Binary"

Repository: dashpay/dash

Length of output: 38

---

🏁 Script executed:

# Look for DEFAULT_WORKER_COUNT
rg -n 'DEFAULT_WORKER_COUNT' src

Repository: dashpay/dash

Length of output: 955

---

Add validation or assertion for worker_count > 0 in the Start() implementation.

The current code relies on DEFAULT_WORKER_COUNT (which clamps values to [1, 4]) at call sites in src/init.cpp, but the Start() method itself accepts any int8_t without validation. To prevent misuse if this method is called with invalid values in the future, either assert or validate worker_count > 0 in QuorumObserver::Start() before passing it to workerPool.resize().

🤖 Prompt for AI Agents

In @src/llmq/observer/context.h around lines 50 - 51, Ensure
QuorumObserver::Start(int8_t worker_count) validates that worker_count > 0
before using it (e.g., before calling workerPool.resize); add an assert or
explicit check that either throws/returns on invalid values or clamps to a
minimum of 1, and include a brief comment referencing DEFAULT_WORKER_COUNT
behavior; target the QuorumObserver::Start implementation and the call site to
workerPool.resize() to prevent resizing with non-positive counts.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix potential compile break + narrowing/overflow in DEFAULT_WORKER_COUNT.

std::clamp requires <algorithm> (not included here), and clamping as int8_t can overflow before clamping on high-core machines (e.g., hardware_concurrency()/2 > 127), potentially yielding 1 worker instead of 4.

Proposed diff

 #ifndef BITCOIN_LLMQ_OPTIONS_H
 #define BITCOIN_LLMQ_OPTIONS_H
 
 #include <dbwrapper.h>
 
 #include <gsl/pointers.h>
 
+#include <algorithm>
 #include <map>
 #include <optional>
 #include <thread>
 #include <vector>
@@
 /** Number of workers allocated per worker pool */
-static int8_t DEFAULT_WORKER_COUNT{std::clamp<int8_t>(std::thread::hardware_concurrency() / 2, 1, 4)};
+inline int8_t DEFAULT_WORKER_COUNT{
+    static_cast<int8_t>(std::clamp<int>(static_cast<int>(std::thread::hardware_concurrency()) / 2, 1, 4))
+};
 /** Maximum number of dedicated script-checking threads allowed */
 static constexpr int8_t MAX_BLSCHECK_THREADS{33};

Also applies to: 44-50

🤖 Prompt for AI Agents

In @src/llmq/options.h around lines 8 - 16, DEFAULT_WORKER_COUNT currently uses
std::clamp without including <algorithm> and clamps using int8_t which can
overflow when hardware_concurrency()/2 > 127; add #include <algorithm>, change
the clamp expression to operate on a wider integer type (e.g., int) such as:
compute auto cores = static_cast<int>(std::thread::hardware_concurrency()) / 2;
then use std::clamp(cores, 1, 4) and only cast the final clamped value to int8_t
if the constant must be int8_t; update the same pattern wherever
DEFAULT_WORKER_COUNT is defined/used (e.g., around the other occurrences
mentioned).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the incorrect namespace closing comment.

This block closes namespace llmq, but the comment says // namespace util. That’s misleading in a public header and easy to fix.

Proposed fix

 namespace llmq {
 struct DashParams;
-} // namespace util
+} // namespace llmq

🤖 Prompt for AI Agents

In @src/node/chainstate.h around lines 30 - 33, The trailing comment after the
namespace closing is wrong: it closes namespace llmq but reads "// namespace
util"; update the comment to "// namespace llmq" (or remove it) to match the
opened namespace llmq in this header (locate the block starting with "namespace
llmq {" and the closing brace that currently has the incorrect comment).

[knst]

All changes except refactoring with DashParams makes sense for me, overall looks fine.

Consider nits and dropping DashParams or separating it to new PR, DashParams is not ready atm

[knst]

nit: maps is a bit too generic name. m_cs_quorums fit for both cs_map_quorums and cs_scan_quorums IMO

[knst]

nit: update include

-#include <set>
+#include <unordered_set>

[knst]

nit: saltedhasher.h is on the place; consensus/params.h should be on top between bls/ and ctpl_stl

[knst]

nit: saltedhasher is supposed to be between random.h and span.h

Details

[knst]

nit: can you make this one also constexpr without lambda function?

[knst]

nit: remove this one; move call of std::thread::hardware_concurrency to the cpp source file

[knst]

nit: it has nothing to do with llmq/options.h;

[knst]

nit: it has nothing to do with DashParams remove db_params from DashParams

[knst]

the name of DashParams is not matched with llmq/options.h and with its purpose: amount of bls_threads, amount of worker_count; amount of mac_recsigs_age`; can you choose other name?

I think you should drop c76d3f2 (refactor: consolidate Dash-specific chainstate params with DashParams) completely rather than keep it in any form; it improves nothing but introduce a new DashParams struct which you still need to initialize every time manually:

                        llmq::DashParams{
                            .bls_threads = llmq::DEFAULT_BLSCHECK_THREADS,
                            .worker_count = llmq::DEFAULT_WORKER_COUNT,
                            .max_recsigs_age = llmq::DEFAULT_MAX_RECOVERED_SIGS_AGE,
                            .db_params = util::DbWrapperParams{
                                .path = node.args->GetDataDirNet(),
                                .memory = true,
                                .wipe = false,
                            },
                        },

[knst]

refactor: isolate DKG session networking code in multiple entities

nit: it doesn't isolate anything; there's nothing for "networking code" isolated if it is inside PeerManagerImpl before and after. It's already inside PeerManager :D maybe I read commit message wrongly, what do you mean here?