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

[kwvg]

Additional Information

• Depends on refactor: extract CActiveMasternodeManager from LLMQContext (1/n, preliminary refactoring) #7056

• Depends on docs(rpc): introduce more RPC help text and deduplicate governance RPC logic to resolve pending definitions #7061

• Dependency for refactor: extract CActiveMasternodeManager from LLMQContext (2/n, CQuorumManager handler separation) #7063

Breaking Changes

See release notes.

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
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[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

DKG debug manager is moved out of the central LLMQContext into context-specific owners: ObserverContext (watch-only) and ActiveContext (masternode). ObserverContext and ActiveContext now construct and expose their own CDKGDebugManager instances. RPC handlers (quorum_dkgstatus, quorum_dkginfo) select the appropriate context at runtime, retrieve the debug manager if present, and produce JSON using the updated CDKGDebugStatus::GetJsonHelp signature (adds inner_optional). Functional tests switched from raw node lists to mninfo-based access for DKG queries.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client as RPC Client
    participant RPC as quorum_* RPC handler
    participant Node as Node Runtime
    participant Ctx as (Active|Observer)Context
    participant DKG as CDKGDebugManager
    participant Sess as CDKGSessionManager

    Note over RPC,Node: client calls quorum_dkgstatus / quorum_dkginfo
    Client->>RPC: request quorum_dkgstatus/quorum_dkginfo
    RPC->>Node: determine mode (masternode ? watch-only ?)
    Node->>Ctx: get context (ActiveContext or ObserverContext)
    Ctx->>DKG: provide owned CDKGDebugManager (dkgdbgman)
    RPC->>DKG: request debug status
    DKG->>Sess: CDKGSessionManager uses DKG for sessions
    Sess-->>DKG: session/status data
    DKG-->>RPC: debug JSON data (uses GetJsonHelp(inner_optional))
    RPC-->>Client: responds with JSON

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 23.81% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: adding mode restrictions for RPC quorum commands and refactoring CDKGDebugManager into context structures.
Description check	✅ Passed	The description is related to the changeset, mentioning dependencies, breaking changes, and test/documentation updates that align with the PR's scope.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

Caution

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

⚠️ Outside diff range comments (1)

src/llmq/core_write.cpp (1)

1-279: Run clang-format to fix column limit violations

The code contains multiple lines exceeding the 120-character column limit defined in .clang-format. Lines 35–38, 41, 43, 45, 47, 49, 55, 59, 63, 67, 71, 75, 127, 129, 130, 171 and others exceed this limit. Run clang-format on this file and commit the formatting changes.

🧹 Nitpick comments (2)

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

18-51: ObserverContext now owns its DKG debug/session managers; API and lifetime look consistent

The header changes cleanly move ownership of CDKGDebugManager into ObserverContext via a const std::unique_ptr, alongside the existing qdkgsman. The updated constructor signature reflects this, and the member order is compatible with the .cpp initializer list. Public exposure of dkgdbgman enables RPCs to access the debug manager without leaking ownership.

If you want to tighten encapsulation later, a CDKGDebugManager* DebugManager() const accessor would avoid exposing the std::unique_ptr itself while leaving semantics unchanged.

src/rpc/quorums.cpp (1)

344-427: Clarify dkgstatus behavior on non-DKG nodes and align sporkman access

The new implementation conditionally fills the DKG status portion only when an ActiveContext or ObserverContext is present, while still always computing quorumConnections/minableCommitments. On plain full nodes without either context, callers will now get an object missing time/timeStr/session but still containing the two arrays.

If the intent of this PR is to restrict quorum dkgstatus to “nodes that can see DKG” in the same way as quorum dkginfo, consider instead returning a clear RPC error when both node.active_ctx and node.observer_ctx are null, to avoid partially populated results that don’t match user expectations.

Also, the two GetQuorumConnections calls use *CHECK_NONFATAL(node.sporkman) and *node.sporkman respectively. For clarity and to make the invariant explicit, it would be better to treat both accesses consistently:

Suggested consistency tweak for sporkman access

-                    auto allConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, *llmq_ctx.qsnapman,
-                                                                            chainman, *CHECK_NONFATAL(node.sporkman),
-                                                                            pQuorumBaseBlockIndex,
-                                                                            node.mn_activeman->GetProTxHash(),
-                                                                            /*onlyOutbound=*/false);
-                    auto outboundConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman,
-                                                                                 *llmq_ctx.qsnapman, chainman,
-                                                                                 *node.sporkman, pQuorumBaseBlockIndex,
-                                                                                 node.mn_activeman->GetProTxHash(),
-                                                                                 /*onlyOutbound=*/true);
+                    auto allConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, *llmq_ctx.qsnapman,
+                                                                            chainman, *CHECK_NONFATAL(node.sporkman),
+                                                                            pQuorumBaseBlockIndex,
+                                                                            node.mn_activeman->GetProTxHash(),
+                                                                            /*onlyOutbound=*/false);
+                    auto outboundConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman,
+                                                                                 *llmq_ctx.qsnapman, chainman,
+                                                                                 *CHECK_NONFATAL(node.sporkman),
+                                                                                 pQuorumBaseBlockIndex,
+                                                                                 node.mn_activeman->GetProTxHash(),
+                                                                                 /*onlyOutbound=*/true);

