refactor: use iwyu analyzer to break circular dependencies #6743
Conversation
knst
changed the title
|
This pull request has conflicts, please rebase. |
These unused headers has been found by running iwyu linter
It improves compilation time insignificantly (1minute out of 98minutes, assuming 1 core compilation, 1% improvement)
|
knst
requested review from
kwvg,
UdjinM6 and
PastaPastaPasta
and removed request for
kwvg
WalkthroughThis set of changes consists of widespread cleanup and reorganization of header file inclusions across numerous source and header files in the codebase. The modifications primarily involve removing unused or redundant Estimated code review effort1 (~8 minutes) ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/evo/mnauth.h (1)
8-14: Optional: align include order with project conventions
If we follow the usual “STL first, project headers after” ordering, consider moving<string_view>above the project-specific headers for consistency:-#include <bls/bls.h> -#include <protocol.h> -#include <serialize.h> -#include <uint256.h> -#include <string_view> +#include <string_view> +#include <bls/bls.h> +#include <protocol.h> +#include <serialize.h> +#include <uint256.h>Purely cosmetic; feel free to ignore if the existing convention differs.
src/stats/client.cpp (1)
9-9: Verify include path & redundancy forlogging.h.
logging.his a project header; most files include it with quoted form ("logging.h"). Using the angle-bracket form may pick up an unintended copy if the build system’s include paths ever shadow the project root. Consider switching to quoted form for consistency and to avoid surprises:-#include <logging.h> +#include "logging.h"Also double-check that both
logging.handutil/system.hare strictly required here (the latter is presumably forArgsManager). If either is transitively included by the other after this PR’s header reshuffle, drop the redundant include to keep the dependency graph minimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
src/bls/bls_worker.cpp(0 hunks)src/coinjoin/client.cpp(1 hunks)src/coinjoin/coinjoin.cpp(0 hunks)src/coinjoin/interfaces.cpp(0 hunks)src/coinjoin/server.cpp(0 hunks)src/coinjoin/util.cpp(0 hunks)src/dsnotificationinterface.cpp(0 hunks)src/evo/chainhelper.cpp(1 hunks)src/evo/deterministicmns.cpp(0 hunks)src/evo/dmnstate.cpp(0 hunks)src/evo/mnauth.cpp(0 hunks)src/evo/mnauth.h(1 hunks)src/evo/simplifiedmns.cpp(0 hunks)src/evo/specialtxman.cpp(0 hunks)src/governance/classes.cpp(0 hunks)src/governance/classes.h(1 hunks)src/governance/common.cpp(0 hunks)src/governance/exceptions.cpp(0 hunks)src/governance/governance.cpp(0 hunks)src/governance/object.cpp(0 hunks)src/governance/vote.cpp(1 hunks)src/llmq/chainlocks.cpp(1 hunks)src/llmq/commitment.h(1 hunks)src/llmq/context.cpp(1 hunks)src/llmq/dkgsession.cpp(1 hunks)src/llmq/dkgsessionmgr.cpp(0 hunks)src/llmq/ehf_signals.cpp(1 hunks)src/llmq/quorums.cpp(1 hunks)src/llmq/signing.cpp(0 hunks)src/llmq/snapshot.cpp(1 hunks)src/llmq/utils.cpp(1 hunks)src/logging.cpp(0 hunks)src/masternode/meta.cpp(0 hunks)src/masternode/node.cpp(0 hunks)src/masternode/payments.cpp(0 hunks)src/masternode/sync.cpp(0 hunks)src/masternode/utils.cpp(0 hunks)src/netfulfilledman.cpp(0 hunks)src/qt/governancelist.cpp(0 hunks)src/qt/guiutil.cpp(0 hunks)src/qt/masternodelist.cpp(0 hunks)src/rpc/coinjoin.cpp(0 hunks)src/rpc/evo.cpp(0 hunks)src/rpc/governance.cpp(0 hunks)src/rpc/masternode.cpp(0 hunks)src/rpc/quorums.cpp(0 hunks)src/saltedhasher.cpp(0 hunks)src/spork.cpp(0 hunks)src/stats/client.cpp(1 hunks)src/test/block_reward_reallocation_tests.cpp(0 hunks)src/test/dynamic_activation_thresholds_tests.cpp(0 hunks)src/test/evo_deterministicmns_tests.cpp(0 hunks)src/test/evo_mnhf_tests.cpp(0 hunks)src/test/evo_utils_tests.cpp(1 hunks)src/wallet/hdchain.cpp(0 hunks)src/wallet/wallet.cpp(0 hunks)test/lint/lint-circular-dependencies.py(0 hunks)
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/governance/vote.cppsrc/llmq/quorums.cppsrc/llmq/utils.cppsrc/llmq/commitment.hsrc/test/evo_utils_tests.cppsrc/llmq/chainlocks.cppsrc/evo/chainhelper.cppsrc/coinjoin/client.cppsrc/llmq/dkgsession.cppsrc/evo/mnauth.hsrc/stats/client.cppsrc/llmq/ehf_signals.cppsrc/llmq/context.cppsrc/governance/classes.hsrc/llmq/snapshot.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/evo_utils_tests.cpp
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/chainhelper.cppsrc/evo/mnauth.h
🧠 Learnings (14)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#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
PR: dashpay/dash#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: knst
PR: dashpay/dash#6533
File: test/functional/feature_llmq_singlenode.py:98-106
Timestamp: 2025-01-22T08:33:31.405Z
Learning: When reviewing PRs, ensure that suggestions are directly related to the PR's primary objectives rather than general code improvements that could be addressed separately.
src/governance/vote.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
src/llmq/quorums.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: knst
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.
src/llmq/utils.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/test/evo_utils_tests.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/llmq/chainlocks.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the CInstantSendManager. Previously, this was handled globally; now it's handled as a class member in LLMQContext, but the concurrency control remains consistent.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/evo/chainhelper.cpp (4)
Learnt from: kwvg
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
src/coinjoin/client.cpp (5)
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Learnt from: kwvg
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
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/llmq/dkgsession.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/evo/mnauth.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/llmq/ehf_signals.cpp (5)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/is_basic_scheme_active=/true, /is_extended_addr=/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
src/llmq/context.cpp (2)
Learnt from: kwvg
PR: #6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the CInstantSendManager. Previously, this was handled globally; now it's handled as a class member in LLMQContext, but the concurrency control remains consistent.
Learnt from: kwvg
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.
src/governance/classes.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/llmq/snapshot.cpp (5)
Learnt from: kwvg
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
💤 Files with no reviewable changes (42)
- src/coinjoin/interfaces.cpp
- src/rpc/coinjoin.cpp
- src/logging.cpp
- src/evo/specialtxman.cpp
- src/masternode/utils.cpp
- src/governance/classes.cpp
- src/evo/deterministicmns.cpp
- src/rpc/quorums.cpp
- src/test/evo_mnhf_tests.cpp
- src/governance/governance.cpp
- src/qt/governancelist.cpp
- src/test/evo_deterministicmns_tests.cpp
- src/netfulfilledman.cpp
- src/evo/simplifiedmns.cpp
- src/coinjoin/util.cpp
- src/llmq/signing.cpp
- src/qt/masternodelist.cpp
- src/coinjoin/server.cpp
- src/governance/object.cpp
- src/masternode/payments.cpp
- src/wallet/hdchain.cpp
- src/dsnotificationinterface.cpp
- src/governance/exceptions.cpp
- src/evo/dmnstate.cpp
- src/llmq/dkgsessionmgr.cpp
- test/lint/lint-circular-dependencies.py
- src/masternode/sync.cpp
- src/rpc/masternode.cpp
- src/coinjoin/coinjoin.cpp
- src/rpc/governance.cpp
- src/evo/mnauth.cpp
- src/masternode/node.cpp
- src/governance/common.cpp
- src/test/dynamic_activation_thresholds_tests.cpp
- src/rpc/evo.cpp
- src/saltedhasher.cpp
- src/qt/guiutil.cpp
- src/wallet/wallet.cpp
- src/masternode/meta.cpp
- src/bls/bls_worker.cpp
- src/test/block_reward_reallocation_tests.cpp
- src/spork.cpp
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/governance/vote.cppsrc/llmq/quorums.cppsrc/llmq/utils.cppsrc/llmq/commitment.hsrc/test/evo_utils_tests.cppsrc/llmq/chainlocks.cppsrc/evo/chainhelper.cppsrc/coinjoin/client.cppsrc/llmq/dkgsession.cppsrc/evo/mnauth.hsrc/stats/client.cppsrc/llmq/ehf_signals.cppsrc/llmq/context.cppsrc/governance/classes.hsrc/llmq/snapshot.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/evo_utils_tests.cpp
src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Masternode lists must use immutable data structures (Immer library) for thread safety
Files:
src/evo/chainhelper.cppsrc/evo/mnauth.h
🧠 Learnings (14)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#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
PR: dashpay/dash#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: knst
PR: dashpay/dash#6533
File: test/functional/feature_llmq_singlenode.py:98-106
Timestamp: 2025-01-22T08:33:31.405Z
Learning: When reviewing PRs, ensure that suggestions are directly related to the PR's primary objectives rather than general code improvements that could be addressed separately.
src/governance/vote.cpp (2)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
src/llmq/quorums.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: knst
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.
src/llmq/utils.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/test/evo_utils_tests.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Learnt from: kwvg
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/llmq/chainlocks.cpp (4)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the CInstantSendManager. Previously, this was handled globally; now it's handled as a class member in LLMQContext, but the concurrency control remains consistent.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/evo/chainhelper.cpp (4)
Learnt from: kwvg
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: kwvg
PR: #6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
src/coinjoin/client.cpp (5)
Learnt from: kwvg
PR: #6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The GetWallet() function calls in src/wallet/rpcwallet.cpp are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Learnt from: kwvg
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
PR: #6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/llmq/dkgsession.cpp (3)
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/evo/mnauth.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
src/llmq/ehf_signals.cpp (5)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
Learnt from: knst
PR: #6691
File: src/test/llmq_params_tests.cpp:148-151
Timestamp: 2025-07-15T14:53:04.819Z
Learning: In the Dash Core LLMQ implementation, signingActiveQuorumCount is never 0 in the actual parameters defined in params.h, making division by zero scenarios unrealistic in the max_cycles() function.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: kwvg
PR: #6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/is_basic_scheme_active=/true, /is_extended_addr=/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.
src/llmq/context.cpp (2)
Learnt from: kwvg
PR: #6504
File: src/llmq/context.cpp:42-43
Timestamp: 2025-01-02T21:50:00.967Z
Learning: LLMQContext manages concurrency for the CInstantSendManager. Previously, this was handled globally; now it's handled as a class member in LLMQContext, but the concurrency control remains consistent.
Learnt from: kwvg
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.
src/governance/classes.h (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
src/llmq/snapshot.cpp (5)
Learnt from: kwvg
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: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : Masternode lists must use immutable data structures (Immer library) for thread safety
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/evo/specialtx.h : Special transactions use payload extensions defined in src/evo/specialtx.h
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue}/** : Do not make changes under any circumstances to vendored dependencies in src/crc32c, src/dashbls, src/gsl, src/immer, src/leveldb, src/minisketch, src/secp256k1, src/univalue
Learnt from: knst
PR: #6511
File: src/evo/deterministicmns.cpp:1369-1373
Timestamp: 2025-01-07T18:50:44.838Z
Learning: The functions MigrateDBIfNeeded and MigrateDBIfNeeded2 in src/evo/deterministicmns.cpp are temporary and will be removed in a future version. Refactoring suggestions for these functions should be avoided.
🔇 Additional comments (16)
src/evo/mnauth.h (1)
11-14: Necessary headers added – compile-time fix is correct
uint256andstd::string_vieware used in this header, so explicitly including<uint256.h>and<string_view>is the right move. No further action required.src/governance/classes.h (1)
13-15: Header is now self-contained; good changeAdding
<memory>,<string>, and<vector>makes the header compile-unit safe without relying on transitive includes.src/governance/vote.cpp (1)
9-10: Explicit dependency inclusion aligns with IWYUIncluding both
<evo/deterministicmns.h>and<evo/dmn_types.h>clarifies the file’s direct dependencies and prevents future breakage if transitive includes change.src/evo/chainhelper.cpp (1)
7-7: LGTM: Header include cleanup aligns with iwyu recommendations.The replacement of
consensus/params.hwithchainparams.his appropriate as part of the systematic include cleanup. The file's functionality remains intact while reducing dependencies.src/coinjoin/client.cpp (1)
27-27: LGTM: Improved header alignment and circular dependency removal.The replacement of
wallet/fees.hwithwallet/coinselection.hbetter reflects the file's actual dependencies, which focus on coin selection functionality. This change also contributes to breaking circular dependencies between wallet components.src/test/evo_utils_tests.cpp (1)
8-8: LGTM: Include reordering improves dependency clarity.Moving
llmq/options.hafterchainparams.hcreates a more logical include order, as LLMQ functionality may depend on chain parameters. This reordering enhances compilation clarity without affecting functionality.src/llmq/snapshot.cpp (1)
7-7: LGTM: Explicit include addition improves dependency clarity.Adding
chainparams.his appropriate since the file extensively usesParams()functionality throughout (lines 135, 138, 394, etc.). Making this dependency explicit rather than relying on transitive includes follows iwyu best practices and improves code maintainability.src/llmq/chainlocks.cpp (2)
11-11: Good catch: required include forCInstantSendManageradded.The new
#include <instantsend/instantsend.h>resolves the previously implicit dependency.
No further action needed.
6-7: chainlocks.h safely forward-declares CSigSharesManager; no member access foundAll references to
CSigSharesManagerinchainlocks.handchainlocks.cppuse it by reference or pointer, and there are noshareman.*orshareman->*calls. The header only forward-declares the class and doesn’t includesigning_shares.h, so dropping that include is safe.• src/llmq/chainlocks.h line 36:
class CSigSharesManager;(forward declaration)
• src/llmq/chainlocks.h line 51:CSigSharesManager& shareman;(no member access in header)
• src/llmq/chainlocks.cpp line 43 & 89: only parameter references, noshareman.usagesrc/llmq/ehf_signals.cpp (2)
12-13: Include realignment looks correct.
<evo/mnhftx.h>is now pulled in, giving a direct definition forMNHFTxPayload; the removed headers are unused in this TU. Build should remain green.
5-8: ehf_signals.cpp – Nosharemanreferences detected, no action required
Ranrg -n '\bshareman\.' src/llmq/ehf_signals.cppand found no matches, so there’s noCSigSharesManager(shareman) usage in this file. The previous caution can be safely removed.src/llmq/commitment.h (1)
18-18:<algorithm>inclusion is required – thumbs-up.
std::countis used at lines 63 & 67; adding<algorithm>prevents implicit-use warnings on modern compilers.src/llmq/utils.cpp (1)
10-10: BLS header added, orphan include removed – looks good.
CBLSSignatureforward-decl remains, but the concrete definitions come from the new<bls/bls.h>include.
No references to EvoDB remain, so dropping that header is safe.src/llmq/dkgsession.cpp (1)
16-25: Include list trimmed correctly – just re-confirm compilation succeeds
<evo/deterministicmns.h>is still required and<array>is now explicitly included for thestd::arrayat line 39 – good catch.
Please kick off a full build (or at leastninja src/llmq/dkgsession.cpp) to ensure nothing in this TU still relied on the now-removed<evo/specialtx.h>,<netmessagemaker.h>or<univalue.h>.src/llmq/context.cpp (1)
5-9: Explicit BLS dependency added – verifies hidden include
<bls/bls_worker.h>brings in the types required for the newstd::shared_ptr<CBLSWorker>at line 25. With the old transitive include gone, this keeps the TU self-contained.
No functional impact; just make sure CI still passes after the header shuffle.Also applies to: 18-18
src/llmq/quorums.cpp (1)
13-18: Header cleanup & BLS additions LGTMSwitching to
<bls/bls.h>/<bls/bls_ies.h>and dropping the indirect includes removes the circular edge the IWYU run flagged. The file already usesCBLSSecretKey,CBLSIESEncryptedObject, etc., so the direct includes are correct.As a sanity check, verify that nothing in this compilation unit still needs the previously pulled-in
<llmq/dkgsession.h>or<univalue.h>.
Issue being fixed or feature implemented
Some includes are leftover after code movements, refactors or feature edits.
Some includes meant to be, but included indirectly.
What was done?
I run iwyu [include what you use] analyzer and got 15k lines diff which touches 800 files. It's a bit too big diff and not quite useful! See full set of changes here: https://github.com/knst/dash/tree/refactor-iwyu-all
Though, it helped to remove 11 exception for circular dependencies linter, so, this PR has a small sub-set of original changes that are split to 2 commits:
How Has This Been Tested?
Here're steps to run iwyu analyser:
See test/lint/lint-circular-dependencies.py, it removed 10 exception for circular dependencies.
Originally has been 11 of them, but one has been found before manually, and merged as #6728, so, 10 only left for this PR.
Compilation time is reduced insignificantly, only 1 minute out of 98 minutes (assuming single core compilation).
Breaking Changes
N/A
Checklist: