refactor: streamline LLMQ utils and snapshot logic #7081
Conversation
|
|
This pull request has conflicts, please rebase. |
Also reorder them to help us get rid of the forward declarations, review with `git log -p -n1 --color-moved=dimmed_zebra`.
kwvg
changed the title
llmq/cycles.{cpp,h}, streamline llmq/utils.{cpp,h}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request centralizes LLMQ context into a new llmq::UtilParameters struct and updates many APIs to accept that single context instead of separate managers/index arguments. It also: converts SnapshotSkipMode to a scoped enum, replaces CQuorumRotationInfo’s flat fields with CycleData/CycleBase wrappers and related constructors/destructors, adds BlsCheck and other helper types, and changes InitQuorumsCache to require explicit consensus params. Call sites across dkgsession, blockprocessor, commitment, utils, quorums, rpc, tests, and others were updated to the new signatures. Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller (DKG / BlockProcessor / RPC)
participant Util as llmq::utils / CFinalCommitment
participant MPs as Managers (dmnman, qsnapman, chainman)
participant Spk as SporkManager
participant Cons as Consensus / CBlockIndex
rect rgb(235,245,255)
Note over Caller, MPs: OLD flow (multiple separate args)
Caller->>Util: CallFunc(..., dmnman, qsnapman, chainman, pIndex, ...)
Util->>MPs: access dmnman/qsnapman/chainman directly
MPs-->>Util: return data (members/snapshots)
Util->>Cons: use pIndex for consensus/deployment checks
Util-->>Caller: result
end
rect rgb(255,245,235)
Note over Caller, MPs: NEW flow (aggregated UtilParameters)
Caller->>Util: CallFunc(..., {dmnman, qsnapman, chainman, pBaseIndex}, sporkman, consensus)
Util->>MPs: use util_params.m_dmnman / m_qsnapman / m_chainman / m_base_index
MPs-->>Util: return data (members/snapshots)
Util->>Cons: use util_params.m_chainman.GetConsensus() & util_params.m_base_index
Util-->>Caller: result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/llmq/utils.cpp (2)
250-261: Minor typo in variable name.
processesdSkipListappears to be a typo forprocessedSkipList. This is an internal variable, so it doesn't affect functionality.🔎 Suggested fix
- std::vector<int> processesdSkipList; + std::vector<int> processedSkipList; for (const auto& s : snapshot.mnSkipList) { if (first_entry_index == 0) { first_entry_index = s; - processesdSkipList.push_back(s); + processedSkipList.push_back(s); } else { - processesdSkipList.push_back(first_entry_index + s); + processedSkipList.push_back(first_entry_index + s); } } int idx = 0; - auto itsk = processesdSkipList.begin(); + auto itsk = processedSkipList.begin(); for (const size_t i : irange::range(numQuorums)) { while (quarterQuorumMembers[i].size() < quarterSize) { - if (itsk != processesdSkipList.end() && idx == *itsk) { + if (itsk != processedSkipList.end() && idx == *itsk) {
375-382: Empty catch blocks may hide unexpected errors.The empty catch blocks for
std::runtime_errorappear intentional for handling duplicate MN additions. However, silently swallowing allruntime_errorexceptions could mask other unexpected issues.Consider logging at debug level when exceptions occur to aid debugging without changing behavior:
🔎 Suggested improvement
try { MnsUsedAtH.AddMN(mn); - } catch (const std::runtime_error& e) { + } catch (const std::runtime_error&) { + // Expected: duplicate MN, safe to ignore } try { MnsUsedAtHIndexed[idx].AddMN(mn); - } catch (const std::runtime_error& e) { + } catch (const std::runtime_error&) { + // Expected: duplicate MN, safe to ignore }src/test/llmq_snapshot_tests.cpp (1)
23-92: Snapshot tests correctly migrated toSnapshotSkipModeand still cover construction, serialization, and edge casesThe tests now:
- use
SnapshotSkipModevalues instead of raw ints,- iterate over all four modes via a constexpr
std::array, and- exercise default, parameterized, move, large/empty, bit-pattern, JSON, and malformed serialization paths.
This gives good confidence in the new enum and serialization behavior.
It might be worth explicitly including
<array>in this file to avoid relying on transitive includes forstd::array.Also applies to: 94-162, 233-279
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/evo/cbtx.cppsrc/evo/specialtxman.cppsrc/llmq/blockprocessor.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/debug.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.cppsrc/llmq/snapshot.hsrc/llmq/utils.cppsrc/llmq/utils.hsrc/rpc/quorums.cppsrc/test/llmq_snapshot_tests.cpp
🧰 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/rpc/quorums.cppsrc/evo/cbtx.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/test/llmq_snapshot_tests.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
src/{masternode,evo}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/cbtx.cppsrc/evo/specialtxman.cpp
src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Files:
src/evo/cbtx.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
src/evo/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Files:
src/evo/cbtx.cppsrc/evo/specialtxman.cpp
src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Files:
src/evo/cbtx.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
src/{masternode,llmq}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
BLS integration must be used for cryptographic foundation of advanced masternode features
Files:
src/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cpp
src/llmq/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Files:
src/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.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/llmq_snapshot_tests.cpp
🧠 Learnings (23)
📓 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/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: 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: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Learnt from: knst
Repo: dashpay/dash PR: 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-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Applied to files:
src/rpc/quorums.cppsrc/evo/cbtx.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/test/llmq_snapshot_tests.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Applied to files:
src/rpc/quorums.cppsrc/evo/cbtx.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/rpc/quorums.cppsrc/evo/cbtx.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/core_write.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Applied to files:
src/evo/cbtx.cppsrc/llmq/quorums.cppsrc/llmq/utils.cpp
📚 Learning: 2025-08-19T15:08:00.835Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/commitment.cpp:54-60
Timestamp: 2025-08-19T15:08:00.835Z
Learning: In Dash Core, llmq_params.size == 1 is used only for regtest environments, not on public networks, which significantly reduces the risk profile of code paths specific to single-member quorums.
Applied to files:
src/evo/cbtx.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.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/dkgsession.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.hsrc/llmq/debug.cppsrc/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Applied to files:
src/llmq/commitment.cppsrc/llmq/snapshot.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/utils.cpp
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/llmq/commitment.hsrc/llmq/snapshot.hsrc/llmq/utils.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/llmq/core_write.cppsrc/llmq/snapshot.cppsrc/test/llmq_snapshot_tests.cppsrc/llmq/snapshot.hsrc/llmq/utils.cppsrc/evo/specialtxman.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/dkgsessionmgr.cppsrc/llmq/quorums.cppsrc/llmq/dkgsessionhandler.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing
Applied to files:
src/llmq/blockprocessor.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/llmq/blockprocessor.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/utils.cpp
📚 Learning: 2025-11-13T20:02:55.480Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6969
File: src/evo/deterministicmns.h:441-479
Timestamp: 2025-11-13T20:02:55.480Z
Learning: In `src/evo/deterministicmns.h`, the `internalId` field in `CDeterministicMN` and the `mnInternalIdMap` in `CDeterministicMNList` are non-deterministic and used only for internal bookkeeping and efficient lookups. Different nodes can assign different internalIds to the same masternode depending on their sync history. Methods like `IsEqual()` intentionally ignore internalId mappings and only compare consensus-critical deterministic fields (proTxHash, collateral, state, etc.).
Applied to files:
src/llmq/utils.hsrc/llmq/utils.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/utils.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
src/llmq/utils.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/llmq/utils.cppsrc/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/llmq/utils.cpp
📚 Learning: 2025-11-04T18:23:28.175Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.
Applied to files:
src/llmq/utils.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/evo/specialtxman.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/**/*.{cpp,h} : Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Applied to files:
src/evo/specialtxman.cpp
🧬 Code graph analysis (13)
src/rpc/quorums.cpp (1)
src/llmq/utils.cpp (2)
GetQuorumConnections(678-700)GetQuorumConnections(678-679)
src/llmq/commitment.h (1)
src/llmq/utils.h (3)
utils(47-95)llmq(29-31)llmq(33-96)
src/llmq/core_write.cpp (1)
src/llmq/snapshot.h (1)
mnSkipListMode(45-57)
src/llmq/dkgsessionmgr.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/snapshot.cpp (1)
src/llmq/snapshot.h (2)
WORK_DIFF_DEPTH(39-39)CQuorumRotationInfo(114-145)
src/llmq/blockprocessor.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/quorums.cpp (1)
src/llmq/utils.cpp (4)
EnsureQuorumConnections(778-829)EnsureQuorumConnections(778-780)GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/snapshot.h (2)
src/llmq/quorums.h (2)
llmq(366-377)llmq(379-379)src/llmq/snapshot.cpp (7)
CQuorumSnapshot(230-230)CQuorumSnapshot(232-238)CQuorumSnapshot(240-240)CQuorumRotationInfo(242-242)CQuorumRotationInfo(244-244)GetCycles(246-254)GetCycles(246-246)
src/llmq/dkgsessionhandler.cpp (1)
src/llmq/utils.cpp (4)
EnsureQuorumConnections(778-829)EnsureQuorumConnections(778-780)AddQuorumProbeConnections(831-874)AddQuorumProbeConnections(831-833)
src/llmq/utils.h (3)
src/evo/deterministicmns.h (1)
CDeterministicMNList(134-195)src/llmq/utils.cpp (17)
BlsCheck(528-528)BlsCheck(530-536)BlsCheck(538-538)DeterministicOutboundConnection(656-676)DeterministicOutboundConnection(656-656)CalcDeterministicWatchConnections(757-776)CalcDeterministicWatchConnections(757-758)GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)GetQuorumConnections(678-700)GetQuorumConnections(678-679)GetQuorumRelayMembers(702-755)GetQuorumRelayMembers(702-703)EnsureQuorumConnections(778-829)EnsureQuorumConnections(778-780)AddQuorumProbeConnections(831-874)AddQuorumProbeConnections(831-833)src/llmq/params.h (1)
LLMQType(14-125)
src/llmq/debug.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/utils.cpp (3)
src/llmq/snapshot.h (1)
WORK_DIFF_DEPTH(39-39)src/llmq/options.cpp (2)
IsQuorumRotationEnabled(45-57)IsQuorumRotationEnabled(45-45)src/llmq/dkgsession.h (1)
quorumIndex(297-297)
src/evo/specialtxman.cpp (2)
src/llmq/commitment.cpp (2)
CheckLLMQCommitment(216-283)CheckLLMQCommitment(216-216)src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
🪛 Cppcheck (2.19.0)
src/llmq/snapshot.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🪛 GitHub Actions: Clang Diff Format Check
src/evo/specialtxman.cpp
[error] 1-1: Process completed with exit code 1.
⏰ 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). (9)
- GitHub Check: mac-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: arm-linux-build / Build source
🔇 Additional comments (47)
src/evo/cbtx.cpp (1)
74-76: LGTM!The explicit passing of consensus parameters to
InitQuorumsCachealigns with the PR's objective to eliminate globalParams()usage within utility functions and make dependencies explicit.src/llmq/dkgsessionmgr.cpp (3)
146-148: LGTM!The explicit consensus parameter passing to
InitQuorumsCacheis consistent with the refactor pattern.
298-298: LGTM!The aggregate initializer
{m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}correctly constructs aUtilParameterscontext as defined insrc/llmq/utils.h.
347-347: LGTM!Consistent use of the composite context pattern for
GetAllQuorumMembers.src/llmq/debug.cpp (1)
31-31: LGTM!The aggregate initializer correctly constructs the
UtilParameterscontext forGetAllQuorumMembers, consistent with the refactoring pattern used throughout the PR.src/llmq/core_write.cpp (2)
185-201: LGTM!The serialization correctly accesses the
CycleDatawrapper members (m_snap,m_diff) and properly usesCHECK_NONFATALfor the optionalcycleHMinus4CwhenextraShareis true. This aligns with the structural changes toCQuorumRotationInfo.
245-245: LGTM!Using
ToUnderlying(mnSkipListMode)is correct sinceSnapshotSkipModewas converted from a plain enum toenum class, which requires explicit casting for numeric output.src/rpc/quorums.cpp (1)
380-387: LGTM!Both calls to
GetQuorumConnectionscorrectly use the updated signature withm_sporkmanas a separate parameter and the composite context{*node.dmnman, *llmq_ctx.qsnapman, chainman, pQuorumBaseBlockIndex}forUtilParameters. TheonlyOutboundflag correctly differentiates between all connections (false) and outbound-only (true).src/evo/specialtxman.cpp (2)
133-133: LGTM!The composite context
{dmnman, qsnapman, chainman, pindexPrev}correctly constructs aUtilParametersforCheckLLMQCommitment. This is a parameter restructuring with no behavioral change to the consensus-critical validation.
445-448: Address the pipeline failure for clang-format.The logic is correct, but the CI indicates a clang-format check failure. Please run
clang-formaton this file to resolve the formatting discrepancy.⛔ Skipped due to learnings
Learnt from: kwvg Repo: dashpay/dash PR: 6529 File: src/rpc/governance.cpp:1074-1089 Timestamp: 2025-02-14T15:15:58.165Z Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.Learnt from: kwvg Repo: dashpay/dash PR: 6729 File: src/rpc/evo.cpp:1273-1273 Timestamp: 2025-07-09T15:05:36.250Z Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.Learnt from: CR Repo: dashpay/dash PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-24T16:41:22.457Z Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)Learnt from: kwvg Repo: dashpay/dash PR: 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.src/llmq/commitment.h (3)
35-39: LGTM!The forward declarations for
UtilParametersandBlsCheckare correctly scoped within thellmqnamespace, enabling the updated function signatures to reference them without including full headers.
79-81: LGTM!The updated signatures for
VerifySignatureAsyncandVerifycorrectly acceptconst llmq::UtilParameters&as the consolidated context parameter, replacing the previous multi-parameter form.
167-167: LGTM!The
CheckLLMQCommitmentsignature correctly takesconst llmq::UtilParameters&as the first parameter, consistent with the implementation and call sites throughout the codebase.src/llmq/dkgsessionhandler.cpp (1)
568-572: LGTM!Both
EnsureQuorumConnectionsandAddQuorumProbeConnectionscorrectly use the updated signatures withm_sporkmanas a separate parameter and the composite context{m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}forUtilParameters. This is consistent with the function signatures shown in the relevant code snippets fromsrc/llmq/utils.cpp.src/llmq/utils.cpp (16)
1-31: LGTM!Includes and forward declarations are appropriate for the refactored functionality.
33-86: LGTM!Well-structured type aliases and helper structs. The
MasternodeScore,QuorumQuarter, andPreviousQuorumQuartersabstractions cleanly encapsulate the data previously passed around as nested vectors. ThecalculateQuorumScoreimplementation correctly follows the documented hash computation.
88-111: LGTM!The
GetHashModifierfunction correctly receivesconsensus_paramsexplicitly instead of relying on global state. The logic remains unchanged, which is appropriate for consensus-critical code. Based on learnings, consensus rules must be preserved exactly.
113-151: LGTM!The
CalculateScoresForQuorumoverloads are cleanly refactored to returnMasternodeScorestructs instead of pairs. The filtering logic for banned MNs and EvoNodes is preserved correctly.
153-182: LGTM!The template function correctly implements deterministic quorum calculation with descending score sorting. The tiebreaker logic using
collateralOutpointmaintains backward compatibility as documented.
287-301: LGTM!Non-rotation quorum member computation is correctly parametrized to pass consensus parameters explicitly to
GetHashModifier.
303-336: LGTM!
BuildQuorumSnapshotcorrectly receives explicit consensus parameters and follows the established pattern. The logic for building snapshot data is preserved.
478-484: Early break on missing snapshot leaves partial data.When a snapshot is not found for a previous cycle, the loop breaks early with a TODO comment about potentially throwing an exception. The
prev_cycles[idx]->m_memberswould remain empty (default initialized) for remaining cycles, which may lead to incomplete quorum composition.Since this is consensus-critical code and the existing behavior is being preserved (as indicated by the TODO comment), this is noted for awareness.
526-566: LGTM!
BlsCheckclass implementation is clean. The BLS signature verification correctly usesVerifySecureAggregatedfor multiple public keys andVerifyInsecurefor single keys, as per BLS integration requirements.
656-676: No changes to review.
DeterministicOutboundConnectionis unchanged.
678-700: LGTM!
GetQuorumConnectionscorrectly updated to useUtilParametersfor consolidated parameter passing.
702-755: LGTM!
GetQuorumRelayMemberscorrectly updated to useUtilParameters.
757-776: No changes to review.
CalcDeterministicWatchConnectionsis unchanged.
778-829: LGTM!
EnsureQuorumConnectionscorrectly updated to useUtilParametersthroughout, maintaining the same connection management logic.
831-874: LGTM!
AddQuorumProbeConnectionscorrectly updated to useUtilParametersfor consolidated parameter passing. Probe connection logic is preserved.
568-654: LGTM!
GetAllQuorumMemberscorrectly handles both rotation and non-rotation quorums with appropriate caching. The use ofUtilParametersconsolidates dependency passing cleanly. The implementation properly supports multiple LLMQ configurations (50/60, 400/60, 400/85) and usesunordered_lru_cachefor the indexed members cache as required by the codebase guidelines.src/llmq/blockprocessor.cpp (3)
30-38: PreComputeQuorumMembers now correctly usesUtilParametersThe new
GetAllQuorumMembers(params.type, {dmnman, qsnapman, chainman, pindex}, reset_cache)call cleanly bundles the previous parameter set intoUtilParameterswithout changing behavior.
48-56: Quorum mined-commitment cache initialization aligned with new helperUsing
utils::InitQuorumsCache(mapHasMinedCommitmentCache, m_chainstate.m_chainman.GetConsensus())matches the new cache initializer API and keeps the cache sizing tied to consensus LLMQ params.
148-149: CFinalCommitment verification call sites correctly migrated toUtilParametersAll
CFinalCommitment::VerifyandVerifySignatureAsynccalls now pass{m_dmnman, m_qsnapman, m_chainstate.m_chainman, pQuorumBaseBlockIndex}, preserving the previous context (DMN manager, snapshot manager, chainman, quorum base index) in the new aggregate form.Also applies to: 218-219, 339-341
src/llmq/dkgsession.cpp (2)
93-141: DKG session quorum member/relay lookups correctly switched toUtilParameters
GetAllQuorumMembersandGetQuorumRelayMembersnow receive{m_dmnman, m_qsnapman, m_chainman, m_quorum_base_block_index}, which preserves the prior context while matching the new utility signatures.
1283-1284: Final commitment verification uses the new aggregate contextBoth
FinalizeCommitments()andFinalizeSingleCommitment()now callfqc.Verify({m_dmnman, m_qsnapman, m_chainman, m_quorum_base_block_index}, true), consistent with the refactoredCFinalCommitment::VerifyAPI and the existing DKG base-index semantics.Also applies to: 1346-1347
src/llmq/commitment.cpp (3)
31-94:VerifySignatureAsynccorrectly migrated to useUtilParametersThe async signature verification now sources quorum members via
utils::GetAllQuorumMembers(llmqType, util_params), preserving the previous behavior while matching the new aggregate context API. The single-member and multi-member verification paths remain unchanged.
97-185:Verifynow derives all context fromUtilParametersUsing
util_params.m_base_indexandutil_params.m_chainmanfor:
- expected
nVersioncomputation,- quorum hash / index checks, and
- member enumeration via
GetAllQuorumMembers(llmqType, util_params)matches prior logic while centralizing the context. As long as callers pass the same base block index they previously used (quorum base in block/commitment paths), behavior stays identical.
Please confirm that all
CFinalCommitment::Verifycall sites now pass the quorum base block asm_base_index(or usereplace_index(...)where appropriate), so that thenVersionand quorum index checks still use the intended height context.
216-283:CheckLLMQCommitmentAPI change toUtilParameterslooks consistent, but relies on correct base indexThe function now:
- logs and validates
qcTx.nHeightagainstutil_params.m_base_index->nHeight + 1,- checks chain membership via
util_params.m_base_index->GetAncestor(...), and- verifies the commitment using
qcTx.commitment.Verify(util_params.replace_index(pQuorumBaseBlockIndex), false).This matches the previous semantics, assuming callers pass the same “previous block” index as
m_base_index. No consensus logic changes are introduced here.Please double-check that all callers constructing
UtilParametersforCheckLLMQCommitmentuse the intendedpindexPrev(or equivalent) form_base_index, since it now drives both the height check and the active-chain ancestor validation.src/llmq/quorums.cpp (2)
229-230: Quorum caches correctly re-initialized with consensus-aware helperBoth
mapQuorumsCacheandcleanupQuorumsCachenow useutils::InitQuorumsCache(cache, m_chainman.GetConsensus(), /*limit_by_connections=*/false), which cleanly centralizes sizing based onkeepOldKeysfor these long-lived quorum caches.Also applies to: 1116-1118
371-372: Quorum connection management now uses the aggregatedUtilParameters
EnsureQuorumConnectionsreceives{m_dmnman, m_qsnapman, m_chainman, quorum->m_quorum_base_block_index}, aligning this call site with the new utils APIs while preserving the previous context (DMN/snapshot managers, chainman, quorum base index).src/test/llmq_snapshot_tests.cpp (1)
164-231: Rotation-info tests updated forCycleDataand optionalcycleHMinus4CThe rotation-info tests now:
- assert the new default state (
extraSharefalse,cycleHMinus4Cdisengaged),- populate
cycleHMinus*C.m_snapand an optionalcycleHMinus4CviaCycleData, and- verify serialization round-trips with and without
extraShare, plus with populatedlastCommitmentPerIndexandquorumSnapshotList.This aligns well with the new
CycleData/extraSharemodel.src/llmq/snapshot.cpp (2)
21-45: Cycle-based rotation refactor is coherent and uses the newCycleDataabstraction correctly
ConstructCyclecleanly encapsulates cycle base/work index resolution and optional snapshot lookup, andBuildQuorumRotationInfonow:
- derives the base cycle via
ConstructCycle(..., skip_snap=true, ...),- fills
cycleHMinus*C(and optionalcycleHMinus4C) throughresponse.GetCycles()and subsequentConstructCyclecalls, and- builds MN list diffs along cycles and extra snapshot heights for both legacy and non-legacy paths.
This keeps the rotation logic centralized around
CycleDatawithout changing the externally visible data layout.Also applies to: 96-147, 162-185
18-19: Usestd::string(notstd::string_view) forDB_QUORUM_SNAPSHOTto keep DB serialization valid
DB_QUORUM_SNAPSHOTis now aconstexpr std::string_view, but it is passed directly intostd::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash)forCEvoDB::Read/Write. The DB layer’s serialization helpers are written forstd::string(and tuples/pairs thereof), notstd::string_view, so this change can lead to compilation or serialization failures.Reverting the key to a
std::string(or a plainconst char[]consistent with other llmq DB prefixes) will avoid issues and match the rest of the codebase.Suggested fix for DB key type
-namespace { -constexpr std::string_view DB_QUORUM_SNAPSHOT{"llmq_S"}; +namespace { +static const std::string DB_QUORUM_SNAPSHOT{"llmq_S"}; @@ - if (m_evoDb.Read(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot)) { + if (m_evoDb.Read(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot)) { @@ - m_evoDb.GetRawDB().Write(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot); + m_evoDb.GetRawDB().Write(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot);Also applies to: 264-292
⛔ Skipped due to learnings
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 listsLearnt from: kwvg Repo: dashpay/dash PR: 6815 File: src/chainlock/clsig.cpp:27-30 Timestamp: 2025-08-13T16:10:51.336Z Learning: The Dash codebase has explicit serialization support for std::string_view through template specializations in src/serialize.h (lines 976-987), allowing SerializeHash and other serialization functions to work directly with string_view without conversion to std::string.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 dataLearnt 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/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLitesrc/llmq/snapshot.h (2)
24-37:SnapshotSkipModeandCQuorumSnapshotrefactor are type-safe and serialization-awareReplacing the raw
intwithenum class SnapshotSkipMode(plusis_serializable_enumspecialization) and updatingmnSkipListModeaccordingly improves type safety while keeping the serialized representation stable. The added constructors/destructor forCQuorumSnapshotare simple data movers and align with the refactored tests.Also applies to: 41-52
104-122: Cycle-based layout inCQuorumRotationInfopreserves serialized structure while adding flexibilityIntroducing
CycleBase/CycleDataand storing:
cycleHMinusC/2C/3CasCycleData(snap + diff),- optional
cycleHMinus4CwhenextraShareis true,then serializing only the
{m_snap, m_diff}parts (with an explicit extraShare-gated H–4C block) keeps the on-disk format compatible while enabling the new cycle-aware API viaGetCycles(). Deleting the copy constructor avoids accidental shallow copies of pointer fields.Also applies to: 128-145, 174-208
src/llmq/utils.h (2)
33-46:UtilParametersandBlsCheckintroduce a clean, reusable context and check type
UtilParametersbundles the DMN manager, snapshot manager, chainman, and base index, simplifying all quorum utility call sites.utils::BlsCheckprovides the required functor interface (operator()/swap) and data fields for use withCCheckQueue, matching the new BLS verification pipeline.Also applies to: 48-61
69-86: Quorum utility APIs andInitQuorumsCacheare consistently refactored to use the new abstractionsThe quorum helpers (
GetAllQuorumMembers,GetQuorumConnections,GetQuorumRelayMembers,EnsureQuorumConnections,AddQuorumProbeConnections) now take aconst UtilParameters&, which keeps their signatures compact and consistent with other modules. The newInitQuorumsCache(cache, consensus_params, limit_by_connections)centralizes cache sizing for different LLMQ caches and is used appropriately across the codebase.Also applies to: 87-94
We can take the most commonly used functions and pass them through a struct to make the internal code more readable.
- Introduce `QuorumMembers` type alias
- Inline `IsV{19,20}Active()` helpers
- Deduplicate quorum member printing logic
There was a problem hiding this comment.
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/dkgsessionhandler.cpp (1)
1-673: Fix reported clang-format issues in this fileCI reports clang-format differences for
src/llmq/dkgsessionhandler.cpp. Please run clang-format over this file (and commit the result) to unblock the pipeline, e.g.:Example command
clang-format -i src/llmq/dkgsessionhandler.cppsrc/llmq/dkgsession.cpp (1)
1-1382: Resolve clang-format differences in this fileCI reports clang-format differences for
src/llmq/dkgsession.cpp. Please run clang-format on this file and commit the result to satisfy the formatting check, e.g.:Example command
clang-format -i src/llmq/dkgsession.cpp
🧹 Nitpick comments (5)
src/llmq/dkgsessionmgr.cpp (1)
145-150: Confirm InitQuorumsCache retention choice for indexedQuorumsCacheThe new call
utils::InitQuorumsCache(indexedQuorumsCache, m_chainman.GetConsensus());will size the per‑type LRU usingllmq.keepOldConnections(defaultlimit_by_connections=true). GivenindexedQuorumsCachestores quorum indices rather than connection state, please double‑check that using the connection‑based retention limit here (vs.keepOldKeysvialimit_by_connections=false) is intentional.src/llmq/commitment.h (1)
167-168: CheckLLMQCommitment signature refactor matches new verification flowUpdating
CheckLLMQCommitmentto take a UtilParameters context is consistent with the.cppimplementation and with other call sites (e.g., blockprocessor) feeding a single quorum context object.src/llmq/blockprocessor.cpp (1)
48-58: InitQuorumsCache for mined-commitment cache – verify intended retention basis
utils::InitQuorumsCache(mapHasMinedCommitmentCache, m_chainstate.m_chainman.GetConsensus());will size the per‑type LRU usingllmq.keepOldConnections(default behavior). SincemapHasMinedCommitmentCachetracks mined commitments rather than network connections, please confirm that using the connection‑based retention bound here (vs.keepOldKeysvialimit_by_connections=false) is intentional and sized appropriately for expected history depth.src/llmq/dkgsessionhandler.cpp (1)
567-573: DKG quorum-connection helpers now use the shared UtilParameters contextBoth
EnsureQuorumConnectionsandAddQuorumProbeConnectionsare wired to the new APIs with{m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}in the correct order; this centralizes all quorum‑connection helpers on the same context object.src/llmq/utils.cpp (1)
375-382: Add documentation comments for consistency with other AddMN calls in the same file.The empty
catch (const std::runtime_error& e)blocks at lines 377-378 and 381-382 silently ignore exceptions fromAddMN. While this pattern appears intentional, the same file documents this elsewhere (see the note around line 402: "AddMN is the one that can throw exceptions"). Adding a brief comment here would maintain consistency with the documented pattern already used for similarAddMNcalls in this file.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/evo/cbtx.cppsrc/evo/specialtxman.cppsrc/llmq/blockprocessor.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsession.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.cppsrc/llmq/snapshot.hsrc/llmq/utils.cppsrc/llmq/utils.hsrc/rpc/quorums.cppsrc/test/llmq_snapshot_tests.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/evo/cbtx.cpp
- src/evo/specialtxman.cpp
🧰 Additional context used
📓 Path-based instructions (6)
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.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/rpc/quorums.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/test/llmq_snapshot_tests.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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/llmq_snapshot_tests.cpp
🧠 Learnings (21)
📓 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: 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: 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: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
📚 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/dkgsessionhandler.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/rpc/quorums.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/test/llmq_snapshot_tests.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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/dkgsessionhandler.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/rpc/quorums.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/llmq/dkgsessionhandler.cppsrc/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/rpc/quorums.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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/dkgsessionhandler.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/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/llmq/snapshot.cppsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/llmq/snapshot.cppsrc/llmq/dkgsession.cppsrc/llmq/commitment.cppsrc/llmq/commitment.hsrc/llmq/debug.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/snapshot.hsrc/llmq/utils.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/snapshot.cppsrc/llmq/commitment.cppsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/utils.cpp
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/llmq/commitment.hsrc/llmq/utils.hsrc/llmq/snapshot.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/debug.cppsrc/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/utils.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/llmq/blockprocessor.cpp
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/llmq/blockprocessor.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/llmq/blockprocessor.cppsrc/llmq/utils.hsrc/llmq/utils.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.hsrc/llmq/utils.cpp
📚 Learning: 2025-11-13T20:02:55.480Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6969
File: src/evo/deterministicmns.h:441-479
Timestamp: 2025-11-13T20:02:55.480Z
Learning: In `src/evo/deterministicmns.h`, the `internalId` field in `CDeterministicMN` and the `mnInternalIdMap` in `CDeterministicMNList` are non-deterministic and used only for internal bookkeeping and efficient lookups. Different nodes can assign different internalIds to the same masternode depending on their sync history. Methods like `IsEqual()` intentionally ignore internalId mappings and only compare consensus-critical deterministic fields (proTxHash, collateral, state, etc.).
Applied to files:
src/llmq/utils.hsrc/llmq/utils.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/utils.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
src/llmq/utils.hsrc/llmq/utils.cpp
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Applied to files:
src/llmq/quorums.cppsrc/llmq/utils.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay
Applied to files:
src/llmq/utils.cpp
📚 Learning: 2025-11-04T18:23:28.175Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/governance/classes.cpp:147-154
Timestamp: 2025-11-04T18:23:28.175Z
Learning: In src/governance/classes.cpp, CSuperblock::GetPaymentsLimit intentionally uses synthetic difficulty constants (nBits = 1 for mainnet, powLimit for networks with min difficulty) and simple height-based V20 activation checks instead of actual chain block data. This is by design because superblocks themselves are "synthetic" governance payment blocks, not regular mined blocks.
Applied to files:
src/llmq/utils.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/llmq/utils.cpp
🧬 Code graph analysis (13)
src/llmq/snapshot.cpp (1)
src/llmq/snapshot.h (2)
WORK_DIFF_DEPTH(39-39)CQuorumRotationInfo(114-145)
src/llmq/dkgsession.cpp (1)
src/llmq/utils.cpp (4)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)GetQuorumRelayMembers(702-755)GetQuorumRelayMembers(702-703)
src/llmq/commitment.cpp (4)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)src/llmq/options.cpp (2)
IsQuorumRotationEnabled(45-57)IsQuorumRotationEnabled(45-45)src/llmq/commitment.h (1)
quorumIndex(56-56)src/llmq/dkgsession.h (1)
quorumIndex(297-297)
src/rpc/quorums.cpp (1)
src/llmq/utils.cpp (2)
GetQuorumConnections(678-700)GetQuorumConnections(678-679)
src/llmq/commitment.h (1)
src/llmq/utils.h (3)
utils(47-95)llmq(29-31)llmq(33-96)
src/llmq/debug.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/dkgsessionmgr.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/llmq/blockprocessor.cpp (1)
src/llmq/utils.cpp (2)
GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)
src/test/llmq_snapshot_tests.cpp (1)
src/llmq/snapshot.cpp (3)
CQuorumSnapshot(230-230)CQuorumSnapshot(232-238)CQuorumSnapshot(240-240)
src/llmq/utils.h (2)
src/llmq/utils.cpp (17)
BlsCheck(528-528)BlsCheck(530-536)BlsCheck(538-538)DeterministicOutboundConnection(656-676)DeterministicOutboundConnection(656-656)CalcDeterministicWatchConnections(757-776)CalcDeterministicWatchConnections(757-758)GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)GetQuorumConnections(678-700)GetQuorumConnections(678-679)GetQuorumRelayMembers(702-755)GetQuorumRelayMembers(702-703)EnsureQuorumConnections(778-829)EnsureQuorumConnections(778-780)AddQuorumProbeConnections(831-874)AddQuorumProbeConnections(831-833)src/llmq/params.h (1)
LLMQType(14-125)
src/llmq/quorums.cpp (2)
src/llmq/utils.cpp (4)
EnsureQuorumConnections(778-829)EnsureQuorumConnections(778-780)GetAllQuorumMembers(568-654)GetAllQuorumMembers(568-568)src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/llmq/snapshot.h (1)
src/llmq/snapshot.cpp (7)
CQuorumSnapshot(230-230)CQuorumSnapshot(232-238)CQuorumSnapshot(240-240)CQuorumRotationInfo(242-242)CQuorumRotationInfo(244-244)GetCycles(246-254)GetCycles(246-246)
src/llmq/utils.cpp (3)
src/llmq/snapshot.h (1)
WORK_DIFF_DEPTH(39-39)src/llmq/options.cpp (2)
IsQuorumRotationEnabled(45-57)IsQuorumRotationEnabled(45-45)src/llmq/dkgsession.h (1)
quorumIndex(297-297)
🪛 Cppcheck (2.19.0)
src/llmq/snapshot.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🪛 GitHub Actions: Clang Diff Format Check
src/llmq/dkgsessionhandler.cpp
[error] 1-1: Clang-format differences detected in src/llmq/dkgsessionhandler.cpp. Run clang-format to fix formatting.
src/llmq/snapshot.cpp
[error] 1-1: Clang-format differences detected in src/llmq/snapshot.cpp. Run clang-format to fix formatting.
src/llmq/dkgsession.cpp
[error] 1-1: Clang-format differences detected in src/llmq/dkgsession.cpp. Run clang-format to fix formatting.
src/test/llmq_snapshot_tests.cpp
[error] 1-1: Clang-format differences detected in src/test/llmq_snapshot_tests.cpp. Run clang-format to fix formatting.
src/llmq/quorums.cpp
[error] 1-1: Clang-format differences detected in src/llmq/quorums.cpp. Run clang-format to fix formatting.
src/llmq/snapshot.h
[error] 1-1: Clang-format differences detected in src/llmq/snapshot.h. Run clang-format to fix formatting.
src/llmq/utils.cpp
[error] 1-1: Clang-format differences detected in src/llmq/utils.cpp. Run clang-format to fix formatting.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (42)
src/llmq/debug.cpp (1)
27-33: GetAllQuorumMembers UtilParameters migration looks correctThe aggregate
{dmnman, qsnapman, chainman, pindex}matchesUtilParameterslayout and preserves the prior behavior of the detail‑level 2 path.src/llmq/dkgsessionmgr.cpp (2)
296-303: UtilParameters use in GetVerifiedContributions is consistent
utils::GetAllQuorumMembers(llmqType, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex})matches the new UtilParameters layout, andpQuorumBaseBlockIndexhas already been validated, so the refactor is mechanically safe.
345-351: UtilParameters use in GetEncryptedContributions is consistentSame as above: the aggregated
{m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}correctly feeds the newGetAllQuorumMembersAPI without altering the surrounding logic.src/rpc/quorums.cpp (1)
372-387: Quorum connection helper migrated cleanly to UtilParametersBoth
GetQuorumConnectionscalls now supply{*node.dmnman, *llmq_ctx.qsnapman, chainman, pQuorumBaseBlockIndex}, which matches the new UtilParameters contract; prior null‑checks onnode.dmnmanandnode.sporkmankeep the assumptions intact.src/llmq/commitment.h (2)
34-40: Forward declaration of UtilParameters is appropriateDeclaring
struct UtilParameters;withinnamespace llmqis sufficient for the new reference parameters and avoids pulling utils.h into this header.
79-82: CFinalCommitment verification API now cleanly depends on UtilParametersSwitching
VerifySignatureAsyncandVerifyto acceptconst llmq::UtilParameters&removes the ad‑hoc manager/index parameter list and aligns with the utilities layer; the header changes are consistent with the implementations in commitment.cpp.src/llmq/blockprocessor.cpp (5)
30-38: PreComputeQuorumMembers now primes the shared quorum-members cacheThe new
GetAllQuorumMembers(params.type, {dmnman, qsnapman, chainman, pindex}, reset_cache)call correctly constructs UtilParameters and simply warms the common cache for rotation-enabled quorums; no behavioral change beyond sharing cache state.
148-155: CFinalCommitment::Verify now cleanly fed with a UtilParameters contextThe constructed context
{m_dmnman, m_qsnapman, m_chainstate.m_chainman, pQuorumBaseBlockIndex}matches the newCFinalCommitment::VerifyAPI and preserves the previous validation flow for incoming QFCOMMITMENT messages.
208-219: Asynchronous BLS checks correctly use UtilParametersWithin the BLS check loop,
VerifySignatureAsync({m_dmnman, m_qsnapman, m_chainstate.m_chainman, pQuorumBaseBlockIndex}, &queue_control)passes the same quorum base index that was just looked up; this is consistent with the commitment-level Verify logic and centralizes member discovery on the shared cache.
332-347: ProcessCommitment’s non-signature validation correctly reuses UtilParametersThe call
qc.Verify({m_dmnman, m_qsnapman, m_chainstate.m_chainman, pQuorumBaseBlockIndex}, /*checksigs=*/false)aligns with the new Verify implementation and avoids re-specifying individual managers/indexes; behavior remains unchanged.
383-389: UndoBlock cache priming refactor is consistentReusing
PreComputeQuorumMembers(m_dmnman, m_qsnapman, m_chainstate.m_chainman, pindex, /*reset_cache=*/true)here mirrors the ProcessBlock path and ensures the shared GetAllQuorumMembers cache stays coherent across reorgs.src/llmq/commitment.cpp (3)
31-94: CFinalCommitment::VerifySignatureAsync correctly migrated to UtilParameters
VerifySignatureAsyncnow usesutils::GetAllQuorumMembers(llmqType, util_params)and derivesllmq_paramsandcommitmentHashexactly as before, while reusing the shared cache and allowing optional batched BLS verification viaqueue_control; the refactor is mechanically sound.
97-185: CFinalCommitment::Verify now consistently derives all context from UtilParametersUsing
util_params.m_base_indexfor rotation, version, quorumHash, and quorumIndex checks, andutil_params.m_chainman.GetConsensus()for deployment height is consistent with how callers construct the context (passing the quorum base block), andGetAllQuorumMembers(llmqType, util_params)centralizes member list retrieval. This keeps the validation logic unchanged while simplifying the API.Please ensure that all call sites invoking
VerifyandVerifySignatureAsyncstill only pass known, configured LLMQ types; otherwiseGetAllQuorumMemberswill assert on missing params instead of returning a clean failure.
216-283: CheckLLMQCommitment’s use of UtilParameters matches the new verification flow
CheckLLMQCommitmentnow:
- Logs using
util_params.m_base_index->nHeight,- Validates
qcTx.nHeightagainstbase_index->nHeight + 1,- Ensures
quorumHashis on the active chain viautil_params.m_chainman,- And finally calls
qcTx.commitment.Verify(util_params.replace_index(pQuorumBaseBlockIndex), false).All of this is consistent with previous behavior while sharing the common UtilParameters context.
src/llmq/dkgsession.cpp (2)
93-157: CDKGSession::Init now sources quorum members/relays via UtilParametersBoth
GetAllQuorumMembersandGetQuorumRelayMembersuse the aggregate{m_dmnman, m_qsnapman, m_chainman, m_quorum_base_block_index}, which matches the UtilParameters definition and keeps the DKG member/relay selection logic unchanged.
1192-1299: FinalizeCommitments and FinalizeSingleCommitment now verify using UtilParametersThe calls to
fqc.Verify({m_dmnman, m_qsnapman, m_chainman, m_quorum_base_block_index}, true)consistently reuse the same quorum base block index that the session was constructed with, aligning with the new CFinalCommitment verification API and avoiding duplicated context wiring.src/llmq/quorums.cpp (4)
229-229: LGTM - Updated to use new InitQuorumsCache signature.The cache initialization now explicitly takes
m_chainman.GetConsensus()instead of relying on globalParams(). This aligns with the broader refactoring to pass consensus parameters explicitly.
414-414: Consistent API update for GetAllQuorumMembers.The call correctly uses the new
UtilParametersaggregate, matching the pattern established throughout this refactor.
1117-1117: Pipeline failure: clang-format differences detected.The CI indicates formatting issues in this file. Run
clang-formatto resolve.
371-372: Parameter struct construction is correct.The
UtilParametersaggregate initialization{m_dmnman, m_qsnapman, m_chainman, quorum->m_quorum_base_block_index}correctly matches the struct member order defined insrc/llmq/utils.h: m_dmnman, m_qsnapman, m_chainman, and m_base_index. The value for m_base_index is properly adapted from the quorum's block index.src/llmq/snapshot.cpp (5)
17-46: Well-structured helper for cycle data construction.The
ConstructCyclehelper properly:
- Uses
std::optionalfor fallible construction- Populates error messages for debugging
- Handles the snapshot skip flag correctly
One minor observation: The static analysis hint about
always_inlineis a false positive from Cppcheck's preprocessor analysis, not related to this code.
111-116: Cycle-based block derivation is cleaner.The new approach using
ConstructCyclewith calculated height simplifies the previous complex block-hunting logic. The error message "Cannot find block" is generic but sufficient for debugging.
129-146: Iterate over cycles via GetCycles() accessor.This loop-based approach using
target_cyclesvector is more maintainable than the previous explicit field access pattern. The assignment*target_cycles[idx] = cycle_opt.value()correctly populates the CycleData members.
162-164: Correctly removes already-processed cycle heights from needed set.The loop erases cycle heights that are already handled by the main cycles, preventing duplicate snapshot fetching.
246-254: GetCycles() correctly handles optional cycleHMinus4C.The method lazily initializes
cycleHMinus4CwhenextraShareis true and it hasn't been set. This ensures the returned vector has consistent size based on theextraShareflag.src/test/llmq_snapshot_tests.cpp (3)
74-79: Good use of constexpr array for testing all skip modes.Using
constexpr std::array<SnapshotSkipMode, 4>is cleaner than the previousstd::vector<int>approach and provides compile-time verification of enum coverage.
208-224: Tests correctly verify CycleData wrapper access patterns.The test properly exercises:
- Direct access via
cycleHMinusC.m_snap,cycleHMinus2C.m_snap, etc.- Optional
cycleHMinus4Cinitialization and assignment- Serialization roundtrip with the new structure
170-170: Verifies optional field default state.
BOOST_CHECK(!rotInfo.cycleHMinus4C.has_value())correctly validates that the optional is empty by default whenextraShareis false.src/llmq/snapshot.h (5)
30-36: Well-designed enum class with serialization support.
SnapshotSkipModeas a scoped enum with explicitintunderlying type ensures type safety while maintaining wire compatibility. Theis_serializable_enumspecialization enables direct serialization.
104-112: Clean composition via CycleBase and CycleData.The inheritance structure is appropriate:
CycleBasecontains snapshot and cycle index (common to all cycles)CycleDataextends with diff and work index (needed for rotation info)This allows future reuse of
CycleBaseif needed.
117-131: Refactored CQuorumRotationInfo with clear structure.The transition from flat fields (
quorumSnapshotAtHMinusC,mnListDiffAtHMinusC, etc.) to composedCycleDatamembers improves maintainability. The deleted copy constructor is appropriate given the potentially large data members.
153-156: Serialization maintains backward compatibility.Using
value_or(CycleData{})for the optional field ensures that whenextraShareis true butcycleHMinus4Cis unset, a default-constructedCycleDatais serialized. This maintains the existing on-disk format as noted in the comment.
208-208: GetCycles() provides unified access to cycle data.Returning
std::vector<CycleData*>allows iteration over all cycles (3 or 4 depending onextraShare) without duplicating logic at each call site.src/llmq/utils.h (3)
34-45: UtilParameters provides clean context aggregation.The struct consolidates the commonly-passed tuple of managers and chain state. The
replace_index()helper is particularly useful for quorum rotation logic where the same managers are used with different block indices.Note: Members are references, so the caller must ensure the referenced objects outlive the
UtilParametersinstance.
87-94: InitQuorumsCache now inline with explicit consensus params.Moving the template implementation inline is appropriate since it's simple. Taking
consensus_paramsexplicitly removes the dependency on globalParams(), aligning with the PR's goal of reducing global state usage.
48-61: BlsCheck provides a clean API for BLS signature verification in parallel queue context.The struct effectively:
- Implements callable interface via
operator()for use in check queue algorithms- Includes
swap()for efficient exchange- Encapsulates all BLS verification data (signature, public keys, message hash, identification)
- Properly uses BLS cryptographic integration per coding guidelines for LLMQ implementations
- Integrates seamlessly with CCheckQueue for distributed signature verification in LLMQ commitments
Code changes are well-designed.
src/llmq/utils.cpp (6)
33-71: Well-organized anonymous namespace for internal types.The helper types (
QuorumMembers,MasternodeScore,QuorumQuarter,PreviousQuorumQuarters) are appropriately scoped to file-local visibility. TheQuorumQuarterandPreviousQuorumQuartersstructures mirror theCycleData/CycleBasepattern fromsnapshot.h, providing symmetry in the codebase.
88-111: GetHashModifier now takes explicit consensus_params.This change removes reliance on global
Params()for consensus parameters, improving testability and making dependencies explicit. The logic remains unchanged, maintaining consensus compatibility.
162-169: Structured binding improves readability in sorting.The comparator using
a.m_scoreanda.m_nodeis cleaner than the previous pair-based approach. The fallback comparison oncollateralOutpointmaintains the existing tie-breaking behavior.
540-558: BlsCheck::operator() handles all pubkey count cases.The implementation correctly:
- Uses
VerifySecureAggregatedfor multiple pubkeys (aggregated signature)- Uses
VerifyInsecurefor single pubkey (standard verification)- Returns false with logging for empty pubkeys (defensive check)
This aligns with the BLS integration requirements for masternode features.
568-654: GetAllQuorumMembers refactored with UtilParameters.The function maintains its caching behavior with static maps while now accepting the consolidated
UtilParameterscontext. Thereplace_index()usage on line 631 demonstrates the utility of that helper method.
473-491: Cycle iteration uses consistent pattern.The loop over
prev_cyclesfollows the same pattern established insnapshot.cpp, iterating viaGetCycles()and populating members from snapshots. The break on missing snapshot (line 484) is appropriate for graceful degradation.
4 participants
Additional Information
CActiveMasternodeManagerfromLLMQContext(1/n, preliminary refactoring) #7056Breaking Changes
None expected.
Checklist