Please confirm whether you prefer the current “partial output on non-DKG nodes” behavior, or if aligning quorum dkgstatus with quorum dkginfo (by throwing when no DKG-capable context exists) would better match the intended restriction.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fdf3ab3 and 66f421a.

📒 Files selected for processing (14)

• doc/release-notes-7062.md
• src/init.cpp
• src/llmq/context.cpp
• src/llmq/context.h
• src/llmq/core_write.cpp
• src/llmq/debug.h
• src/llmq/observer/context.cpp
• src/llmq/observer/context.h
• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/rpc/quorums.cpp
• test/functional/feature_llmq_rotation.py
• test/functional/feature_llmq_simplepose.py
• test/functional/test_framework/test_framework.py

💤 Files with no reviewable changes (2)

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

🧰 Additional context used

📓 Path-based instructions (8)

{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted

Files:

• doc/release-notes-7062.md

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.h
• src/llmq/observer/context.h
• src/llmq/debug.h
• src/rpc/quorums.cpp
• src/init.cpp
• src/llmq/core_write.cpp
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.h
• src/masternode/active/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.h
• src/llmq/observer/context.h
• src/llmq/debug.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.h
• src/llmq/observer/context.h
• src/llmq/debug.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.h
• src/llmq/observer/context.h
• src/llmq/debug.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/llmq/observer/context.h
• src/llmq/debug.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.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_rotation.py
• test/functional/feature_llmq_simplepose.py
• test/functional/test_framework/test_framework.py

🧠 Learnings (14)

📓 Common learnings

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

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

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

📚 Learning: 2025-10-03T11:20:37.475Z

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

Applied to files:

• src/masternode/active/context.h
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: 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/masternode/active/context.h
• src/llmq/observer/context.h
• src/rpc/quorums.cpp
• src/init.cpp
• src/llmq/observer/context.cpp
• src/masternode/active/context.cpp

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

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

Applied to files:

• src/masternode/active/context.h

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/masternode/active/context.cpp

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

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

Applied to files:

• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/masternode/active/context.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/masternode/active/context.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/init.cpp
• src/masternode/active/context.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/masternode/active/context.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/masternode/active/context.h

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

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

Applied to files:

• src/masternode/active/context.h
• src/llmq/observer/context.h
• 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/rpc/quorums.cpp
• src/init.cpp
• src/masternode/active/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_simplepose.py
• test/functional/test_framework/test_framework.py

📚 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/init.cpp

🧬 Code graph analysis (7)

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

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

• llmq (30-35)

src/llmq/context.h (1)

• llmq (22-29)

src/net_processing.h (1)

• llmq (34-36)

src/node/context.h (1)

• llmq (38-40)

test/functional/feature_llmq_rotation.py (1)

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

• get_node (1223-1228)

src/llmq/debug.h (2)

src/evo/deterministicmns.h (8)

• nodiscard (88-116)
• nodiscard (263-271)
• nodiscard (273-286)
• nodiscard (426-429)
• nodiscard (431-438)
• nodiscard (506-524)
• nodiscard (526-544)
• nodiscard (546-561)

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/rpc/quorums.cpp (3)

src/interfaces/chain.h (1)

• node (40-42)

src/rpc/rawtransaction.cpp (1)

• WITH_LOCK (651-651)

src/llmq/utils.cpp (2)

• GetQuorumConnections (733-757)
• GetQuorumConnections (733-736)

test/functional/feature_llmq_simplepose.py (1)

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

• wait_for_quorum_commitment (2068-2089)

src/init.cpp (4)

src/interfaces/node.h (2)

• node (48-50)
• node (481-481)

src/kernel/coinstats.h (1)

• node (19-21)

src/rpc/blockchain.h (1)

• node (33-36)

src/init.h (1)

• node (23-25)

src/llmq/core_write.cpp (1)

src/evo/core_write.cpp (16)

• GetJsonHelp (88-103)
• GetJsonHelp (88-88)
• GetJsonHelp (124-135)
• GetJsonHelp (124-124)
• GetJsonHelp (149-161)
• GetJsonHelp (149-149)
• GetJsonHelp (181-193)
• GetJsonHelp (181-181)
• GetJsonHelp (195-218)
• GetJsonHelp (195-195)
• GetJsonHelp (253-276)
• GetJsonHelp (253-253)
• GetJsonHelp (278-298)
• GetJsonHelp (278-278)
• GetJsonHelp (325-336)
• GetJsonHelp (325-325)

🪛 Cppcheck (2.19.0)

src/rpc/quorums.cpp

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

(preprocessorErrorDirective)

src/llmq/observer/context.cpp

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

(syntaxError)

🪛 GitHub Actions: Clang Diff Format Check

src/rpc/quorums.cpp

[error] 1-1: Clang format differences detected in quorums.cpp. Differences exist between 'before' and 'after' formatting. Run the clang-format-diff tool and apply formatting changes.

src/llmq/core_write.cpp

[error] 1-1: Clang format differences detected in core_write.cpp. Differences exist between 'before' and 'after' formatting. Run the clang-format-diff tool and apply formatting changes.

⏰ 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). (8)

• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: win64-build / Build source

🔇 Additional comments (14)

doc/release-notes-7062.md (1)

1-8: LGTM! Clear documentation of breaking RPC changes.

The release notes accurately describe the new restrictions on quorum dkginfo and quorum dkgstatus RPCs, making it clear that regular nodes without DKG insight can no longer access these commands or certain fields.

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

2068-2088: LGTM! Consistent refactoring to use masternode info abstractions.

The method signature change from nodes to mninfos aligns with the broader pattern of using MasternodeInfo abstractions throughout the test framework. The internal implementation correctly retrieves each node via mn.get_node(self) before querying DKG status.

test/functional/feature_llmq_simplepose.py (1)

143-143: LGTM! Call site correctly updated.

The change from nodes to mninfos_online is consistent with the updated wait_for_quorum_commitment signature in the test framework.

test/functional/feature_llmq_rotation.py (2)

75-78: LGTM! Consistent shift to masternode info abstractions.

The refactoring from iterating over self.nodes to self.mninfo aligns with the broader pattern throughout the test framework. The code correctly retrieves each node via mn.get_node(self) before calling quorum("dkginfo").

---

97-101: LGTM! Second loop consistently updated.

This loop follows the same pattern as the first, correctly iterating over masternode info and deriving nodes when needed.

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

31-31: LGTM! Proper ownership transfer to ActiveContext.

The addition of dkgdbgman as a member follows the correct pattern:

• Forward declaration properly added
• unique_ptr ensures RAII ownership semantics
• Placement as a public member makes it accessible for DKG-related operations

This aligns with the broader refactoring to move debug manager ownership from LLMQContext to context-specific implementations. Based on learnings, member lifetime will be properly managed via explicit reset() calls during shutdown.

Also applies to: 67-67

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

14-14: LGTM! Necessary include added.

The inclusion of llmq/debug.h is required to access the full definition of CDKGDebugManager for instantiation.

---

29-32: LGTM! Correct instantiation and wiring of the debug manager.

The changes properly:

1. Instantiate dkgdbgman using std::make_unique in the member initializer list
2. Pass the dereferenced debug manager (*dkgdbgman) to CDKGSessionManager instead of *llmq_ctx.dkg_debugman
3. Maintain the correct parameter order for the constructor

This completes the ownership transfer from LLMQContext to ActiveContext, enabling masternode-specific debug management.

src/llmq/debug.h (1)

95-95: LGTM! Backward-compatible API extension.

Adding the inner_optional parameter with a default value of false is a good design choice that:

• Maintains backward compatibility for existing callers
• Provides granular control over JSON schema field optionality
• Supports the RPC changes that conditionally emit time, timeStr, and session fields based on node mode

The signature change is safe and well-designed.

src/init.cpp (1)

