refactor: extract CActiveMasternodeManager from LLMQContext (1/n, preliminary refactoring)
#7056
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe patch introduces llmq::ObserverContext for watch/observer-mode LLMQ handling alongside the existing ActiveContext for masternodes. Signer lifecycle registration APIs were renamed (Start/Stop → RegisterRecoveryInterface/UnregisterRecoveryInterface) and signer Start/Stop calls removed from several Start/Stop paths. Many constructors and factory signatures were extended to accept quorum-related parameters (llmq::QvvecSyncModeMap, quorums_recovery, quorums_watch, bls_threads, max_recsigs_age). CQuorumManager can now connect/disconnect a CDKGSessionManager, and PeerManager is updated to accept and route messages to an optional observer_ctx. Sequence Diagram(s)sequenceDiagram
participant Init as init.cpp
participant NodeCtx as NodeContext
participant LLMQ as LLMQContext
participant QMgr as CQuorumManager
participant Active as ActiveContext
participant Obs as ObserverContext
participant Peer as PeerManager
Note over Init: Parse args (sync_map, quorums_recovery, quorums_watch, bls_threads, max_recsigs_age)
Init->>LLMQ: Create LLMQContext(sync_map, quorums_recovery, quorums_watch, bls_threads, max_recsigs_age)
LLMQ->>QMgr: Construct CQuorumManager(sync_map, quorums_recovery, quorums_watch)
alt Masternode enabled
Init->>Active: Create ActiveContext(db_params, quorums_watch,...)
Active->>Active: Create qdkgsman (CDKGSessionManager)
Active->>QMgr: QMgr.ConnectManager(qdkgsman)
Init->>Peer: Create PeerManager(..., active_ctx, observer_ctx=null)
Peer->>Peer: Route QUORUM messages -> active_ctx.qdkgsman
else Observer/watch mode
Init->>Obs: Create ObserverContext(db_params, quorums_watch=true,...)
Obs->>Obs: Create qdkgsman (CDKGSessionManager)
Obs->>QMgr: QMgr.ConnectManager(qdkgsman)
Init->>Peer: Create PeerManager(..., active_ctx=null, observer_ctx)
Peer->>Peer: Route QUORUM messages -> observer_ctx.qdkgsman
end
Note over Peer, QMgr: UpdatedBlockTip events forwarded to appropriate qdkgsman
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (45)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (11)
🧰 Additional context used📓 Path-based instructions (9)src/**/*.{cpp,h,hpp,cc}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/{masternode,llmq}/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/llmq/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/{masternode,evo}/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/{test,wallet/test}/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/node/**/*.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/node/chainstate.{cpp,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (51)📓 Common learnings📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-12-22T15:42:48.595ZApplied to files:
📚 Learning: 2025-07-29T14:32:48.369ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-10-02T18:29:54.756ZApplied to files:
📚 Learning: 2025-12-01T18:13:21.314ZApplied to files:
📚 Learning: 2025-08-11T17:16:36.654ZApplied to files:
📚 Learning: 2025-10-13T12:37:12.357ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-07-15T14:53:04.819ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-08-19T14:57:31.801ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-10-21T11:09:34.688ZApplied to files:
📚 Learning: 2025-05-10T00:54:30.887ZApplied to files:
📚 Learning: 2025-10-03T11:20:37.475ZApplied to files:
📚 Learning: 2025-10-02T18:29:54.756ZApplied to files:
📚 Learning: 2024-12-29T17:43:41.755ZApplied to files:
📚 Learning: 2025-01-02T21:50:00.967ZApplied to files:
📚 Learning: 2025-06-09T16:43:20.996ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-25T10:53:37.523ZApplied to files:
📚 Learning: 2025-11-25T10:53:37.523ZApplied to files:
📚 Learning: 2025-11-13T20:02:55.480ZApplied to files:
📚 Learning: 2025-08-08T07:01:47.332ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-02-06T14:34:30.466ZApplied to files:
📚 Learning: 2025-11-04T18:24:27.241ZApplied to files:
📚 Learning: 2025-08-08T04:30:37.971ZApplied to files:
📚 Learning: 2025-12-02T12:59:28.247ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-08-19T15:08:00.835ZApplied to files:
📚 Learning: 2025-10-05T20:38:28.457ZApplied to files:
📚 Learning: 2025-11-24T16:41:22.457ZApplied to files:
📚 Learning: 2025-10-25T07:08:51.918ZApplied to files:
📚 Learning: 2025-07-29T14:33:01.040ZApplied to files:
📚 Learning: 2025-12-17T13:58:26.891ZApplied to files:
📚 Learning: 2025-07-17T15:48:29.418ZApplied to files:
📚 Learning: 2025-10-28T08:54:00.392ZApplied to files:
📚 Learning: 2025-07-22T14:38:45.606ZApplied to files:
📚 Learning: 2025-02-14T15:15:58.165ZApplied to files:
📚 Learning: 2025-07-09T15:05:36.250ZApplied to files:
🧬 Code graph analysis (21)src/chainlock/signing.cpp (2)
src/llmq/signing.cpp (1)
src/masternode/active/notificationinterface.cpp (1)
src/instantsend/signing.cpp (2)
src/instantsend/instantsend.h (1)
src/masternode/active/context.cpp (2)
src/test/net_peer_connection_tests.cpp (1)
src/node/chainstate.h (4)
src/masternode/active/context.h (5)
src/test/util/setup_common.cpp (2)
src/net_processing.h (3)
src/llmq/dkgsessionmgr.cpp (1)
src/llmq/signing.h (1)
src/llmq/observer/context.h (3)
src/test/util/setup_common.h (1)
src/llmq/quorums.cpp (1)
src/init.cpp (3)
src/llmq/context.h (1)
src/llmq/context.cpp (3)
src/llmq/quorums.h (3)
src/llmq/observer/context.cpp (4)
🪛 Cppcheck (2.19.0)src/masternode/active/context.cpp[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation (syntaxError) [error] 24-24: #error No known always_inline attribute for this platform. (preprocessorErrorDirective) src/test/validation_chainstatemanager_tests.cpp[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation (syntaxError) [error] 24-24: #error No known always_inline attribute for this platform. (preprocessorErrorDirective) src/llmq/context.cpp[error] 24-24: #error No known always_inline attribute for this platform. (preprocessorErrorDirective) src/llmq/observer/context.cpp[error] 10-10: failed to evaluate #if condition, undefined function-like macro invocation (syntaxError) 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/llmq/context.cpp (1)
1-54: Fix formatting to pass CI.The pipeline reports clang-format differences in this file. Please run
clang-format-diff.pyorclang-formatto fix the formatting issues before merging.#!/bin/bash # Check for clang-format differences git diff HEAD~1 --name-only | xargs -I{} clang-format --dry-run -Werror {} 2>&1 | head -50
🧹 Nitpick comments (6)
src/llmq/blockprocessor.h (1)
61-62: Consider wideningbls_threadsparameter type
bls_threadsis declared asint8_t, which will silently truncate largerintconfiguration values and is easy to accidentally pass a negative value into. Since this is just a thread-count knob, usingintorunsignedwould avoid surprising narrowing with no downside.- explicit CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb, - CQuorumSnapshotManager& qsnapman, int8_t bls_threads); + explicit CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb, + CQuorumSnapshotManager& qsnapman, int bls_threads);src/llmq/blockprocessor.cpp (1)
48-58: Ctor correctly delegates BLS worker thread count to callersInitializing the mined-commitment cache and then starting
m_bls_queueworker threads with the suppliedbls_threadsvalue is straightforward and keeps threading policy outside this component. Combined with the header change, just ensure call sites validate or boundbls_threads(e.g., non‑negative, reasonable upper limit), especially if it comes from user-configurable args.src/llmq/options.h (1)
14-20: ArgsManager-based Qvvec sync API and alias are a nice cleanupSwitching
GetEnabledQuorumVvecSyncEntriesto takeconst ArgsManager&and introducing theQvvecSyncModeMapalias improves clarity and removes dependence on global args; header and options.cpp stay consistent.As a small follow-up, since
LLMQTypeis now forward-declared with an explicituint8_tunderlying type here, consider including<cstdint>in this header so theuint8_tdefinition doesn’t rely on transitive includes.Also applies to: 22-47
src/llmq/observer/context.h (1)
31-32: Consider documenting whym_qmanis stored as a reference.The private
m_qmanreference is presumably used forConnectManager/DisconnectManagercalls in constructor/destructor. A brief comment would clarify this coupling for future maintainers.src/llmq/quorums.cpp (1)
278-285: Optional: avoid double lookup inm_sync_mapwhen computingsyncMode.In
TriggerQuorumDataRecoveryThreadsyou currently do:const bool fSyncForTypeEnabled = m_sync_map.count(pQuorum->qc->llmqType) > 0; const QvvecSyncMode syncMode = fSyncForTypeEnabled ? m_sync_map.at(pQuorum->qc->llmqType) : QvvecSyncMode::Invalid;You can avoid the second tree lookup and clarify intent by using
find:- const bool fSyncForTypeEnabled = m_sync_map.count(pQuorum->qc->llmqType) > 0; - const QvvecSyncMode syncMode = fSyncForTypeEnabled ? m_sync_map.at(pQuorum->qc->llmqType) - : QvvecSyncMode::Invalid; + const auto it = m_sync_map.find(pQuorum->qc->llmqType); + const bool fSyncForTypeEnabled = it != m_sync_map.end(); + const QvvecSyncMode syncMode = fSyncForTypeEnabled ? it->second : QvvecSyncMode::Invalid;Not critical given the small map size, but a low-cost micro‑cleanup.
src/llmq/quorums.h (1)
244-248: Clarify and enforce lifetime & invariants aroundm_qdkgsman.With
m_qdkgsmannow being an atomic raw pointer, correctness hinges on a couple of implicit contracts:
ConnectManager()is only ever called whileCDKGSessionManageris fully constructed and before any threads that might callBuildQuorumContributions()/ProcessMessage()start usingCQuorumManager.DisconnectManager()is called beforeCDKGSessionManageris destroyed, and after all worker/network threads that might loadm_qdkgsmanhave been quiesced, to avoid a potential use‑after‑free between the load and dereference.- Whenever
m_qdkgsmanis non‑null,m_mn_activemanmust also be non‑null, sinceBuildQuorumContributions()unconditionally dereferences*m_mn_activemanafter a successfulGetVerifiedContributions().The acquire/release memory orders on the atomic are fine for visibility, but they don’t by themselves guarantee these lifetime invariants.
If they aren’t already enforced by higher‑level sequencing in
LLMQContext/ActiveContext, consider:
- Documenting these assumptions explicitly in comments near
ConnectManager/DisconnectManager, and/or- Tightening the shutdown sequence so that
DisconnectManager()is paired with stopping the worker pool and any call paths that might still reachm_qdkgsman.This keeps the refactor safe while still allowing the optional manager to be absent in observer mode.
Also applies to: 275-280, 282-289, 330-341, 449-459, 802-808
|
This pull request has conflicts, please rebase. |
kwvg
force-pushed
the
active_p1
branch
2 times, most recently
from
bb27019 to
e92062c
Compare
5 tasks
|
This pull request has conflicts, please rebase. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/init.cpp (2)
571-577: LLMQ CLI options and parameter interaction are sensible; double parsing is a minor nitThe new options
-maxrecsigsage,-parbls,-watchquorums,-llmq-data-recovery, and-llmq-qvvec-sync, plus theAppInitParameterInteractionblock that validates-llmq-qvvec-syncand warns when recovery is disabled, look correct and defensive. Note thatGetEnabledQuorumVvecSyncEntries(args)is now called here and again later inAppInitMain; if this begins to grow more expensive, you might consider caching the parsed map once and reusing it, but that’s purely a micro-optimization.Also applies to: 770-787, 1414-1421
1961-1963: BLS verification thread count calculation is correct but could avoid early narrowing to int8_tThe
-parblshandling (auto/“leave N cores free” semantics plus clamping toMAX_BLSCHECK_THREADS) is consistent with the help text and mirrors-par. However, you currently read the arg directly into anint8_t, which can silently wrap if someone passes a value outside the documented range before you clamp. Consider computing in anintand casting only once after clamping, e.g.:- /*bls_threads=*/[&args]() -> int8_t { - int8_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); + /*bls_threads=*/[&args]() -> int8_t { + int threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); if (threads <= 0) { // -parbls=0 means autodetect (number of cores - 1 validator threads) // -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads) threads += GetNumCores(); } - // Subtract 1 because the main thread counts towards the par threads - return std::clamp<int8_t>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS); + // Subtract 1 because the main thread counts towards the par threads + threads = std::clamp(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS); + return static_cast<int8_t>(threads); }(),This keeps behaviour the same while avoiding any potential narrowing surprises.
Also applies to: 2007-2015, 2031-2052
src/llmq/quorums.cpp (1)
529-543: Consider guarding ScanQuorums(llmqType, size_t) against a null tip before forwarding to not_null overloadThe first
ScanQuorums(LLMQType, size_t)overload now forwardsm_chainstate.m_chain.Tip()directly into thegsl::not_nulloverload. In practice this probably only runs once there is a non‑null tip, but if it were ever called earlier (e.g. very early startup or in tests), constructinggsl::not_nullfrom a null pointer would violate its contract. You could defensively add a simple guard here:std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t nCountRequested) const { - const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip()); - return ScanQuorums(llmqType, pindex, nCountRequested); + const CBlockIndex* pindex = WITH_LOCK(::cs_main, return m_chainstate.m_chain.Tip()); + if (pindex == nullptr) { + return {}; + } + return ScanQuorums(llmqType, pindex, nCountRequested); }This keeps existing behaviour for non‑empty chains while making the non‑null invariant explicit.
src/net_processing.cpp (1)
2353-2369: ObserverContext DKG lookups are correctly prioritized; duplication could be factored later
AlreadyHave()andProcessGetData()now consultm_observer_ctx->qdkgsmanfirst and fall back tom_active_ctx->qdkgsman, which matches the intended “observer node vs active masternode” separation without changing lookup semantics for active nodes.- DSQ handling similarly checks both
m_cj_walletmanandm_active_ctx->cj_server, preserving existing behavior while allowing wallet‑only nodes to participate.You repeat the
(m_observer_ctx && ctx->qdkgsman->... ) || (m_active_ctx && ctx->qdkgsman->...)pattern in multiple places; if this grows further, consider a small helper (e.g.ForEachQDKGSManager(...)) to centralize that branching, but this is purely optional and not required for this PR.Also applies to: 2929-2963
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/Makefile.am(2 hunks)src/chainlock/chainlock.cpp(0 hunks)src/chainlock/signing.cpp(1 hunks)src/chainlock/signing.h(1 hunks)src/dsnotificationinterface.cpp(0 hunks)src/evo/mnhftx.cpp(4 hunks)src/evo/mnhftx.h(3 hunks)src/init.cpp(9 hunks)src/instantsend/instantsend.h(1 hunks)src/instantsend/net_instantsend.cpp(0 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(1 hunks)src/llmq/blockprocessor.cpp(1 hunks)src/llmq/blockprocessor.h(1 hunks)src/llmq/context.cpp(2 hunks)src/llmq/context.h(2 hunks)src/llmq/dkgsessionhandler.cpp(3 hunks)src/llmq/dkgsessionhandler.h(2 hunks)src/llmq/dkgsessionmgr.cpp(2 hunks)src/llmq/dkgsessionmgr.h(2 hunks)src/llmq/observer/context.cpp(1 hunks)src/llmq/observer/context.h(1 hunks)src/llmq/options.cpp(1 hunks)src/llmq/options.h(1 hunks)src/llmq/quorums.cpp(14 hunks)src/llmq/quorums.h(5 hunks)src/llmq/signing.cpp(2 hunks)src/llmq/signing.h(2 hunks)src/llmq/utils.cpp(2 hunks)src/llmq/utils.h(1 hunks)src/masternode/active/context.cpp(3 hunks)src/masternode/active/context.h(3 hunks)src/masternode/active/notificationinterface.cpp(2 hunks)src/net_processing.cpp(8 hunks)src/net_processing.h(2 hunks)src/node/chainstate.cpp(5 hunks)src/node/chainstate.h(3 hunks)src/node/context.cpp(1 hunks)src/node/context.h(2 hunks)src/test/denialofservice_tests.cpp(4 hunks)src/test/net_peer_connection_tests.cpp(1 hunks)src/test/util/setup_common.cpp(7 hunks)src/test/util/setup_common.h(1 hunks)src/test/validation_chainstatemanager_tests.cpp(1 hunks)test/util/data/non-backported.txt(1 hunks)
💤 Files with no reviewable changes (3)
- src/chainlock/chainlock.cpp
- src/dsnotificationinterface.cpp
- src/instantsend/net_instantsend.cpp
🚧 Files skipped from review as they are similar to previous changes (14)
- src/node/context.cpp
- src/llmq/blockprocessor.h
- src/chainlock/signing.h
- src/instantsend/signing.h
- src/masternode/active/notificationinterface.cpp
- src/instantsend/signing.cpp
- src/llmq/dkgsessionhandler.h
- src/masternode/active/context.h
- src/llmq/observer/context.cpp
- src/llmq/dkgsessionmgr.h
- src/test/util/setup_common.cpp
- src/llmq/signing.h
- src/llmq/utils.cpp
- src/llmq/blockprocessor.cpp
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/instantsend/instantsend.hsrc/llmq/options.hsrc/llmq/options.cppsrc/test/net_peer_connection_tests.cppsrc/chainlock/signing.cppsrc/test/util/setup_common.hsrc/evo/mnhftx.hsrc/test/denialofservice_tests.cppsrc/llmq/signing.cppsrc/net_processing.hsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/evo/mnhftx.cppsrc/llmq/context.hsrc/node/chainstate.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.hsrc/test/validation_chainstatemanager_tests.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/options.hsrc/llmq/options.cppsrc/evo/mnhftx.hsrc/llmq/signing.cppsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/evo/mnhftx.cppsrc/llmq/context.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cpp
src/{masternode,llmq}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
BLS integration must be used for cryptographic foundation of advanced masternode features
Files:
src/llmq/options.hsrc/llmq/options.cppsrc/llmq/signing.cppsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cpp
src/llmq/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Files:
src/llmq/options.hsrc/llmq/options.cppsrc/llmq/signing.cppsrc/llmq/observer/context.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cpp
src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Files:
src/llmq/options.hsrc/llmq/options.cppsrc/evo/mnhftx.hsrc/llmq/signing.cppsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/evo/mnhftx.cppsrc/llmq/context.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cpp
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/net_peer_connection_tests.cppsrc/test/util/setup_common.hsrc/test/denialofservice_tests.cppsrc/test/validation_chainstatemanager_tests.cpp
src/{masternode,evo}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/mnhftx.hsrc/masternode/active/context.cppsrc/evo/mnhftx.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/mnhftx.hsrc/evo/mnhftx.cpp
src/node/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Files:
src/node/chainstate.hsrc/node/chainstate.cppsrc/node/context.h
src/node/chainstate.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Files:
src/node/chainstate.hsrc/node/chainstate.cpp
🧠 Learnings (33)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:15-250
Timestamp: 2025-07-23T09:28:32.783Z
Learning: In refactoring PRs like #6761, kwvg prefers to defer code formatting fixes to separate follow-up PRs when formatting is not the primary objective, to maintain focus on the structural changes and avoid scope creep.
Learnt from: 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: 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: 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} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/Makefile.amsrc/instantsend/instantsend.hsrc/llmq/options.hsrc/llmq/options.cppsrc/net_processing.htest/util/data/non-backported.txtsrc/masternode/active/context.cppsrc/llmq/context.hsrc/node/chainstate.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.hsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/Makefile.amsrc/llmq/options.hsrc/llmq/options.cppsrc/llmq/signing.cppsrc/net_processing.htest/util/data/non-backported.txtsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/node/chainstate.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.hsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/Makefile.amsrc/llmq/options.htest/util/data/non-backported.txtsrc/llmq/context.hsrc/node/chainstate.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/utils.hsrc/init.cppsrc/llmq/quorums.hsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: 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/Makefile.amsrc/llmq/options.hsrc/evo/mnhftx.hsrc/net_processing.hsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/evo/mnhftx.cppsrc/llmq/context.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/dkgsessionmgr.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/Makefile.amsrc/llmq/options.hsrc/evo/mnhftx.hsrc/net_processing.htest/util/data/non-backported.txtsrc/evo/mnhftx.cppsrc/llmq/context.hsrc/node/chainstate.hsrc/node/chainstate.cppsrc/net_processing.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.hsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/Makefile.amtest/util/data/non-backported.txtsrc/net_processing.cppsrc/init.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/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/Makefile.amsrc/evo/mnhftx.htest/util/data/non-backported.txtsrc/evo/mnhftx.cppsrc/node/chainstate.hsrc/net_processing.cppsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Applied to files:
src/test/net_peer_connection_tests.cppsrc/test/util/setup_common.hsrc/test/denialofservice_tests.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Applied to files:
src/chainlock/signing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
src/test/util/setup_common.hsrc/evo/mnhftx.hsrc/evo/mnhftx.cppsrc/node/chainstate.hsrc/node/chainstate.cppsrc/init.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
src/test/util/setup_common.htest/util/data/non-backported.txtsrc/node/chainstate.hsrc/node/chainstate.cppsrc/init.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
src/test/util/setup_common.hsrc/init.cppsrc/node/context.h
📚 Learning: 2025-10-03T11:20:37.475Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
Applied to files:
src/test/util/setup_common.hsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/context.hsrc/init.cppsrc/node/context.hsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
Applied to files:
src/test/util/setup_common.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/evo/mnhftx.hsrc/masternode/active/context.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/node/chainstate.cppsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/context.h
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/evo/mnhftx.hsrc/net_processing.hsrc/llmq/observer/context.hsrc/masternode/active/context.cppsrc/llmq/context.hsrc/init.cppsrc/llmq/quorums.hsrc/llmq/context.cppsrc/node/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/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Applied to files:
src/test/denialofservice_tests.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-10-13T12:37:12.357Z
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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:
test/util/data/non-backported.txtsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Applied to files:
src/masternode/active/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-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/evo/mnhftx.cpp
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Applied to files:
src/node/chainstate.cpp
📚 Learning: 2025-10-28T18:36:40.263Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6923
File: src/test/util/setup_common.cpp:235-251
Timestamp: 2025-10-28T18:36:40.263Z
Learning: In `src/test/util/setup_common.cpp`, the `CEvoDB` instance in `BasicTestingSetup` is constructed with `.memory = true` flag (memory-only mode), so it does not create file handles on disk. This makes the destructor teardown order safe even if `fs::remove_all(m_path_root)` is called before `m_node.evodb.reset()`.
Applied to files:
src/node/chainstate.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-12-02T12:59:28.247Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7026
File: src/rpc/evo.cpp:1713-1799
Timestamp: 2025-12-02T12:59:28.247Z
Learning: In the Dash codebase, CChain::operator[] has built-in safe bounds checking. Accessing chainman.ActiveChain()[height] is safe even if height exceeds the current chain height, as the operator will handle bounds checking internally. There is no need to manually check chain height before using the [] operator on CChain.
Applied to files:
src/net_processing.cpp
📚 Learning: 2025-11-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/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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.cppsrc/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/test/validation_chainstatemanager_tests.cpp
🧬 Code graph analysis (19)
src/llmq/options.h (3)
src/node/chainstate.h (1)
Consensus(30-32)src/llmq/params.h (1)
LLMQType(14-125)src/llmq/options.cpp (12)
IsAllMembersConnectedEnabled(34-37)IsAllMembersConnectedEnabled(34-34)IsQuorumPoseEnabled(39-42)IsQuorumPoseEnabled(39-39)IsQuorumRotationEnabled(45-57)IsQuorumRotationEnabled(45-45)IsQuorumTypeEnabled(106-109)IsQuorumTypeEnabled(106-106)IsQuorumTypeEnabledInternal(111-156)IsQuorumTypeEnabledInternal(111-112)GetEnabledQuorumVvecSyncEntries(59-104)GetEnabledQuorumVvecSyncEntries(59-59)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(130-140)MakePeerManager(130-135)
src/chainlock/signing.cpp (2)
src/instantsend/signing.cpp (4)
RegisterAsRecoveredSigsListener(49-52)RegisterAsRecoveredSigsListener(49-49)UnregisterAsRecoveredSigsListener(54-57)UnregisterAsRecoveredSigsListener(54-54)src/llmq/signing_shares.cpp (4)
RegisterAsRecoveredSigsListener(240-243)RegisterAsRecoveredSigsListener(240-240)UnregisterAsRecoveredSigsListener(245-248)UnregisterAsRecoveredSigsListener(245-245)
src/test/util/setup_common.h (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(130-140)MakePeerManager(130-135)
src/test/denialofservice_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(130-140)MakePeerManager(130-135)
src/llmq/signing.cpp (1)
src/llmq/signing.h (1)
CSigningManager(158-233)
src/net_processing.h (3)
src/llmq/observer/context.h (2)
llmq(18-24)llmq(29-52)src/node/context.h (1)
llmq(38-40)src/coinjoin/walletman.h (2)
llmq(29-31)CJWalletManager(36-64)
src/llmq/observer/context.h (2)
src/net_processing.h (1)
llmq(34-36)src/node/context.h (1)
llmq(38-40)
src/masternode/active/context.cpp (1)
src/llmq/context.cpp (2)
Stop(50-54)Stop(50-50)
src/llmq/dkgsessionhandler.cpp (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/evo/mnhftx.cpp (1)
src/txmempool.cpp (4)
ConnectManagers(440-447)ConnectManagers(440-440)DisconnectManagers(449-453)DisconnectManagers(449-449)
src/llmq/context.h (1)
src/llmq/context.cpp (2)
LLMQContext(17-37)LLMQContext(39-42)
src/net_processing.cpp (2)
src/llmq/dkgsession.cpp (4)
inv(301-301)inv(608-608)inv(819-819)inv(1174-1174)src/llmq/dkgsessionhandler.cpp (2)
Misbehaving(113-117)Misbehaving(113-113)
src/llmq/dkgsessionmgr.cpp (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/llmq/quorums.cpp (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/init.cpp (6)
src/net_processing.cpp (12)
node(615-615)node(616-616)node(641-641)node(756-756)node(791-791)node(892-896)node(907-907)node(918-918)node(929-929)node(937-937)make(2042-2055)make(2042-2052)src/init.h (1)
node(23-25)src/llmq/options.cpp (2)
GetEnabledQuorumVvecSyncEntries(59-104)GetEnabledQuorumVvecSyncEntries(59-59)src/llmq/options.h (1)
DEFAULT_WATCH_QUORUMS(36-50)src/util/system.cpp (2)
GetNumCores(1455-1458)GetNumCores(1455-1455)src/coinjoin/walletman.cpp (2)
make(152-163)make(152-155)
src/llmq/quorums.h (3)
src/llmq/utils.h (1)
llmq(31-92)src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)src/llmq/params.h (1)
LLMQType(14-125)
src/llmq/context.cpp (7)
src/llmq/quorums.cpp (2)
Start(239-245)Start(239-239)src/instantsend/net_instantsend.cpp (2)
Start(67-75)Start(67-67)src/llmq/net_signing.cpp (2)
Start(37-45)Start(37-37)src/llmq/net_signing.h (1)
Start(33-35)src/instantsend/net_instantsend.h (1)
Start(36-38)src/llmq/signing_shares.cpp (2)
Start(199-218)Start(199-199)src/bls/bls_worker.cpp (2)
Start(59-65)Start(59-59)
src/node/context.h (3)
src/net_processing.h (1)
llmq(34-36)src/llmq/observer/context.h (2)
llmq(18-24)llmq(29-52)src/llmq/observer/context.cpp (2)
ObserverContext(11-22)ObserverContext(24-27)
🪛 Cppcheck (2.18.0)
src/masternode/active/context.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/llmq/context.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/test/validation_chainstatemanager_tests.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
UdjinM6
previously approved these changes
Dec 16, 2025
| const CSporkManager& m_sporkman; | ||
| const bool m_quorums_watch{false}; | ||
|
|
||
| private: |
Collaborator
Author
Collaborator
There was a problem hiding this comment.
I guess:
Suggested change
| private: |
`
Collaborator
Author
There was a problem hiding this comment.
I use private: as a divider between ctor variables, internal state variables, function definitions, etc.
5 tasks
5 tasks
knst
approved these changes
Dec 17, 2025
| /*dash_dbs_in_memory=*/false, | ||
| quorums_recovery, | ||
| quorums_watch, | ||
| /*bls_threads=*/[&args]() -> int8_t { |
Collaborator
There was a problem hiding this comment.
nit:
I would suggest a refactoring, that takes quorums_recovery, quorums_watch, bls_threads, macrecsigsage and move them to something like struct llmq::Options inside llmq/options.h
Bit amount of bool & int arguments makes code difficult to change and support
Collaborator
Author
There was a problem hiding this comment.
Can be done as part of dash#7066 or subsequent follow-up.
| *m_node.govman, *m_node.sporkman, /*mn_activeman=*/nullptr, m_node.dmnman, | ||
| /*active_ctx=*/nullptr, /*cj_walletman=*/nullptr, m_node.llmq_ctx, | ||
| /*ignore_incoming_txs=*/false); | ||
| auto peerLogic = MakePeerManager(*connman, m_node, /*banman=*/nullptr, /*mn_activeman=*/nullptr, chainparams, /*ignore_incoming_txs=*/false); |
Collaborator
There was a problem hiding this comment.
👍 refactor: introduce MakePeerManager() helper for test setup
| namespace llmq { | ||
| struct ObserverContext final : public CValidationInterface { | ||
| ObserverContext(const ObserverContext&) = delete; | ||
| ObserverContext& operator=(const ObserverContext&) = delete; |
Collaborator
There was a problem hiding this comment.
nit: as usually, this object have unique_ptr inside, so, it's already non-copyable.
Collaborator
Author
src/net_processing.cpp
Outdated
| CMasternodeSync& m_mn_sync; | ||
| CGovernanceManager& m_govman; | ||
| CSporkManager& m_sporkman; | ||
| /** Pointer to this node's CJWalletManager. May be nullptr - check existence before dereferencing. */ |
Collaborator
There was a problem hiding this comment.
nit: seems irrelevant to re-order m_cj_walletman for refactor: create stub class for watch-only mode context
Downsides: increase diff size; increase probability of conflicts with other PRs; pollute git blame output
UPD: I see cj_walletman is moved again in the next-next commit, definitely worth to revert this change in commit refactor: create stub class for watch-only mode context
Collaborator
Author
There was a problem hiding this comment.
Currently have multiple branches that build on current set of changes, it makes the diff a bit noisy, maybe but reverting that change requires propagation throughout.
| PostProcessMessage(m_active_ctx->cj_server->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); | ||
| PostProcessMessage(m_active_ctx->qdkgsman->ProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId()); | ||
| } | ||
| if (m_observer_ctx) { |
Collaborator
There was a problem hiding this comment.
can m_active_ctx and m_observer_ctx co-exist, or they can't be created together?
I see a lot of duplicated code such as:
if ((m_observer_ctx && m_observer_ctx->qdkgsman->GetContribution(inv.hash, o))
|| (m_active_ctx && m_active_ctx->qdkgsman->GetContribution(inv.hash, o))) {
It would be great to unify it to one call:
m_masternode_context->GetContribution(inv.hash, o));
same here:
PostProcessMessage(m_active_ctx->qdkgsman->ProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId());
...
PostProcessMessage(m_observer_ctx->qdkgsman->ProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId());
So far as I see it can be only one of them be created:
if (node.mn_activeman) {
node.active_ctx = std::make_unique<ActiveContext>(chainman, *node.connman, *node.dmnman, *node.dstxman, *node.govman, *node.mn_metaman,
*node.mnhf_manager, *node.sporkman, *node.mempool, *node.llmq_ctx, *node.peerman,
*node.mn_activeman, *node.mn_sync, dash_db_params, quorums_watch);
...
} else if (quorums_watch) {
node.observer_ctx = std::make_unique<llmq::ObserverContext>(*node.llmq_ctx->bls_worker, chainman.ActiveChainstate(), *node.dmnman, *node.mn_metaman,
*node.llmq_ctx->dkg_debugman, *node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qman,
*node.llmq_ctx->qsnapman, *node.sporkman, dash_db_params);
}consider to create interface-struct that generalize ActiveContext and llmq::ObserverContext;
and replace after that all logic from:
m_node.active_ctx && m_node.active_ctx.DoSomething();
m_node.observer_ctx && m_node.quorum_ctx.DoSomething();
to just
m_node.quorum_ctx && m_node.quorum_ctx.DoSomething();
Collaborator
Author
There was a problem hiding this comment.
Could be addressed through an a followup, probably in dash#7066 or a subsequent PR. It won't be quorum_ctx, as it'd be expanding even more, once all the changes are in place, we can look into optimisations.
knst
reviewed
Dec 17, 2025
Collaborator
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
ops,too early, see comments about auto qdkgsman
knst
requested changes
Dec 17, 2025
Collaborator
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
see upd about atomic for qdkgsman
This was referenced Dec 18, 2025
|
This pull request has conflicts, please rebase. |
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 (1)
src/llmq/context.cpp (1)
17-37: Fix clang-format issues.CI is failing due to clang-format differences in this file. Please run the project's clang-format tooling (e.g.,
clang-format -i --style=file src/llmq/context.cpporgit clang-format) to resolve the formatting discrepancies.
♻️ Duplicate comments (2)
src/llmq/quorums.cpp (2)
454-457: The atomic load pattern form_qdkgsmanwithmemory_order_acquireand null check is consistent with the design discussed in past reviews. The graceful handling whenqdkgsmanis nullptr supports non-masternode and observer modes.
803-806: Consistent atomic load pattern form_qdkgsmanaccess, matching the pattern used inBuildQuorumContributions.
🧹 Nitpick comments (4)
src/llmq/blockprocessor.cpp (1)
49-58: Consider validating thebls_threadsparameter.The constructor now accepts
bls_threadsdirectly without validation. Consider adding a check to ensure the value is reasonable (e.g.,>= 0and perhaps<= MAX_REASONABLE_VALUE) to prevent misconfiguration from propagating silently.While callers should provide valid values, defensive validation at this boundary would catch configuration errors early.
🔎 Suggested validation
CQuorumBlockProcessor::CQuorumBlockProcessor(CChainState& chainstate, CDeterministicMNManager& dmnman, CEvoDB& evoDb, CQuorumSnapshotManager& qsnapman, int8_t bls_threads) : m_chainstate{chainstate}, m_dmnman{dmnman}, m_evoDb{evoDb}, m_qsnapman{qsnapman} { + if (bls_threads < 0) { + throw std::invalid_argument("bls_threads must be non-negative"); + } utils::InitQuorumsCache(mapHasMinedCommitmentCache); LogPrintf("BLS verification uses %d additional threads\n", bls_threads); m_bls_queue.StartWorkerThreads(bls_threads); }src/node/chainstate.h (1)
103-116: Consider whether default parameter values could simplify call sites.The
LoadChainstatesignature now has many parameters (16+), with several new LLMQ-related ones added. While this is acceptable for an initialization function, consider whether some parameters could have default values to simplify common use cases.For example,
quorums_recovery,quorums_watch, andbls_threadsmight have sensible defaults derived from other parameters or constants.Note: If most call sites will use the same values, defaults can reduce duplication and make the intent clearer.
src/llmq/quorums.cpp (1)
280-282: Consider usingfind()instead ofcount()+at().The current pattern calls
count()thenat(), which performs two lookups. Usingfind()would be slightly more efficient.🔎 Optional simplification
- const bool fSyncForTypeEnabled = m_sync_map.count(pQuorum->qc->llmqType) > 0; - const QvvecSyncMode syncMode = fSyncForTypeEnabled ? m_sync_map.at(pQuorum->qc->llmqType) - : QvvecSyncMode::Invalid; + const auto it = m_sync_map.find(pQuorum->qc->llmqType); + const bool fSyncForTypeEnabled = it != m_sync_map.end(); + const QvvecSyncMode syncMode = fSyncForTypeEnabled ? it->second : QvvecSyncMode::Invalid;src/net_processing.cpp (1)
1638-1639: Re‑check QUORUM inv/getdata and misbehavior behavior when neither ActiveContext nor ObserverContext is presentWith these changes, nodes that are neither masternodes nor quorum observers behave differently for QUORUM objects:
AlreadyHave()forMSG_QUORUM_CONTRIB/COMPLAINT/JUSTIFICATION/PREMATURE_COMMITMENTnow returnsfalsewhenever bothm_active_ctxandm_observer_ctxare null.- In the INV path, that still leads to
RequestObject(...)being queued for those invs once IBD is over.ProcessGetData()now only serves those objects if one of the two contexts is present (*_ctx->qdkgsman->Get*); otherwise we will respond with NOTFOUND for requests we initiated.- At the same time, the new
if (!m_active_ctx && !m_observer_ctx)block inProcessMessagemarks directQCONTRIB/QCOMPLAINT/QJUSTIFICATION/QPCOMMITMENT/QWATCHmessages as misbehavior (howmuch = 10), regardless of whether we previously requested these via getdata.- QWATCH announcements are now only sent from us after VERACK when
qman->IsWatching()and the peer is recognized as a masternode, which seems correct for watcher-mode handshakes.This is probably fine if, in practice, nodes without either context never receive QUORUM invs and never send corresponding GETDATA (e.g., because upstream code only broadcasts those invs to masternodes/observers). If they can receive such invs today, they will now:
- Request QUORUM objects they cannot store or serve.
- Potentially penalize peers (
Misbehaving(10)) for answering with Q* messages we effectively solicited.I’d suggest double‑checking the intended behavior for “plain” nodes (no
ActiveContext, noObserverContext):
- If they are supposed to ignore QUORUM traffic entirely, consider short‑circuiting
AlreadyHave()totruefor QUORUM types when both contexts are null, or gating the INV/GETDATA path on the same condition.- Alternatively, if they should not even see QUORUM invs, please confirm that the llmq components never call into the generic
RelayInv/RequestObjectflow for peers without an associated quorum context.Given how subtle the p2p/llmq interaction is, this feels worth a quick verification before merging.
Also applies to: 2360-2361, 2938-2972, 4015-4017, 5462-5483
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/Makefile.am(2 hunks)src/chainlock/chainlock.cpp(0 hunks)src/chainlock/signing.cpp(1 hunks)src/chainlock/signing.h(1 hunks)src/dsnotificationinterface.cpp(0 hunks)src/init.cpp(9 hunks)src/instantsend/instantsend.h(1 hunks)src/instantsend/net_instantsend.cpp(0 hunks)src/instantsend/signing.cpp(1 hunks)src/instantsend/signing.h(1 hunks)src/llmq/blockprocessor.cpp(1 hunks)src/llmq/blockprocessor.h(1 hunks)src/llmq/context.cpp(2 hunks)src/llmq/context.h(2 hunks)src/llmq/dkgsessionhandler.cpp(3 hunks)src/llmq/dkgsessionhandler.h(2 hunks)src/llmq/dkgsessionmgr.cpp(2 hunks)src/llmq/dkgsessionmgr.h(2 hunks)src/llmq/observer/context.cpp(1 hunks)src/llmq/observer/context.h(1 hunks)src/llmq/options.cpp(1 hunks)src/llmq/options.h(1 hunks)src/llmq/quorums.cpp(14 hunks)src/llmq/quorums.h(5 hunks)src/llmq/signing.cpp(2 hunks)src/llmq/signing.h(2 hunks)src/llmq/signing_shares.cpp(1 hunks)src/llmq/signing_shares.h(1 hunks)src/llmq/utils.cpp(2 hunks)src/llmq/utils.h(1 hunks)src/masternode/active/context.cpp(3 hunks)src/masternode/active/context.h(4 hunks)src/masternode/active/notificationinterface.cpp(2 hunks)src/net_processing.cpp(8 hunks)src/net_processing.h(2 hunks)src/node/chainstate.cpp(4 hunks)src/node/chainstate.h(3 hunks)src/node/context.cpp(1 hunks)src/node/context.h(2 hunks)src/test/denialofservice_tests.cpp(4 hunks)src/test/net_peer_connection_tests.cpp(1 hunks)src/test/util/setup_common.cpp(5 hunks)src/test/util/setup_common.h(1 hunks)src/test/validation_chainstatemanager_tests.cpp(1 hunks)test/util/data/non-backported.txt(1 hunks)
💤 Files with no reviewable changes (3)
- src/instantsend/net_instantsend.cpp
- src/chainlock/chainlock.cpp
- src/dsnotificationinterface.cpp
🚧 Files skipped from review as they are similar to previous changes (12)
- src/masternode/active/notificationinterface.cpp
- test/util/data/non-backported.txt
- src/instantsend/signing.h
- src/instantsend/signing.cpp
- src/test/denialofservice_tests.cpp
- src/llmq/signing.h
- src/Makefile.am
- src/node/context.cpp
- src/llmq/signing_shares.cpp
- src/chainlock/signing.cpp
- src/llmq/dkgsessionmgr.cpp
- src/instantsend/instantsend.h
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/blockprocessor.hsrc/llmq/signing_shares.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.hsrc/net_processing.hsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/test/util/setup_common.hsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/test/net_peer_connection_tests.cppsrc/masternode/active/context.hsrc/node/chainstate.cppsrc/llmq/utils.hsrc/chainlock/signing.h
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/test/util/setup_common.hsrc/test/net_peer_connection_tests.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/blockprocessor.hsrc/llmq/signing_shares.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/llmq/dkgsessionhandler.hsrc/llmq/quorums.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/llmq/utils.h
src/{masternode,llmq}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
BLS integration must be used for cryptographic foundation of advanced masternode features
Files:
src/llmq/blockprocessor.hsrc/llmq/signing_shares.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/llmq/dkgsessionhandler.hsrc/llmq/quorums.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/llmq/utils.h
src/llmq/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Files:
src/llmq/blockprocessor.hsrc/llmq/signing_shares.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/llmq/dkgsessionhandler.hsrc/llmq/quorums.hsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/llmq/utils.h
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/blockprocessor.hsrc/llmq/signing_shares.hsrc/llmq/observer/context.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/llmq/dkgsessionhandler.hsrc/llmq/quorums.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/llmq/utils.h
src/node/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Files:
src/node/chainstate.hsrc/node/context.hsrc/node/chainstate.cpp
src/node/chainstate.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Files:
src/node/chainstate.hsrc/node/chainstate.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.cppsrc/masternode/active/context.h
🧠 Learnings (49)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: 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: 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: 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.
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/evodb/**/*.{cpp,h} : Evolution Database (CEvoDb) must handle masternode snapshots, quorum state, governance objects with efficient differential updates for masternode lists
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.cppsrc/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/{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/test/validation_chainstatemanager_tests.cppsrc/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,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/test/validation_chainstatemanager_tests.cppsrc/llmq/signing_shares.hsrc/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:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/test/util/setup_common.hsrc/llmq/quorums.cppsrc/test/net_peer_connection_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/utils.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.hsrc/net_processing.hsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/node/context.hsrc/llmq/quorums.cppsrc/llmq/options.hsrc/llmq/context.hsrc/masternode/active/context.hsrc/node/chainstate.cppsrc/llmq/utils.h
📚 Learning: 2025-10-28T18:36:40.263Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6923
File: src/test/util/setup_common.cpp:235-251
Timestamp: 2025-10-28T18:36:40.263Z
Learning: In `src/test/util/setup_common.cpp`, the `CEvoDB` instance in `BasicTestingSetup` is constructed with `.memory = true` flag (memory-only mode), so it does not create file handles on disk. This makes the destructor teardown order safe even if `fs::remove_all(m_path_root)` is called before `m_node.evodb.reset()`.
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.hsrc/test/util/setup_common.hsrc/node/chainstate.cpp
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/init.cppsrc/test/util/setup_common.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/blockprocessor.hsrc/llmq/dkgsessionmgr.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.hsrc/net_processing.hsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/signing.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/node/chainstate.cppsrc/llmq/utils.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/observer/context.cppsrc/llmq/utils.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/net_processing.hsrc/init.cppsrc/net_processing.cppsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/node/chainstate.cppsrc/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/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/test/util/setup_common.hsrc/masternode/active/context.hsrc/node/chainstate.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/blockprocessor.hsrc/llmq/blockprocessor.cppsrc/test/util/setup_common.cppsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/llmq/options.hsrc/llmq/context.hsrc/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/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/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/signing_shares.hsrc/llmq/utils.cppsrc/llmq/options.cppsrc/llmq/blockprocessor.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/net_processing.hsrc/init.cppsrc/net_processing.cppsrc/node/chainstate.hsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/options.hsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/node/chainstate.cppsrc/llmq/utils.h
📚 Learning: 2025-10-03T11:20:37.475Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/llmq/observer/context.cppsrc/llmq/observer/context.hsrc/test/util/setup_common.cppsrc/init.cppsrc/net_processing.cppsrc/node/context.hsrc/masternode/active/context.cppsrc/test/util/setup_common.hsrc/llmq/quorums.cppsrc/llmq/context.hsrc/masternode/active/context.h
📚 Learning: 2025-08-19T14:57:31.801Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.
Applied to files:
src/llmq/blockprocessor.hsrc/llmq/blockprocessor.cppsrc/llmq/quorums.hsrc/llmq/quorums.cpp
📚 Learning: 2025-09-09T21:36:11.833Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:0-0
Timestamp: 2025-09-09T21:36:11.833Z
Learning: In RawSender class (src/stats/rawsender.cpp), cs_net is the appropriate mutex for protecting socket access (m_sock) and network operations, not additional custom locks. The implementation correctly uses cs_net with GUARDED_BY annotations and EXCLUSIVE_LOCKS_REQUIRED to synchronize socket access between SendDirectly() and ReconnectThread().
Applied to files:
src/llmq/signing_shares.h
📚 Learning: 2025-09-09T21:36:58.969Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6837
File: src/stats/rawsender.cpp:223-232
Timestamp: 2025-09-09T21:36:58.969Z
Learning: In RawSender class (src/stats/rawsender.cpp), socket operations are properly synchronized using the cs_net mutex. The m_sock and m_server members are GUARDED_BY(cs_net), and methods like Connect(), SendDirectly(), and ReconnectThread() use appropriate locking with cs_net to prevent race conditions on socket access.
Applied to files:
src/llmq/signing_shares.h
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/llmq/observer/context.cppsrc/llmq/observer/context.hsrc/net_processing.hsrc/init.cppsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/context.cppsrc/llmq/context.hsrc/masternode/active/context.hsrc/node/chainstate.cpp
📚 Learning: 2025-02-14T15:15:58.165Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6529
File: src/rpc/governance.cpp:1074-1089
Timestamp: 2025-02-14T15:15:58.165Z
Learning: Code blocks marked with `// clang-format off` and `// clang-format on` directives should be excluded from clang-format suggestions as they are intentionally formatted manually for better readability.
Applied to files:
src/llmq/observer/context.cpp
📚 Learning: 2025-07-09T15:05:36.250Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6729
File: src/rpc/evo.cpp:1273-1273
Timestamp: 2025-07-09T15:05:36.250Z
Learning: When clang-format suggestions significantly harm readability (like splitting logical parameter groups across multiple lines), it's acceptable to use `// clang-format off` and `// clang-format on` directives to exclude the problematic section from automatic formatting, prioritizing code readability over strict line length compliance.
Applied to files:
src/llmq/observer/context.cpp
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Applied to files:
src/llmq/observer/context.hsrc/init.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/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
src/test/util/setup_common.cppsrc/init.cppsrc/net_processing.cppsrc/node/context.hsrc/test/util/setup_common.hsrc/masternode/active/context.h
📚 Learning: 2024-12-29T17:43:41.755Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/quorums.cpp:224-224
Timestamp: 2024-12-29T17:43:41.755Z
Learning: The `CQuorumManager` is fully initialized by `LLMQContext`, addressing any concerns about the manager’s initialization sequence.
Applied to files:
src/llmq/dkgsessionhandler.hsrc/llmq/quorums.hsrc/node/context.hsrc/masternode/active/context.cppsrc/llmq/quorums.cppsrc/llmq/context.cppsrc/llmq/context.hsrc/node/chainstate.cpp
📚 Learning: 2025-08-19T15:08:00.835Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/commitment.cpp:54-60
Timestamp: 2025-08-19T15:08:00.835Z
Learning: In Dash Core, llmq_params.size == 1 is used only for regtest environments, not on public networks, which significantly reduces the risk profile of code paths specific to single-member quorums.
Applied to files:
src/init.cppsrc/llmq/quorums.hsrc/llmq/quorums.cppsrc/node/chainstate.cpp
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Applied to files:
src/init.cppsrc/llmq/quorums.cpp
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Applied to files:
src/init.cppsrc/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
Applied to files:
src/init.cppsrc/test/util/setup_common.h
📚 Learning: 2025-11-04T18:24:27.241Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.
Applied to files:
src/init.cppsrc/net_processing.cppsrc/llmq/quorums.hsrc/llmq/quorums.cpp
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Applied to files:
src/init.cppsrc/net_processing.cppsrc/llmq/quorums.cppsrc/chainlock/signing.h
📚 Learning: 2025-10-28T08:54:00.392Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6926
File: test/functional/feature_governance_cl.py:0-0
Timestamp: 2025-10-28T08:54:00.392Z
Learning: In Dash tests, the scheduler (mockscheduler) operates independently of network state. Isolated nodes should still run scheduler-based cleanup processes like governance cleanup, even if they have different state due to network isolation.
Applied to files:
src/init.cpp
📚 Learning: 2025-10-13T12:37:12.357Z
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Applied to files:
src/net_processing.cppsrc/llmq/quorums.cpp
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: 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/net_processing.cppsrc/masternode/active/context.cppsrc/masternode/active/context.h
📚 Learning: 2025-08-08T04:30:37.971Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:0-0
Timestamp: 2025-08-08T04:30:37.971Z
Learning: In the Dash codebase, direct RPC calling has been removed in recent commits, making suggestions about RPC command validation and error handling for direct RPC calls obsolete. The ProposalWizard and related components no longer use direct RPC calling patterns.
Applied to files:
src/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/net_processing.cppsrc/masternode/active/context.h
📚 Learning: 2025-12-02T12:59:28.247Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7026
File: src/rpc/evo.cpp:1713-1799
Timestamp: 2025-12-02T12:59:28.247Z
Learning: In the Dash codebase, CChain::operator[] has built-in safe bounds checking. Accessing chainman.ActiveChain()[height] is safe even if height exceeds the current chain height, as the operator will handle bounds checking internally. There is no need to manually check chain height before using the [] operator on CChain.
Applied to files:
src/net_processing.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/net_processing.cppsrc/masternode/active/context.h
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Applied to files:
src/node/chainstate.hsrc/masternode/active/context.h
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.
Applied to files:
src/node/chainstate.hsrc/node/chainstate.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/quorums.hsrc/llmq/quorums.cppsrc/llmq/signing.cpp
📚 Learning: 2025-10-21T11:09:34.688Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.
Applied to files:
src/llmq/quorums.h
📚 Learning: 2025-07-22T14:38:45.606Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6738
File: src/evo/smldiff.h:91-94
Timestamp: 2025-07-22T14:38:45.606Z
Learning: In the Dash codebase, EXCLUSIVE_LOCKS_REQUIRED annotations are Clang thread safety annotations that are enforced at compile time on supported platforms like macOS. If callers don't properly hold the required locks, the build will fail with compile-time errors, not runtime issues.
Applied to files:
src/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
src/llmq/quorums.hsrc/test/util/setup_common.h
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Applied to files:
src/llmq/quorums.cpp
📚 Learning: 2025-07-29T14:33:01.040Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Applied to files:
src/llmq/quorums.cpp
📚 Learning: 2025-07-17T15:48:29.418Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
Applied to files:
src/llmq/quorums.cpp
🧬 Code graph analysis (18)
src/llmq/signing_shares.h (3)
src/chainlock/signing.cpp (4)
RegisterRecoveryInterface(33-36)RegisterRecoveryInterface(33-33)UnregisterRecoveryInterface(38-41)UnregisterRecoveryInterface(38-38)src/instantsend/signing.cpp (4)
RegisterRecoveryInterface(49-52)RegisterRecoveryInterface(49-49)UnregisterRecoveryInterface(54-57)UnregisterRecoveryInterface(54-54)src/llmq/signing_shares.cpp (4)
RegisterRecoveryInterface(240-243)RegisterRecoveryInterface(240-240)UnregisterRecoveryInterface(245-248)UnregisterRecoveryInterface(245-245)
src/llmq/observer/context.cpp (4)
src/net_processing.cpp (3)
UpdatedBlockTip(2211-2246)UpdatedBlockTip(2211-2211)pindexNew(607-607)src/llmq/dkgsessionmgr.cpp (2)
UpdatedBlockTip(76-90)UpdatedBlockTip(76-76)src/masternode/active/notificationinterface.cpp (2)
UpdatedBlockTip(22-32)UpdatedBlockTip(22-23)src/dsnotificationinterface.cpp (2)
UpdatedBlockTip(67-86)UpdatedBlockTip(67-67)
src/llmq/observer/context.h (4)
src/net_processing.h (1)
llmq(34-36)src/node/context.h (1)
llmq(38-40)src/llmq/dkgsessionmgr.cpp (4)
CDKGSessionManager(32-58)CDKGSessionManager(60-60)UpdatedBlockTip(76-90)UpdatedBlockTip(76-76)src/llmq/observer/context.cpp (4)
ObserverContext(11-22)ObserverContext(24-27)UpdatedBlockTip(29-35)UpdatedBlockTip(29-29)
src/llmq/dkgsessionhandler.h (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/net_processing.h (3)
src/llmq/observer/context.h (2)
llmq(18-24)llmq(29-51)src/node/context.h (1)
llmq(38-40)src/coinjoin/walletman.h (2)
llmq(29-31)CJWalletManager(36-64)
src/init.cpp (5)
src/llmq/options.cpp (2)
GetEnabledQuorumVvecSyncEntries(59-104)GetEnabledQuorumVvecSyncEntries(59-59)src/llmq/options.h (1)
DEFAULT_WATCH_QUORUMS(37-51)src/util/system.cpp (2)
GetNumCores(1455-1458)GetNumCores(1455-1455)src/coinjoin/walletman.cpp (2)
make(152-163)make(152-155)src/llmq/dkgsessionmgr.cpp (2)
CleanupOldContributions(391-433)CleanupOldContributions(391-391)
src/llmq/quorums.h (7)
src/llmq/blockprocessor.h (1)
llmq(36-103)src/llmq/utils.h (1)
llmq(32-99)src/llmq/types.h (1)
llmq(10-17)src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)src/dbwrapper.h (1)
CDBWrapper(214-534)src/llmq/params.h (1)
LLMQType(14-125)src/llmq/quorums.cpp (5)
GUARDED_BY(38-38)CQuorumManager(210-232)CQuorumManager(234-237)Start(239-245)Start(239-239)
src/masternode/active/context.cpp (2)
src/llmq/context.cpp (2)
Stop(50-54)Stop(50-50)src/llmq/signing_shares.cpp (2)
Stop(220-238)Stop(220-220)
src/test/util/setup_common.h (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(130-140)MakePeerManager(130-135)
src/llmq/quorums.cpp (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/llmq/signing.cpp (1)
src/llmq/signing.h (1)
CSigningManager(158-233)
src/llmq/dkgsessionhandler.cpp (1)
src/llmq/dkgsessionmgr.h (1)
m_quorums_watch(60-126)
src/llmq/options.h (2)
src/llmq/params.h (1)
LLMQType(14-125)src/llmq/options.cpp (2)
GetEnabledQuorumVvecSyncEntries(59-104)GetEnabledQuorumVvecSyncEntries(59-59)
src/llmq/context.cpp (4)
src/llmq/quorums.cpp (2)
Start(239-245)Start(239-239)src/instantsend/net_instantsend.cpp (2)
Start(67-75)Start(67-67)src/llmq/net_signing.cpp (2)
Start(37-45)Start(37-37)src/bls/bls_worker.cpp (2)
Start(59-65)Start(59-59)
src/llmq/context.h (1)
src/llmq/context.cpp (2)
LLMQContext(17-37)LLMQContext(39-42)
src/test/net_peer_connection_tests.cpp (1)
src/test/util/setup_common.cpp (2)
MakePeerManager(130-140)MakePeerManager(130-135)
src/masternode/active/context.h (5)
src/masternode/meta.cpp (2)
CMasternodeMetaMan(20-23)CMasternodeMetaMan(25-29)src/llmq/dkgsessionmgr.cpp (2)
CDKGSessionManager(32-58)CDKGSessionManager(60-60)src/llmq/context.h (1)
util(31-33)src/llmq/observer/context.h (1)
util(25-27)src/instantsend/instantsend.h (1)
util(36-38)
src/chainlock/signing.h (3)
src/chainlock/signing.cpp (4)
RegisterRecoveryInterface(33-36)RegisterRecoveryInterface(33-33)UnregisterRecoveryInterface(38-41)UnregisterRecoveryInterface(38-38)src/instantsend/signing.cpp (4)
RegisterRecoveryInterface(49-52)RegisterRecoveryInterface(49-49)UnregisterRecoveryInterface(54-57)UnregisterRecoveryInterface(54-54)src/llmq/signing_shares.cpp (4)
RegisterRecoveryInterface(240-243)RegisterRecoveryInterface(240-240)UnregisterRecoveryInterface(245-248)UnregisterRecoveryInterface(245-245)
🪛 Cppcheck (2.18.0)
src/test/validation_chainstatemanager_tests.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/masternode/active/context.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
src/llmq/context.cpp
[error] 24-24: #error No known always_inline attribute for this platform.
(preprocessorErrorDirective)
🪛 GitHub Actions: Clang Diff Format Check
src/llmq/observer/context.cpp
[error] 1-1: Clang-format differences detected. File was reformatted by clang-format-diff.
src/llmq/context.cpp
[error] 1-1: Clang-format differences detected. File was reformatted by clang-format-diff.
⏰ 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: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: Lint / Run linters
- GitHub Check: win64-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
5 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
Dec 23, 2025
040ac27 ci: bump CTCACHE_COMMIT to e393144d5c49b060a1dbc7ae15b9c6973efb967d (UdjinM6) 7c310c7 refactor: derive ctcache Python script path from wrapper location (UdjinM6) 28577db refactor: improve ctcache configuration maintainability (UdjinM6) 7ef44b6 chore: adjust removal count message (UdjinM6) 64ff657 fix: improve file removal pipeline robustness (UdjinM6) 2944562 feat: verify clang and ctcache are installed correctly (UdjinM6) a1b98b1 chore: make paths more maintainable (UdjinM6) ead9136 feat: integrate ctcache for clang-tidy result caching in CI (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Add [ctcache](https://github.com/matus-chochlik/ctcache) to cache clang-tidy analysis results, reducing time spent in "Run linters" step from 33-34 minutes to 3-4 minutes in the best case scenarios. ## What was done? - Download ctcache in `contrib/containers/ci/ci.Dockerfile` and patch it to work correctly - Add new steps in `.github/workflows/build-src.yml` to save/restore ctcache cache (for `linux64_multiprocess`, save on pushes to default branch only) - Adjust `ci/dash/lint-tidy.sh` to configure ctcache, force `run-clang-tidy` to use it - Add manual cache cleanup in `ci/dash/lint-tidy.sh` since ctcache doesn't have a way to set internal limits like ccache does ## How Has This Been Tested? Run, check "Run linters" step in `linux64_multiprocess-build` job. develop, no ctcache: [31m 43s](https://github.com/UdjinM6/dash/actions/runs/20273923736/job/58217581581) develop*, empty cache: 0% hits, [33m 19s](https://github.com/UdjinM6/dash/actions/runs/20287332888/job/58265404189) develop*, second run, using cache: 100% hits, [3m 27s](https://github.com/UdjinM6/dash/actions/runs/20287332888/job/58281396883) New branch*, merged #6947 (chainlock, evo): 96.5% hits, [5m 13s](https://github.com/UdjinM6/dash/actions/runs/20300087485/job/58303447008) New branch*, merged #7056 (chainlock, instantsend, llmq, etc): 59.3%, [19m 50s](https://github.com/UdjinM6/dash/actions/runs/20300940029/job/58306338715) New branch*, merged #7005 (wallet, utils): 40.9% hits, [25m 49s](https://github.com/UdjinM6/dash/actions/runs/20293363566/job/58282142424) _* with this patch merged into develop_ ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: utACK 040ac27 PastaPastaPasta: utACK 040ac27 Tree-SHA512: 3fe629b533773c6200caf0037c541180c91d7d9b9468eba410c0c51c17ae5df28d3d02380a4d4c14654eb73e20ea07d2c0748462874b11e33e1a7a1ae0dd22f4
dash-chainstate may not have ArgsManager, we should try and resolve the expected value before moving to chainstate logic.
Same rationale as previous commit.
Same rationale as previous commit.
Same rationale as previous commit.
The context that creates the signer should register the listener rather than relying on the timing being exactly right on when Start() is called by the controlling manager. Also: - Use more descriptive function names replacing Start()/Stop() - Fixup order of events in ActiveContext c/dtor and Start()/Stop()
Also: - Drop unneeded argument from LLMQContext::Start() - Drop LLMQContext::Interrupt() that does nothing
We are going to modify PeerManager multiple times, this should make the diffs that'll attract smaller.
Needed when `CActiveMasternodeManager` is moved to `ActiveContext`
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 (1)
src/init.cpp (1)
1415-1421: Avoid usingint8_tdirectly for-parblsthread-count calculationsThe
bls_threadslambda currently usesint8_tfor both reading-parblsand doing arithmetic:/*bls_threads=*/[&args]() -> int8_t { int8_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); if (threads <= 0) { threads += GetNumCores(); } return std::clamp<int8_t>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS); }(),For large positive
-parblsvalues (>= 128), this will overflow when narrowing toint8_tbefore the clamping logic runs, and the subsequentthreads += GetNumCores()path can behave unexpectedly (though it effectively clamps down to 0 threads rather than breaking things).To keep behaviour well-defined for all integer inputs while preserving existing semantics for normal ranges, it would be safer to perform the computation in a wider type and only narrow at the very end:
Proposed fix
- /*bls_threads=*/[&args]() -> int8_t { - int8_t threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); - if (threads <= 0) { - // -parbls=0 means autodetect (number of cores - 1 validator threads) - // -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads) - threads += GetNumCores(); - } - // Subtract 1 because the main thread counts towards the par threads - return std::clamp<int8_t>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS); - }(), + /*bls_threads=*/[&args]() -> int8_t { + int threads = args.GetIntArg("-parbls", llmq::DEFAULT_BLSCHECK_THREADS); + if (threads <= 0) { + // -parbls=0 means autodetect (number of cores - 1 validator threads) + // -parbls=-n means "leave n cores free" (number of cores - n - 1 validator threads) + threads += GetNumCores(); + } + // Subtract 1 because the main thread counts towards the par threads + threads = std::clamp<int>(threads - 1, 0, llmq::MAX_BLSCHECK_THREADS); + return static_cast<int8_t>(threads); + }(),This keeps
LLMQContextand downstream constructors receiving the same value in the expected range but avoids accidental overflow on unusual-parblsinputs.Also applies to: 1961-1963, 2041-2051
♻️ Duplicate comments (1)
src/instantsend/instantsend.h (1)
121-121: Proper acquire-release synchronization.The explicit
std::memory_order_acquirecorrectly pairs with thestd::memory_order_releasestores inConnectSignerandDisconnectSigner, ensuring proper visibility of the signer pointer across threads.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
src/Makefile.amsrc/chainlock/chainlock.cppsrc/chainlock/signing.cppsrc/chainlock/signing.hsrc/dsnotificationinterface.cppsrc/init.cppsrc/instantsend/instantsend.hsrc/instantsend/net_instantsend.cppsrc/instantsend/signing.cppsrc/instantsend/signing.hsrc/llmq/blockprocessor.cppsrc/llmq/blockprocessor.hsrc/llmq/context.cppsrc/llmq/context.hsrc/llmq/dkgsessionhandler.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/dkgsessionmgr.cppsrc/llmq/dkgsessionmgr.hsrc/llmq/observer/context.cppsrc/llmq/observer/context.hsrc/llmq/options.cppsrc/llmq/options.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/signing.cppsrc/llmq/signing.hsrc/llmq/signing_shares.cppsrc/llmq/signing_shares.hsrc/llmq/utils.cppsrc/llmq/utils.hsrc/masternode/active/context.cppsrc/masternode/active/context.hsrc/masternode/active/notificationinterface.cppsrc/net_processing.cppsrc/net_processing.hsrc/node/chainstate.cppsrc/node/chainstate.hsrc/node/context.cppsrc/node/context.hsrc/test/denialofservice_tests.cppsrc/test/net_peer_connection_tests.cppsrc/test/util/setup_common.cppsrc/test/util/setup_common.hsrc/test/validation_chainstatemanager_tests.cpptest/util/data/non-backported.txt
💤 Files with no reviewable changes (3)
- src/dsnotificationinterface.cpp
- src/instantsend/net_instantsend.cpp
- src/chainlock/chainlock.cpp
🚧 Files skipped from review as they are similar to previous changes (11)
- src/llmq/signing.cpp
- src/instantsend/signing.cpp
- src/instantsend/signing.h
- src/node/context.h
- src/test/denialofservice_tests.cpp
- src/llmq/signing_shares.cpp
- src/Makefile.am
- src/llmq/observer/context.h
- src/llmq/signing_shares.h
- src/chainlock/signing.h
- src/llmq/observer/context.cpp
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/llmq/dkgsessionmgr.hsrc/node/chainstate.hsrc/test/util/setup_common.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/masternode/active/context.cppsrc/chainlock/signing.cppsrc/llmq/options.cppsrc/instantsend/instantsend.hsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/node/chainstate.cppsrc/test/net_peer_connection_tests.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.cpp
src/{masternode,evo}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/masternode/active/notificationinterface.cppsrc/masternode/active/context.hsrc/masternode/active/context.cpp
src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/llmq/dkgsessionmgr.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/masternode/active/context.cppsrc/llmq/options.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.cpp
src/{masternode,llmq}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
BLS integration must be used for cryptographic foundation of advanced masternode features
Files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/llmq/dkgsessionmgr.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/masternode/active/context.cppsrc/llmq/options.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.cpp
src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/llmq/dkgsessionmgr.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/masternode/active/context.cppsrc/llmq/options.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.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/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/llmq/dkgsessionmgr.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/options.cppsrc/llmq/dkgsessionhandler.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.cpp
src/{test,wallet/test}/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Files:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.hsrc/test/util/setup_common.cppsrc/test/net_peer_connection_tests.cpp
src/node/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Files:
src/node/context.cppsrc/node/chainstate.hsrc/node/chainstate.cpp
src/node/chainstate.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Files:
src/node/chainstate.hsrc/node/chainstate.cpp
🧠 Learnings (49)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Learnt from: 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: 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: 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: 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-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/masternode/active/notificationinterface.cppsrc/llmq/blockprocessor.hsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/node/chainstate.hsrc/llmq/blockprocessor.cppsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/llmq/context.htest/util/data/non-backported.txtsrc/llmq/options.hsrc/init.cppsrc/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing
Applied to files:
src/masternode/active/notificationinterface.cppsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/init.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
Applied to files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/node/chainstate.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/llmq/options.cppsrc/instantsend/instantsend.hsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/context.cpptest/util/data/non-backported.txtsrc/node/chainstate.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying
Applied to files:
src/masternode/active/notificationinterface.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/evodb/**/*.{cpp,h} : Evolution Database (CEvoDb) must handle masternode snapshots, quorum state, governance objects with efficient differential updates for masternode lists
Applied to files:
src/masternode/active/notificationinterface.cppsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.cppsrc/net_processing.cpptest/util/data/non-backported.txtsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: Across net_processing.cpp, once LLMQContext (m_llmq_ctx) is asserted non-null, its subcomponents (e.g., isman, qdkgsman, quorum_block_processor) are treated as initialized and used without extra null checks.
Applied to files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/masternode/active/context.cppsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/node/chainstate.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
Applied to files:
src/masternode/active/notificationinterface.cppsrc/llmq/utils.cppsrc/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/llmq/dkgsessionmgr.hsrc/node/chainstate.hsrc/llmq/blockprocessor.cppsrc/llmq/dkgsessionhandler.hsrc/masternode/active/context.cppsrc/llmq/options.cppsrc/test/util/setup_common.cppsrc/llmq/dkgsessionhandler.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/context.cpptest/util/data/non-backported.txtsrc/node/chainstate.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.hsrc/llmq/dkgsessionmgr.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/masternode/active/notificationinterface.cppsrc/masternode/active/context.hsrc/node/context.cppsrc/net_processing.cpptest/util/data/non-backported.txt
📚 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.hsrc/llmq/blockprocessor.cppsrc/llmq/quorums.cppsrc/llmq/quorums.h
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/llmq/blockprocessor.hsrc/llmq/signing.hsrc/masternode/active/context.hsrc/llmq/dkgsessionmgr.hsrc/node/chainstate.hsrc/test/util/setup_common.hsrc/llmq/dkgsessionhandler.hsrc/instantsend/instantsend.hsrc/llmq/context.hsrc/llmq/options.hsrc/llmq/utils.hsrc/net_processing.hsrc/llmq/quorums.h
📚 Learning: 2025-10-03T11:20:37.475Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/active/context.cpp:29-33
Timestamp: 2025-10-03T11:20:37.475Z
Learning: In Dash codebase, NodeContext (src/node/context.h) serves only as a container with trivial c/d-tors; member lifetime is explicitly managed via reset() calls in the shutdown sequence (src/init.cpp), not by declaration order. For example, active_ctx.reset() is called before DashChainstateSetupClose handles llmq_ctx teardown, ensuring proper destruction order regardless of declaration order in the struct.
Applied to files:
src/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/test/util/setup_common.hsrc/masternode/active/context.cppsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/quorums.cppsrc/init.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h} : Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data
Applied to files:
src/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/node/chainstate.hsrc/test/util/setup_common.hsrc/instantsend/instantsend.hsrc/test/util/setup_common.cpptest/util/data/non-backported.txtsrc/node/chainstate.cppsrc/init.cppsrc/llmq/quorums.h
📚 Learning: 2025-01-02T21:50:00.967Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the `CInstantSendManager`. Previously, this was handled globally; now it's handled as a class member in `LLMQContext`, but the concurrency control remains consistent.
Applied to files:
src/masternode/active/context.hsrc/masternode/active/context.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/node/chainstate.cppsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.h
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: The file src/masternode/sync.h containing `CMasternodeSync` is misnamed and misplaced—it has nothing to do with "masternode" functionality. It should eventually be renamed to `NodeSyncing` or `NodeSyncStatus` and moved to src/node/sync.h as a future refactoring.
Applied to files:
src/masternode/active/context.h
📚 Learning: 2025-11-13T20:02:55.480Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6969
File: src/evo/deterministicmns.h:441-479
Timestamp: 2025-11-13T20:02:55.480Z
Learning: In `src/evo/deterministicmns.h`, the `internalId` field in `CDeterministicMN` and the `mnInternalIdMap` in `CDeterministicMNList` are non-deterministic and used only for internal bookkeeping and efficient lookups. Different nodes can assign different internalIds to the same masternode depending on their sync history. Methods like `IsEqual()` intentionally ignore internalId mappings and only compare consensus-critical deterministic fields (proTxHash, collateral, state, etc.).
Applied to files:
src/masternode/active/context.hsrc/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention
Applied to files:
src/masternode/active/context.hsrc/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/node/chainstate.hsrc/llmq/dkgsessionhandler.hsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/llmq/context.hsrc/llmq/context.cpptest/util/data/non-backported.txtsrc/node/chainstate.cppsrc/llmq/options.hsrc/llmq/utils.hsrc/llmq/quorums.cppsrc/init.cppsrc/net_processing.hsrc/llmq/quorums.h
📚 Learning: 2025-10-02T18:29:54.756Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6840
File: src/net_processing.cpp:2882-2886
Timestamp: 2025-10-02T18:29:54.756Z
Learning: 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.hsrc/masternode/active/context.cppsrc/net_processing.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{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/test/validation_chainstatemanager_tests.cpptest/util/data/non-backported.txt
📚 Learning: 2025-06-09T16:43:20.996Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/test/util/setup_common.hsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/test/net_peer_connection_tests.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/{test,wallet/test}/**/*.{cpp,h} : Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-10-28T18:36:40.263Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6923
File: src/test/util/setup_common.cpp:235-251
Timestamp: 2025-10-28T18:36:40.263Z
Learning: In `src/test/util/setup_common.cpp`, the `CEvoDB` instance in `BasicTestingSetup` is constructed with `.memory = true` flag (memory-only mode), so it does not create file handles on disk. This makes the destructor teardown order safe even if `fs::remove_all(m_path_root)` is called before `m_node.evodb.reset()`.
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files
Applied to files:
src/test/validation_chainstatemanager_tests.cppsrc/node/context.cppsrc/node/chainstate.hsrc/test/util/setup_common.hsrc/test/util/setup_common.cppsrc/node/chainstate.cppsrc/init.cpp
📚 Learning: 2025-12-01T18:13:21.314Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 7018
File: test/lint/lint-github-actions.py:1-9
Timestamp: 2025-12-01T18:13:21.314Z
Learning: In the Dash repository, the file test/util/data/non-backported.txt should only include C++ files (.cpp or .h) because it is used for running clang-format. Other file types (such as Python files, .ui files, etc.) should not be added to this list.
Applied to files:
src/test/validation_chainstatemanager_tests.cpptest/util/data/non-backported.txt
📚 Learning: 2025-08-11T17:16:36.654Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
Applied to files:
src/test/validation_chainstatemanager_tests.cpptest/util/data/non-backported.txtsrc/init.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/**/*.{cpp,h} : Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Applied to files:
src/test/validation_chainstatemanager_tests.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/**/*.{cpp,h} : NodeContext must be extended with Dash-specific managers for each subsystem during initialization
Applied to files:
src/node/context.cppsrc/test/util/setup_common.hsrc/test/util/setup_common.cppsrc/net_processing.cppsrc/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,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures
Applied to files:
src/node/context.cppsrc/instantsend/instantsend.htest/util/data/non-backported.txtsrc/llmq/quorums.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Dash extends Bitcoin Core through composition with minimal changes to the Bitcoin Core foundation
Applied to files:
src/test/util/setup_common.hsrc/init.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.hsrc/masternode/active/context.cppsrc/llmq/context.hsrc/llmq/context.cppsrc/node/chainstate.cppsrc/llmq/quorums.cppsrc/llmq/quorums.h
📚 Learning: 2025-07-29T14:32:48.369Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.
Applied to files:
src/chainlock/signing.cppsrc/net_processing.cppsrc/llmq/quorums.cppsrc/init.cpp
📚 Learning: 2025-10-21T11:09:34.688Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6849
File: src/governance/governance.cpp:1339-1343
Timestamp: 2025-10-21T11:09:34.688Z
Learning: In the Dash Core codebase, `CacheMap` (defined in src/cachemap.h) is internally thread-safe and uses its own `mutable CCriticalSection cs` to protect access to its members. Methods like `GetSize()`, `Insert()`, `Get()`, `HasKey()`, etc., can be called without holding external locks.
Applied to files:
src/instantsend/instantsend.hsrc/llmq/quorums.h
📚 Learning: 2025-05-10T00:54:30.887Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6629
File: src/evo/netinfo.h:248-256
Timestamp: 2025-05-10T00:54:30.887Z
Learning: The Dash codebase includes a specialization for serializing std::shared_ptr objects in src/serialize.h, which allows direct serialization of shared_ptr instances with stream operators.
Applied to files:
src/instantsend/instantsend.h
📚 Learning: 2025-02-06T14:34:30.466Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Applied to files:
src/net_processing.cppsrc/init.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/net_processing.cppsrc/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.h
📚 Learning: 2025-08-08T04:30:37.971Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:0-0
Timestamp: 2025-08-08T04:30:37.971Z
Learning: In the Dash codebase, direct RPC calling has been removed in recent commits, making suggestions about RPC command validation and error handling for direct RPC calls obsolete. The ProposalWizard and related components no longer use direct RPC calling patterns.
Applied to files:
src/net_processing.cpp
📚 Learning: 2025-12-02T12:59:28.247Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7026
File: src/rpc/evo.cpp:1713-1799
Timestamp: 2025-12-02T12:59:28.247Z
Learning: In the Dash codebase, CChain::operator[] has built-in safe bounds checking. Accessing chainman.ActiveChain()[height] is safe even if height exceeds the current chain height, as the operator will handle bounds checking internally. There is no need to manually check chain height before using the [] operator on CChain.
Applied to files:
src/net_processing.cpp
📚 Learning: 2025-10-13T12:37:12.357Z
Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.
Applied to files:
test/util/data/non-backported.txtsrc/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/{masternode,evo}/**/*.{cpp,h} : Masternode lists must use immutable data structures (Immer library) for thread safety
Applied to files:
test/util/data/non-backported.txt
📚 Learning: 2025-11-25T10:53:37.523Z
Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.
Applied to files:
src/node/chainstate.cpp
📚 Learning: 2025-10-05T20:38:28.457Z
Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.
Applied to files:
src/llmq/quorums.cppsrc/init.cpp
📚 Learning: 2025-10-25T07:08:51.918Z
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
Applied to files:
src/llmq/quorums.cpp
📚 Learning: 2025-07-29T14:33:01.040Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.
Applied to files:
src/llmq/quorums.cpp
📚 Learning: 2025-08-19T15:08:00.835Z
Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/commitment.cpp:54-60
Timestamp: 2025-08-19T15:08:00.835Z
Learning: In Dash Core, llmq_params.size == 1 is used only for regtest environments, not on public networks, which significantly reduces the risk profile of code paths specific to single-member quorums.
Applied to files:
src/llmq/quorums.cppsrc/init.cppsrc/llmq/quorums.h
📚 Learning: 2025-07-15T14:53:04.819Z
Learnt from: knst
Repo: dashpay/dash PR: 6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Applied to files:
src/llmq/quorums.cppsrc/llmq/quorums.h
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Applied to files:
src/llmq/quorums.cppsrc/init.cpp
📚 Learning: 2025-07-17T15:48:29.418Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.
Applied to files:
src/llmq/quorums.cpp
📚 Learning: 2025-10-28T08:54:00.392Z
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6926
File: test/functional/feature_governance_cl.py:0-0
Timestamp: 2025-10-28T08:54:00.392Z
Learning: In Dash tests, the scheduler (mockscheduler) operates independently of network state. Isolated nodes should still run scheduler-based cleanup processes like governance cleanup, even if they have different state due to network isolation.
Applied to files:
src/init.cpp
📚 Learning: 2025-07-22T14:38:45.606Z
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6738
File: src/evo/smldiff.h:91-94
Timestamp: 2025-07-22T14:38:45.606Z
Learning: In the Dash codebase, EXCLUSIVE_LOCKS_REQUIRED annotations are Clang thread safety annotations that are enforced at compile time on supported platforms like macOS. If callers don't properly hold the required locks, the build will fail with compile-time errors, not runtime issues.
Applied to files:
src/llmq/quorums.h
knst
approved these changes
Dec 23, 2025
PastaPastaPasta
added a commit
that referenced
this pull request
Dec 24, 2025
7c1dc2d lint: apply most `clang-format` suggestions (Kittywhiskers Van Gogh) 3ba6daa refactor: use `UtilParameters` in more LLMQ code (Kittywhiskers Van Gogh) ff71875 refactor: drop `Params()` use in `llmq/utils.cpp` (Kittywhiskers Van Gogh) 8a3d852 refactor: misc. refactoring in `llmq/utils.cpp` (Kittywhiskers Van Gogh) 6a6a240 refactor: simplify `BuildQuorumRotationInfo()` (3/n) (Kittywhiskers Van Gogh) 3e5e273 refactor: simplify `BuildQuorumRotationInfo()` (2/n) (Kittywhiskers Van Gogh) 515163a refactor: simplify `BuildQuorumRotationInfo()` (1/n) (Kittywhiskers Van Gogh) 8c4f83b refactor: introduce `MasternodeScore` for better docs (Kittywhiskers Van Gogh) 85221d4 refactor: introduce `UtilParameters` struct to slim down arguments (Kittywhiskers Van Gogh) 1eb1330 refactor: introduce `QuorumQuarter` struct to reduce code duplication (Kittywhiskers Van Gogh) 3b6d997 refactor: extract quorum cycle data into struct, move ctors to source (Kittywhiskers Van Gogh) 17f5d88 refactor: make `SnapshotSkipMode` an `enum class` (Kittywhiskers Van Gogh) a3a6a7d refactor: move `llmq::utils::BlsCheck` {c,d}tors and functions to source (Kittywhiskers Van Gogh) afb4428 move-only: move `InitQuorumsCache()` to header, drop specializations (Kittywhiskers Van Gogh) 09d522d move-only: move `llmq/util.cpp` internals to anonymous namespace (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #7056 ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 7c1dc2d Tree-SHA512: b78c8c7f1ee74dc84a84b9bde75861e931cfa33ffb1eb38cca61de59e4dd1d2685676db9700be6f27c045eb04b2bcea6102d1bc0ef91242b77d8577eee9514f3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional Information
Dependency for refactor(rpc): add watch-only/masternode mode restrictions for
quorum dkg{info,status}, moveCDKGDebugManagerto{Active,Observer}Context#7062As mentioned in bitcoin#25862, code in
libbitcoin_kernelshould not interact withArgsManagerand currently, Dash-specific chainstate validation relies on it to query flags like-watchquorumsand-llmq-data-recovery.They have been refactored to be taken as constructor arguments in the style of bitcoin#23280 and passed from the context to the constructor of the underlying managers. Similar changes have been extended to remaining
gArgsusage.When debugging dependent PRs, it was found that there was a potential race condition as the creation of the signer was done by one entity (
ActiveContext) but activation was done by the parent entity (e.g.CChainLocksHandler). Further complicating the relationship is the introduction of a network entity (e.g.NetInstantSend) which could cause potential lifecycle issues.To simpilfy this, the entity that is responsible for creation of the signer is also responsible for registration of the signer. The functions have also been renamed to follow the convention in
CSigSharesManagerto{Unr,R}egisterAsRecoveredSigsListener()as it more descriptive as opposed toStart/Stop(), which is associated with spawning worker entities.CDKGSessionManageris a dormant entity if the node is not in watch-only mode or masternode mode (source), to allow moving them to contexts that are mode-specific (in order to keepLLMQContextlimited to only chainstate validation logic), it is now a connectable entity in the style of dash#5980 (a5be37c).ChainstateManagerwhen constructingCMNHFManagerso we can partially revert the change introduced in dash#6078 (208b1c0).As subsequent PRs will change the ctor arguments for
PeerManagermultiple times, a helper has been added in the form ofMakePeerManager()to contain the changes to the helper.Currently,
CJWalletManageris initialised ifCActiveMasternodeManageris not initialised. This is fine until we moveCActiveMasternodeManagerintoActiveContextasActiveContextis initialised afterCJWalletManagerand cannot be moved upward due to hard dependencies.As there's no reason that
CJWalletManagerneeds to be initialised early, we can move it down instead.Breaking Changes
None expected.
Checklist