2218-2231: ObserverContext construction correctly updated to new ownership model

The watcher-mode ObserverContext is now constructed with only LLMQ/chain/spork dependencies, matching the new constructor and keeping ownership of the DKG debug/session managers inside the context. Given the prior guarantee that llmq_ctx subcomponents are initialized once llmq_ctx exists, these dereferences are safe and the shutdown sequence still resets observer_ctx before LLMQ teardown. Based on learnings, this wiring looks sound.

src/llmq/core_write.cpp (1)

80-95: Inner optionality flag cleanly decouples DKG status envelope from inner fields

Using inner_optional only for time, timeStr, and session lets the outer DKG status object remain non-optional while allowing those fields to be omitted when no local DKG debug data is available (as in the new quorum_dkgstatus behavior). The implementation matches the updated declaration in llmq/debug.h and is consistent with how quorum_dkgstatus_help now constructs its schema.

Please double‑check other call sites of CDKGDebugStatus::GetJsonHelp (if any beyond quorum_dkgstatus_help) to ensure they either rely on the default inner_optional=false or explicitly pass the right value for their semantics.

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

7-36: ObserverContext internalizes debug/session manager wiring correctly

The constructor now creates its own CDKGDebugManager and passes it into CDKGSessionManager, then connects the session manager to CQuorumManager. Initialization order matches member declaration order, and the destructor cleanly disconnects the manager. UpdatedBlockTip’s guard against IBD/disconnect-only updates before delegating to qdkgsman is unchanged and safe.

src/rpc/quorums.cpp (2)

298-320: Updated dkgstatus help correctly treats DKG status fields as optional and extends schema

Building the help result from CDKGDebugStatus::GetJsonHelp(/*inner_optional=*/true) makes the time, timeStr, and session keys explicitly optional, which matches the new behavior where nodes without DKG visibility may omit them entirely. Appending the quorumConnections and minableCommitments arrays into mod_inner keeps the overall schema coherent for the RPC doc checker.

---

1002-1026: dkginfo restriction cleanly enforces masternode/watch-only mode

The explicit check for !node.active_ctx && !node.observer_ctx with a clear error message ensures quorum dkginfo is only usable when the node runs in masternode or watch-only (quorums_watch) mode. Using the context’s owned dkgdbgman to populate CDKGDebugStatus removes the old dependency on a global debug object and is consistent with the new ownership model.

If there are functional tests that previously called quorum dkginfo on non-masternode, non-watch-only nodes, make sure they’ve been updated to only exercise this RPC where a context with DKG visibility is present.

[PastaPastaPasta]

utACK 66f421a

[UdjinM6]

utACK 66f421a

[github-actions]

This pull request has conflicts, please rebase.

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (2)

src/llmq/core_write.cpp (1)

80-95: Inner optional flag for DKG debug help is correct; fix formatting for CI

The new inner_optional parameter cleanly scopes optionality to the time, timeStr, and session fields while preserving the top-level CDKGDebugStatus object’s optional behavior. This matches how quorum_dkgstatus_help() wants to expose fields that may be absent depending on context.

CI is currently failing on clang-format for this file; please run the project’s clang-format-diff step over the modified hunks before merging.

src/rpc/quorums.cpp (1)

983-1027: quorum_dkginfo restriction and context-local debug manager usage look correct; remember clang-format

The updated quorum dkginfo handler:

• Explicitly rejects requests when neither node.active_ctx nor node.observer_ctx is present, with "Only available in masternode or watch-only mode.".
• Uses the appropriate context-local CDKGDebugManager (active_ctx->dkgdbgman or observer_ctx->dkgdbgman) to fill CDKGDebugStatus, matching the refactor that moved debug ownership out of LLMQContext.
• Leaves the next_dkg computation unchanged and independent of the context, which is fine since it only depends on consensus parameters and tip height.

This aligns cleanly with the PR’s goal of restricting DKG introspection to nodes that actually track DKG state via the new contexts.

CI is currently flagging clang-format differences in this file; before merging, please run the project’s clang-format-diff tooling over the modified hunks so the formatting check passes.

🧹 Nitpick comments (1)

src/rpc/quorums.cpp (1)

344-427: quorum_dkgstatus behavior on non-DKG nodes and sporkman usage—please confirm intent

The new logic:

• Pulls CDKGDebugManager from node.active_ctx or node.observer_ctx if present and uses it to populate the DKG debug portion of the result.
• Still computes and returns quorumConnections and minableCommitments for all nodes with an LLMQContext, even when both contexts are missing (in which case the top-level object contains only these two fields).

That means quorum dkgstatus no longer hard-fails on non-masternode, non-watch-only nodes; it just omits the DKG-specific parts. In contrast, quorum dkginfo now throws in that situation. If the release-notes intent was that both RPCs be entirely unavailable on nodes that can’t see DKG, this asymmetry might be surprising—please double-check that this partial result on plain nodes is what you want.

Minor consistency nit: in the quorum-connections section you use *CHECK_NONFATAL(node.sporkman) for allConnections but bare *node.sporkman for outboundConnections. Given sporkman is expected to be non-null here, it would be slightly clearer either to use CHECK_NONFATAL in both places or omit it in both.

Also applies to: 383-390

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 66f421a and e98fe4f.

📒 Files selected for processing (14)

• doc/release-notes-7062.md
• src/init.cpp
• src/llmq/context.cpp
• src/llmq/context.h
• src/llmq/core_write.cpp
• src/llmq/debug.h
• src/llmq/observer/context.cpp
• src/llmq/observer/context.h
• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/rpc/quorums.cpp
• test/functional/feature_llmq_rotation.py
• test/functional/feature_llmq_simplepose.py
• test/functional/test_framework/test_framework.py

💤 Files with no reviewable changes (2)

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

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

• test/functional/feature_llmq_simplepose.py
• src/llmq/debug.h
• test/functional/feature_llmq_rotation.py

🧰 Additional context used

📓 Path-based instructions (8)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/llmq/core_write.cpp
• src/rpc/quorums.cpp
• src/llmq/observer/context.h
• src/init.cpp
• src/llmq/observer/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.h
• src/llmq/observer/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.h
• src/llmq/observer/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/llmq/core_write.cpp
• src/llmq/observer/context.h
• src/llmq/observer/context.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/llmq/core_write.cpp
• src/llmq/observer/context.h
• src/llmq/observer/context.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/test_framework/test_framework.py

{guix-build*,releases,**/guix-build*,releases/**,.github/**,depends/**,ci/**,contrib/**,doc/**}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to build system files (guix-build*), release artifacts, or avoid changes to .github, depends, ci, contrib, and doc directories unless specifically prompted

Files:

• doc/release-notes-7062.md

🧠 Learnings (15)

📓 Common learnings

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

📚 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/masternode/active/context.cpp
• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/llmq/observer/context.h
• src/init.cpp
• src/llmq/observer/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/masternode/active/context.cpp
• src/masternode/active/context.h
• src/llmq/observer/context.cpp

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

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

Applied to files:

• src/masternode/active/context.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/rpc/quorums.cpp
• src/init.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

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

Applied to files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• src/rpc/quorums.cpp

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

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

Applied to files:

• src/masternode/active/context.cpp
• src/masternode/active/context.h
• 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} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

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

Applied to files:

• src/masternode/active/context.h

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

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

Applied to files:

• src/masternode/active/context.h
• src/llmq/observer/context.h

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

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

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

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

Applied to files:

• src/init.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/test_framework/test_framework.py

🧬 Code graph analysis (3)

src/llmq/core_write.cpp (1)

src/evo/core_write.cpp (16)

• GetJsonHelp (88-103)
• GetJsonHelp (88-88)
• GetJsonHelp (124-135)
• GetJsonHelp (124-124)
• GetJsonHelp (149-161)
• GetJsonHelp (149-149)
• GetJsonHelp (181-193)
• GetJsonHelp (181-181)
• GetJsonHelp (195-218)
• GetJsonHelp (195-195)
• GetJsonHelp (253-276)
• GetJsonHelp (253-253)
• GetJsonHelp (278-298)
• GetJsonHelp (278-278)
• GetJsonHelp (325-336)
• GetJsonHelp (325-325)

src/rpc/quorums.cpp (1)

src/llmq/utils.cpp (2)

• GetQuorumConnections (678-700)
• GetQuorumConnections (678-679)

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

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

• llmq (30-35)

src/llmq/context.h (1)

• llmq (22-29)

src/net_processing.h (1)

• llmq (34-36)

src/node/context.h (1)

• llmq (38-40)

🪛 Cppcheck (2.19.0)

src/rpc/quorums.cpp

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

(preprocessorErrorDirective)

src/llmq/observer/context.cpp

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

(syntaxError)

🪛 GitHub Actions: Clang Diff Format Check

src/llmq/core_write.cpp

[error] 1-1: Clang format differences found. The CI step failed due to formatting changes detected by clang-format-diff.py. Please reformat this file (and related changes) to pass the formatting check. Command: git diff -U0 origin/develop -- $(git ls-files -- $(cat test/util/data/non-backported.txt)) | ./contrib/devtools/clang-format-diff.py -p1 > diff_output.txt

src/rpc/quorums.cpp

[error] 1-1: Clang format differences found. The CI step failed due to formatting changes detected by clang-format-diff.py. Please reformat this file to pass the formatting check. Command: git diff -U0 origin/develop -- $(git ls-files -- $(cat test/util/data/non-backported.txt)) | ./contrib/devtools/clang-format-diff.py -p1 > diff_output.txt

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: arm-linux-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: Lint / Run linters

🔇 Additional comments (8)

doc/release-notes-7062.md (1)

1-8: Release notes clearly document breaking changes to DKG RPC commands.

The release notes effectively communicate the two RPC behavior changes and their rationale. Each change (quorum dkginfo requiring specific modes, quorum dkgstatus removing DKG-related fields on regular nodes) is well-explained.

Please verify that the RPC handler implementations for quorum dkginfo and quorum dkgstatus (in src/rpc/ or equivalent) properly enforce these restrictions and return appropriate errors or omit fields as documented.

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

2068-2172: LGTM! Refactor aligns with PR objectives.

The refactor of wait_for_quorum_commitment to accept mninfos instead of nodes is well-executed:

• Function signature, implementation, and call site are all consistently updated
• The new approach of iterating over masternode infos and accessing nodes via mn.get_node(self) aligns with the broader DKG context refactor described in the PR
• This change supports the restriction of DKG RPCs to only nodes that can see DKG, as mentioned in the PR objectives

Based on learnings, this change is directly related to MasternodeInfo refactoring and avoids scope creep.

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

31-35: CDKGDebugManager ownership in ActiveContext looks sound

Forward-declaring llmq::CDKGDebugManager and adding const std::unique_ptr<llmq::CDKGDebugManager> dkgdbgman alongside qdkgsman cleanly localizes DKG debug state to ActiveContext and exposes it for RPCs without changing lifetimes. No issues from an ownership or null-safety perspective.

Also applies to: 65-70

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

14-16: Masternode DKG debug wiring is consistent and safe

Including <llmq/debug.h>, constructing dkgdbgman before qdkgsman, and passing *dkgdbgman into CDKGSessionManager preserves the previous dependency ordering while moving debug ownership into ActiveContext. Object lifetime and registration with m_llmq_ctx.qman remain well-defined.

Also applies to: 26-41

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

7-8: ObserverContext-local DKG debug manager is wired correctly

Constructing dkgdbgman inside ObserverContext and passing *dkgdbgman into CDKGSessionManager (with mn_activeman=nullptr and quorums_watch=true) gives watch-only nodes their own DKG debug state without altering existing connect/disconnect semantics. This matches the ActiveContext refactor and keeps lifetimes clear.

Also applies to: 12-23

src/init.cpp (1)

2218-2231: ObserverContext construction matches updated interface

The revised ObserverContext construction in AppInitMain passes arguments in the same order as the new constructor signature (BLS worker, DMM, meta man, quorum block processor, quorum manager, snapshot manager, chainman, sporkman, db params). This preserves the previous mode-selection logic (quorums_watch and non-masternode) without introducing lifetime issues.

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

19-24: ObserverContext interface and debug manager ownership are consistent

Removing the external CDKGDebugManager& parameter from the constructor and adding the dkgdbgman unique_ptr member makes ObserverContext self-contained with respect to DKG debug state. The updated ctor parameters align with the new call site in init.cpp, and exposing dkgdbgman alongside qdkgsman is reasonable for RPC access.

Also applies to: 38-41, 49-51

src/rpc/quorums.cpp (1)

24-25: quorum_dkgstatus_help correctly layers DKG and quorum metadata

Including the observer context and reusing CDKGDebugStatus::GetJsonHelp("", /*optional=*/false, /*inner_optional=*/true) as the base, then appending quorumConnections and minableCommitments, keeps the help text in sync with the underlying DKG debug object while making the extra arrays explicit. The use of inner_optional matches the fact that the time/session fields may be absent when no local DKG debug data exists.

Also applies to: 298-320