Skip to content

Conversation

@knst
Copy link
Collaborator

@knst knst commented Jan 14, 2026

Issue being fixed or feature implemented

This PR:

  • fixes the only remained check in headers, i.e., modernize-use-default-member-init
  • forces clang-tidy check all headers

What was done?

It's more than 100 failures for dash specific code; it is extracted from #7100 to its own PR to validate clang-tidy failures and address them one-by-one.

How Has This Been Tested?

Run CI & fix failures

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

@knst knst added this to the 23.1 milestone Jan 14, 2026
@github-actions
Copy link

github-actions bot commented Jan 14, 2026

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Walkthrough

This PR moves many constructor member initializers into in-class brace default initializers across numerous files, removing redundant initializer-list entries. It updates RPCArg/RPCResult interfaces to use value/move semantics and adds a bool parameter to RPCArg::ToDescriptionString, introducing a new Optional enum alias. Several extern RecursiveMutex cs_main declarations were removed from headers and added in specific test/bench files. A HeaderFilterRegex entry was added to src/.clang-tidy. No functional algorithm changes beyond initialization timing and RPC help-string generation signatures.

Sequence Diagram(s)

sequenceDiagram
  participant RPCH as RPCHelpMan::ToString
  participant Sections as Sections::Push/Render
  participant RPCArg as RPCArg::ToDescriptionString
  participant Out as Output (help string)

  RPCH->>Sections: build sections for args
  Sections->>RPCArg: For each arg, call ToDescriptionString(is_named_arg)
  RPCArg-->>Sections: return description text
  Sections-->>RPCH: assembled section strings
  RPCH-->>Out: final help string
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main changes: backporting clang-tidy header checks and related modernize-use-default-member-init fixes.
Description check ✅ Passed The description is directly related to the changeset, explaining what was fixed (clang-tidy header checks), how it was tested (CI runs), and the scope of changes.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a380a6 and 21192c0.

📒 Files selected for processing (39)
  • src/.clang-tidy
  • src/bench/mempool_eviction.cpp
  • src/bench/rpc_mempool.cpp
  • src/coinjoin/coinjoin.h
  • src/crypto/ripemd160.cpp
  • src/crypto/ripemd160.h
  • src/crypto/sha1.cpp
  • src/crypto/sha1.h
  • src/crypto/sha256.cpp
  • src/crypto/sha256.h
  • src/crypto/sha512.cpp
  • src/crypto/sha512.h
  • src/cuckoocache.h
  • src/dbwrapper.h
  • src/key.h
  • src/node/blockstorage.h
  • src/policy/fees.cpp
  • src/policy/fees.h
  • src/serialize.h
  • src/span.h
  • src/streams.h
  • src/support/lockedpool.cpp
  • src/support/lockedpool.h
  • src/test/blockencodings_tests.cpp
  • src/test/mempool_tests.cpp
  • src/test/policyestimator_tests.cpp
  • src/test/util/net.h
  • src/tinyformat.h
  • src/torcontrol.cpp
  • src/torcontrol.h
  • src/txmempool.h
  • src/univalue/include/univalue_utffilter.h
  • src/wallet/bdb.cpp
  • src/wallet/bdb.h
  • src/wallet/db.h
  • src/wallet/wallet.h
  • src/zmq/zmqabstractnotifier.h
  • src/zmq/zmqnotificationinterface.cpp
  • src/zmq/zmqnotificationinterface.h
💤 Files with no reviewable changes (4)
  • src/node/blockstorage.h
  • src/txmempool.h
  • src/policy/fees.cpp
  • src/coinjoin/coinjoin.h
🚧 Files skipped from review as they are similar to previous changes (16)
  • src/crypto/sha1.h
  • src/wallet/bdb.h
  • src/crypto/ripemd160.h
  • src/policy/fees.h
  • src/wallet/wallet.h
  • src/dbwrapper.h
  • src/.clang-tidy
  • src/zmq/zmqnotificationinterface.cpp
  • src/wallet/db.h
  • src/tinyformat.h
  • src/zmq/zmqabstractnotifier.h
  • src/crypto/sha512.h
  • src/crypto/sha256.h
  • src/crypto/sha1.cpp
  • src/cuckoocache.h
  • src/span.h
🧰 Additional context used
📓 Path-based instructions (5)
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/bench/mempool_eviction.cpp
  • src/test/policyestimator_tests.cpp
  • src/test/blockencodings_tests.cpp
  • src/support/lockedpool.h
  • src/test/mempool_tests.cpp
  • src/serialize.h
  • src/crypto/sha512.cpp
  • src/torcontrol.cpp
  • src/torcontrol.h
  • src/wallet/bdb.cpp
  • src/crypto/sha256.cpp
  • src/streams.h
  • src/zmq/zmqnotificationinterface.h
  • src/key.h
  • src/support/lockedpool.cpp
  • src/univalue/include/univalue_utffilter.h
  • src/crypto/ripemd160.cpp
  • src/bench/rpc_mempool.cpp
  • src/test/util/net.h
src/bench/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • src/bench/mempool_eviction.cpp
  • src/bench/rpc_mempool.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/policyestimator_tests.cpp
  • src/test/blockencodings_tests.cpp
  • src/test/mempool_tests.cpp
  • src/test/util/net.h
src/wallet/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

  • src/wallet/bdb.cpp
src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Do not make changes to vendored dependencies: src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue} or src/crypto/{ctaes,x11}

Files:

  • src/univalue/include/univalue_utffilter.h
🧠 Learnings (12)
📓 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: 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.
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.
Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like `#6786`, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding 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: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.
📚 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/bench/mempool_eviction.cpp
  • src/test/policyestimator_tests.cpp
  • src/test/blockencodings_tests.cpp
  • src/test/mempool_tests.cpp
  • src/bench/rpc_mempool.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/bench/mempool_eviction.cpp
  • src/test/blockencodings_tests.cpp
  • src/test/mempool_tests.cpp
  • src/bench/rpc_mempool.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.

Applied to files:

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

Applied to files:

  • src/support/lockedpool.h
  • src/serialize.h
  • src/torcontrol.h
  • src/streams.h
  • src/zmq/zmqnotificationinterface.h
  • src/key.h
  • src/univalue/include/univalue_utffilter.h
  • src/test/util/net.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/mempool_tests.cpp
📚 Learning: 2025-01-14T09:06:19.717Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/test/fuzz/netaddress.cpp:83-84
Timestamp: 2025-01-14T09:06:19.717Z
Learning: In fuzzer harness tests, CServiceHash can be used with both default constructor (CServiceHash()) and parameterized constructor (CServiceHash(salt_k0, salt_k1)) to test different variants. The usage pattern CServiceHash()(service) and CServiceHash(0, 0)(service) is valid and intentional in such tests.

Applied to files:

  • src/crypto/sha512.cpp
  • src/crypto/sha256.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/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

  • src/wallet/bdb.cpp
📚 Learning: 2025-01-14T09:05:12.032Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6532
File: src/test/util/net.h:204-204
Timestamp: 2025-01-14T09:05:12.032Z
Learning: The SetNonBlocking() method in Sock and its derived classes is correctly marked const as it doesn't modify the socket's internal state, only its blocking mode configuration.

Applied to files:

  • src/test/util/net.h
🧬 Code graph analysis (6)
src/serialize.h (1)
src/chain.h (1)
  • nSize (44-44)
src/crypto/sha512.cpp (1)
src/crypto/sha512.h (1)
  • CSHA512 (12-27)
src/crypto/sha256.cpp (1)
src/crypto/sha256.h (1)
  • CSHA256 (13-27)
src/key.h (1)
src/bls/bls.h (1)
  • fValid (53-120)
src/support/lockedpool.cpp (1)
src/support/lockedpool.h (2)
  • LockedPool (126-205)
  • LockedPageAllocator (19-43)
src/crypto/ripemd160.cpp (1)
src/crypto/ripemd160.h (1)
  • CRIPEMD160 (12-26)
🪛 Cppcheck (2.19.0)
src/bench/mempool_eviction.cpp

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

(syntaxError)

⏰ 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: win64-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: Lint / Run linters
🔇 Additional comments (19)
src/key.h (1)

51-68: LGTM!

The in-class default initializers for fValid and fCompressed correctly replace the previous constructor initializer list entries. This aligns with the modernize-use-default-member-init clang-tidy check and is consistent with similar patterns in the codebase (e.g., CBLSWrapper::fValid{false} in src/bls/bls.h).

src/test/util/net.h (1)

136-140: LGTM!

The m_consumed member now uses an in-class default initializer {0} instead of being initialized in the constructor's initializer list. Semantics are preserved, and the mutable qualifier is correctly retained for use in const member functions.

Also applies to: 227-227

src/torcontrol.h (1)

90-90: LGTM!

The b_conn pointer now uses an in-class default initializer {nullptr}, ensuring safe initialization regardless of constructor implementation. This is a defensive improvement aligned with the clang-tidy modernization.

src/zmq/zmqnotificationinterface.h (1)

48-48: LGTM!

The pcontext pointer now uses an in-class default initializer {nullptr}, ensuring safe initialization. This aligns with the project-wide pattern for pointer members noted in related ZMQ files.

src/serialize.h (1)

1350-1354: LGTM!

The nSize member now uses an in-class default initializer {0}, while nVersion correctly remains in the constructor's initializer list (as it depends on the constructor parameter). Semantics are preserved, and this aligns with similar patterns in the codebase (e.g., unsigned int nSize{} in src/chain.h).

src/crypto/sha256.cpp (1)

693-696: LGTM!

The constructor correctly relies on the in-class default member initializer uint64_t bytes{0} defined in the header. The sha256::Initialize(s) call properly initializes the state array. This is a standard modernize-use-default-member-init clang-tidy fix.

src/wallet/bdb.cpp (1)

321-330: LGTM!

The constructor correctly omits activeTxn(nullptr) from the initializer list, relying on the in-class default member initializer activeTxn{nullptr} in bdb.h. The initialization of pdb and m_cursor in the initializer list is consistent with the pattern, and pdb is properly reassigned in the constructor body.

src/crypto/sha512.cpp (1)

154-157: LGTM!

Consistent with the pattern applied to other crypto implementations (CSHA256, CSHA1, CRIPEMD160). The bytes member is initialized via the in-class default uint64_t bytes{0} in the header, and the state array is properly initialized by sha512::Initialize(s).

src/univalue/include/univalue_utffilter.h (1)

16-19: Vendored file modification in backport - acceptable.

This file is under src/univalue/, which is normally a vendored dependency where changes should be avoided. However, based on learnings, backported changes should match upstream even when they modify vendored code. The in-class initializers for is_valid{true}, codepoint{0}, state{0}, and surpair{0} follow the standard modernize-use-default-member-init pattern being applied across the codebase.

Also applies to: 82-85, 97-97

src/torcontrol.cpp (1)

66-69: LGTM!

The constructor correctly relies on the in-class default member initializer struct bufferevent* b_conn{nullptr} in torcontrol.h. The minor parameter formatting adjustment (struct event_base* _base) is consistent with the codebase style. All code paths properly handle b_conn being nullptr (destructor, Disconnect, Connect, Command).

src/crypto/ripemd160.cpp (1)

242-245: LGTM!

The constructor correctly delegates state array initialization to ripemd160::Initialize(s) and relies on the in-class default initializer for bytes{0} in the header. The buf array is intentionally left uninitialized as it's a working buffer that gets filled before being read.

src/streams.h (2)

627-628: LGTM!

Moving nSrcPos and m_read_pos initialization to in-class default initializers follows the modernize-use-default-member-init clang-tidy rule. This is semantically equivalent to the previous constructor initializer list approach.


673-679: LGTM!

The constructor correctly relies on the new in-class default initializers for nSrcPos and m_read_pos while continuing to explicitly initialize other members that require constructor arguments.

src/support/lockedpool.cpp (1)

284-287: LGTM!

The cumulative_bytes_locked member now uses an in-class default initializer ({0} in the header), making the constructor initializer list entry redundant. Clean refactor following clang-tidy's modernize-use-default-member-init rule.

src/bench/mempool_eviction.cpp (1)

10-10: Acceptable for backport consistency.

The extern RecursiveMutex cs_main; declaration enables the EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) annotation on AddTx. While including validation.h would reference the canonical declaration, this local extern pattern is consistent with the upstream backport and other files in this PR.

The Cppcheck error is a false positive—RecursiveMutex is properly defined through the transitive includes.

src/test/mempool_tests.cpp (1)

16-17: LGTM!

The extern RecursiveMutex cs_main; declaration correctly provides access to the global synchronization primitive defined in validation.cpp. This is necessary after the extern declaration was removed from headers like txmempool.h, and aligns with the pattern used in other test/bench files in this PR.

src/test/policyestimator_tests.cpp (1)

16-17: LGTM!

The extern declaration for cs_main is correctly added to enable the LOCK2(cs_main, mpool.cs) usage in the test. This follows the same pattern as other test files in this backport.

src/bench/rpc_mempool.cpp (1)

13-13: LGTM!

The extern declaration for cs_main is correctly added to satisfy both the EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) annotation on AddTx and the LOCK2(cs_main, pool.cs) call in the benchmark function.

src/support/lockedpool.h (1)

201-201: LGTM!

The in-class default member initializer {0} for cumulative_bytes_locked is a modern C++ idiom that ensures proper zero-initialization. This addresses the modernize-use-default-member-init clang-tidy check and is consistent with the broader initialization modernization in this backport.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@knst knst changed the title Merge bitcoin/bitcoin#26705: clang-tidy: Fix modernize-use-default-member-init in headers and force to check all headers backport: Merge bitcoin/bitcoin#26705: clang-tidy: Fix modernize-use-default-member-init in headers and force to check all headers Jan 14, 2026
fanquake and others added 3 commits January 14, 2026 17:53
fad56f7 doc: Properly report optional RPC args (MarcoFalke)
fa09cb6 refactor: Introduce is_top_level_arg (MarcoFalke)
fab92a5 refactor: Remove const to fix performance-move-const-arg clang-tidy errors (MarcoFalke)

Pull request description:

  `OMITTED_NAMED_ARG` and `OMITTED` are a confusing burden:

  * It puts the burden on developers to pick the right one of the two
  * They can be interchanged without introducing a compile failure or other error
  * Picking the wrong one is leading to incorrect docs
  * They are redundant, because the correct one can already be determined by the surrounding type

  Fix all issues by making them an alias of each other; Pick the right one based on the outer type.

ACKs for top commit:
  fanquake:
    ACK fad56f7

Tree-SHA512: 6e7193a05a852ba1618a9cb3261220c7ad3282bc5595325c04437aa811f529a88e2851e9c7dbf9878389b8aa42e98f8817b7eb0260fbb9e123da0893cbae6ca2
@knst knst changed the title backport: Merge bitcoin/bitcoin#26705: clang-tidy: Fix modernize-use-default-member-init in headers and force to check all headers backport: bitcoin/bitcoin#26705, #26706 - clang-tidy: for all headers and related fixes Jan 14, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/wallet/hdchain.h (1)

6-10: Fix clang-format failure in this header (CI is currently blocked).
CI reports a clang-format diff at src/wallet/hdchain.h Line 9; please run the repo’s clang-format (or the same “diff format check” target) and update the file accordingly.

🤖 Fix all issues with AI agents
In `@src/test/blockencodings_tests.cpp`:
- Around line 16-17: Replace the local extern declaration "extern RecursiveMutex
cs_main;" in blockencodings_tests.cpp with the canonical declaration by adding
an `#include` for validation.h and removing the extern; specifically, remove the
local extern RecursiveMutex cs_main; and ensure the file includes validation.h
so cs_main (and its RecursiveMutex type) is referenced from the authoritative
declaration rather than a local extern.
🧹 Nitpick comments (1)
src/wallet/bdb.cpp (1)

321-330: Watch for redundant init: pdb(nullptr) may be unnecessary now that pdb{nullptr} exists in-class.
If clang-tidy is configured to warn on redundant member initializers, consider removing pdb(nullptr) from the ctor init-list and rely on the in-class default (keeping m_cursor(nullptr) and m_database(database) as-is).

Proposed tweak (only if clang-tidy complains)
-BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), m_cursor(nullptr), m_database(database)
+BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : m_cursor(nullptr), m_database(database)
 {
     database.AddRef();
     database.Open();
     fReadOnly = read_only;
     fFlushOnClose = fFlushOnCloseIn;
     env = database.env.get();
     pdb = database.m_db.get();
     strFile = fs::PathToString(database.m_filename);
 }
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c076746 and 6a380a6.

📒 Files selected for processing (40)
  • src/.clang-tidy
  • src/bls/bls.h
  • src/coinjoin/coinjoin.h
  • src/crypto/ripemd160.cpp
  • src/crypto/ripemd160.h
  • src/crypto/sha1.cpp
  • src/crypto/sha1.h
  • src/crypto/sha256.cpp
  • src/crypto/sha256.h
  • src/crypto/sha512.cpp
  • src/crypto/sha512.h
  • src/cuckoocache.h
  • src/dbwrapper.h
  • src/evo/mnhftx.h
  • src/key.h
  • src/node/blockstorage.h
  • src/policy/fees.cpp
  • src/policy/fees.h
  • src/rpc/util.cpp
  • src/rpc/util.h
  • src/serialize.h
  • src/span.h
  • src/streams.h
  • src/support/lockedpool.cpp
  • src/support/lockedpool.h
  • src/test/blockencodings_tests.cpp
  • src/test/util/net.h
  • src/tinyformat.h
  • src/torcontrol.cpp
  • src/torcontrol.h
  • src/txmempool.h
  • src/univalue/include/univalue_utffilter.h
  • src/wallet/bdb.cpp
  • src/wallet/bdb.h
  • src/wallet/db.h
  • src/wallet/hdchain.h
  • src/wallet/wallet.h
  • src/zmq/zmqabstractnotifier.h
  • src/zmq/zmqnotificationinterface.cpp
  • src/zmq/zmqnotificationinterface.h
💤 Files with no reviewable changes (4)
  • src/coinjoin/coinjoin.h
  • src/node/blockstorage.h
  • src/txmempool.h
  • src/policy/fees.cpp
✅ Files skipped from review due to trivial changes (2)
  • src/evo/mnhftx.h
  • src/bls/bls.h
🚧 Files skipped from review as they are similar to previous changes (18)
  • src/span.h
  • src/support/lockedpool.h
  • src/wallet/wallet.h
  • src/univalue/include/univalue_utffilter.h
  • src/crypto/sha1.h
  • src/support/lockedpool.cpp
  • src/cuckoocache.h
  • src/zmq/zmqnotificationinterface.h
  • src/torcontrol.cpp
  • src/crypto/sha512.cpp
  • src/tinyformat.h
  • src/crypto/ripemd160.h
  • src/wallet/db.h
  • src/serialize.h
  • src/key.h
  • src/crypto/sha256.h
  • src/crypto/sha1.cpp
  • src/test/util/net.h
🧰 Additional context used
📓 Path-based instructions (3)
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/policy/fees.h
  • src/wallet/hdchain.h
  • src/wallet/bdb.h
  • src/test/blockencodings_tests.cpp
  • src/zmq/zmqabstractnotifier.h
  • src/crypto/ripemd160.cpp
  • src/torcontrol.h
  • src/streams.h
  • src/wallet/bdb.cpp
  • src/rpc/util.cpp
  • src/crypto/sha512.h
  • src/crypto/sha256.cpp
  • src/dbwrapper.h
  • src/rpc/util.h
  • src/zmq/zmqnotificationinterface.cpp
src/wallet/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

  • src/wallet/hdchain.h
  • src/wallet/bdb.h
  • src/wallet/bdb.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/blockencodings_tests.cpp
🧠 Learnings (12)
📓 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: 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.
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.
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: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.
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.
📚 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/policy/fees.h
  • src/test/blockencodings_tests.cpp
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.

Applied to files:

  • src/policy/fees.h
  • src/wallet/hdchain.h
  • src/wallet/bdb.h
  • src/zmq/zmqabstractnotifier.h
  • src/torcontrol.h
  • src/streams.h
  • src/crypto/sha512.h
  • src/dbwrapper.h
  • src/rpc/util.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

  • src/wallet/hdchain.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/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

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

Applied to files:

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

Applied to files:

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

Applied to files:

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

Applied to files:

  • src/crypto/sha256.cpp
🧬 Code graph analysis (5)
src/zmq/zmqabstractnotifier.h (1)
src/zmq/zmqabstractnotifier.cpp (1)
  • CZMQAbstractNotifier (11-14)
src/crypto/ripemd160.cpp (1)
src/crypto/ripemd160.h (1)
  • CRIPEMD160 (12-26)
src/crypto/sha512.h (3)
src/crypto/ripemd160.h (1)
  • bytes (17-17)
src/crypto/sha1.h (1)
  • bytes (17-17)
src/crypto/sha256.h (1)
  • bytes (18-18)
src/dbwrapper.h (1)
src/wallet/bdb.cpp (2)
  • ssKey (492-492)
  • ssValue (493-493)
src/rpc/util.h (1)
src/rpc/util.cpp (6)
  • ToDescriptionString (503-518)
  • ToDescriptionString (503-503)
  • ToDescriptionString (520-523)
  • ToDescriptionString (520-520)
  • ToDescriptionString (655-716)
  • ToDescriptionString (655-655)
🪛 GitHub Actions: Clang Diff Format Check
src/wallet/hdchain.h

[error] 9-9: Clang-format differences detected. Update formatting (diff_output.txt) to align with clang-format rules.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: linux64_multiprocess-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: arm-linux-build / Build source
  • GitHub Check: mac-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
🔇 Additional comments (21)
src/crypto/sha512.h (1)

15-27: Good: default-initialize bytes to avoid indeterminate state and align with other hashers.
This matches the bytes{0} pattern already used in src/crypto/sha1.h, src/crypto/sha256.h, and src/crypto/ripemd160.h, and should satisfy modernize-use-default-member-init while keeping Size() behavior unchanged.

src/torcontrol.h (1)

88-92: Good: default-initialize b_conn in-class to satisfy clang-tidy and avoid redundant ctor init.
This aligns with the backport/clang-tidy objective and keeps behavior unchanged (still starts as nullptr).

src/zmq/zmqabstractnotifier.h (2)

38-38: Constructor init list simplification is behavior-preserving here.

Keeping outbound_message_high_water_mark(DEFAULT_ZMQ_SNDHWM) while moving psocket init in-class matches the clang-tidy goal and should remain equivalent for the default ctor.


81-81: Good: psocket now defaults to nullptr for all constructors.

This makes the destructor invariant (assert(!psocket) in src/zmq/zmqabstractnotifier.cpp) safer across future ctor additions and aligns with modernize-use-default-member-init.

src/dbwrapper.h (1)

82-83: Good clang-tidy-driven cleanup (default member init keeps behavior stable).
Moving size_estimate to an in-class {0} initializer and removing it from the ctor init-list keeps Clear() semantics intact and avoids future “forgot to init” footguns.

Also applies to: 88-94

src/policy/fees.h (3)

243-246: LGTM!

The in-class brace initializers follow the modernize-use-default-member-init clang-tidy check correctly. The syntax combining GUARDED_BY annotations with brace initialization is valid C++.


248-253: LGTM!

The struct member initializers correctly follow the modernize-use-default-member-init pattern. The empty constructor body is appropriate for a backport matching upstream Bitcoin Core.


263-264: LGTM!

Consistent application of in-class brace initialization for the tracking counters.

src/zmq/zmqnotificationinterface.cpp (1)

25-27: Constructor change is correct and safe.

Verification confirms pcontext has a default member initializer (void* pcontext{nullptr}; in the header, line 48), and there is only one constructor, which is now empty. This ensures pcontext is properly initialized to nullptr on construction, satisfying the assert(!pcontext) check in Initialize(). The removal of the explicit initializer list aligns with modern C++20 practices using default member initialization.

src/crypto/ripemd160.cpp (1)

242-245: Constructor now relies on in-class init for bytes (OK) and Initialize(s) for state (OK).

This matches the modernize-use-default-member-init pattern: s is fully initialized via ripemd160::Initialize(s) and bytes is already defaulted to 0 in src/crypto/ripemd160.h. buf stays uninitialized, but the current Write/Finalize flow only uses it after writing into it, so this should be safe. Based on learnings, keeping this aligned with upstream is preferred.

src/crypto/sha256.cpp (1)

693-696: The bytes member is properly zero-initialized in the header.

The header file (src/crypto/sha256.h, line 18) has uint64_t bytes{0};, confirming that bytes is zero-initialized via an in-class initializer. The constructor correctly relies on this for proper initialization.

src/streams.h (1)

626-679: LGTM: safe move to in-class default initialization for stream positions.
This aligns with modernize-use-default-member-init and keeps the constructor initializer list focused on non-trivial members.

src/wallet/bdb.h (1)

191-199: LGTM: pdb / activeTxn in-class nullptr initialization.
One follow-up: ensure clang-tidy config won’t flag redundant ctor init for members that now have in-class defaults (e.g. pdb(nullptr) in bdb.cpp).

src/wallet/hdchain.h (1)

12-21: No semantic shift risk found in practice.

The removal of the explicit constructor does technically make CHDAccount an aggregate, but a codebase search found no brace-initialization patterns like CHDAccount{x, y}. All existing call sites use either default initialization (CHDAccount acc;) or explicit default constructor calls (CHDAccount()), both of which remain valid. No behavior change is possible.

src/rpc/util.cpp (3)

596-604: ToString() now treats argument descriptions as “named” (Line 599) — confirm this matches expected RPC help text.

arg.ToDescriptionString(/*is_named_arg=*/true) will now append “, optional” for omitted named args; this is a user-visible doc output change.


655-716: Signature + semantics change in RPCArg::ToDescriptionString(bool) is fine; ensure is_named_arg is only set where appropriate.

In particular, OMITTED now conditionally emits “optional” only when is_named_arg is true (Lines 701-705). That’s subtle and easy to misapply at call sites.


357-409: Formatting logic is correct and properly handles help output for obj/arr vs scalar args.

The verification confirms the implementation matches the intended design:

  • Zero-arg calls to ToDescriptionString() found at lines 608 and 611 are on RPCResults and RPCExamples types, not RPCArg, so they are expected and correct.
  • Boolean-arg calls to RPCArg::ToDescriptionString(/*is_named_arg=*/...) are properly present at lines 379, 384, 399, and 599.
  • The is_top_level_arg early-return for scalar types (line 371) and conditional empty RHS for top-level obj/arr (lines 384, 399) correctly avoid duplicating descriptions already emitted by RPCHelpMan::ToString().

The code is ready as-is.

src/rpc/util.h (3)

188-225: RPCArg ctor + ToDescriptionString(bool) signature change: verify no stale declarations/usages remain.

Given the by-value ctor params and new ToDescriptionString(bool) API, compilation will catch most issues—but it’s worth confirming there aren’t any ambiguous/accidentally-reordered ctor call sites.

Also applies to: 249-250


276-324: RPCResult by-value constructors: OK; confirm call sites didn’t rely on implicit conversions.

Switching to by-value/move can change overload resolution (especially with {} initializer lists). Quick scan of construction sites is recommended.


156-178: Fallback refactor is well-designed for backward compatibility.

The Fallback = std::variant<Optional, DefaultHint, Default> change to RPCArg is sound. Existing call sites already pass all three variant member types (RPCArg::Optional::NO/OMITTED, RPCArg::Default{...}, RPCArg::DefaultHint{...}), and implicit variant conversion handles them transparently. RPCResult is unaffected by this change and requires no updates. The OMITTED_NAMED_ARG deprecation alias adds no breaking changes.

src/.clang-tidy (1)

29-29: The pattern HeaderFilterRegex: '.' correctly selects all headers—no change needed.

HeaderFilterRegex uses llvm::Regex, which performs substring matching by default. The pattern . matches any file path containing at least one character, which covers all normal headers. Your concern about full-string matching requiring 4-character patterns is not applicable here.

If preferred for clarity, .* is more idiomatic and equally effective, but the current pattern works as intended.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +16 to +17
extern RecursiveMutex cs_main;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for canonical cs_main declaration (non-extern)
rg -n '^\s*RecursiveMutex\s+cs_main' --type cpp --type h

Repository: dashpay/dash

Length of output: 102


🏁 Script executed:

# Search for where cs_main is declared in headers
rg -n 'RecursiveMutex\s+cs_main' --type h

Repository: dashpay/dash

Length of output: 773


🏁 Script executed:

# Check current includes in blockencodings_tests.cpp
head -30 src/test/blockencodings_tests.cpp

Repository: dashpay/dash

Length of output: 855


🏁 Script executed:

# Search for other test files using extern RecursiveMutex cs_main
rg -l 'extern\s+RecursiveMutex\s+cs_main' src/test --type cpp

Repository: dashpay/dash

Length of output: 89


🏁 Script executed:

# Check if validation.h includes or declares cs_main
rg -n 'cs_main' src/validation.h | head -20

Repository: dashpay/dash

Length of output: 1357


🏁 Script executed:

# Check what setup_common.h includes
head -50 src/test/util/setup_common.h

Repository: dashpay/dash

Length of output: 1479


🏁 Script executed:

# Check if setup_common.h already includes validation.h (transitively)
rg 'validation\.h' src/test/util/setup_common.h

Repository: dashpay/dash

Length of output: 38


🏁 Script executed:

# Verify no circular dependency would occur by checking validation.h's includes
rg '#include.*test.*setup_common' src/validation.h

Repository: dashpay/dash

Length of output: 38


Include validation.h to reference the canonical cs_main declaration instead of using a local extern statement

The file currently relies on a local extern RecursiveMutex cs_main; declaration at lines 16-17, but the canonical declaration is in src/validation.h:115. Following the codebase convention (used in all other headers and implementations), include validation.h directly and remove the local extern. This avoids brittle signature drift if the canonical declaration changes.

Recommended fix
 `#include` <blockencodings.h>
 `#include` <chainparams.h>
 `#include` <consensus/merkle.h>
 `#include` <pow.h>
 `#include` <streams.h>
 `#include` <test/util/txmempool.h>
+#include <validation.h>
 
 `#include` <test/util/setup_common.h>
 
 `#include` <boost/test/unit_test.hpp>
 
-extern RecursiveMutex cs_main;
🤖 Prompt for AI Agents
In `@src/test/blockencodings_tests.cpp` around lines 16 - 17, Replace the local
extern declaration "extern RecursiveMutex cs_main;" in blockencodings_tests.cpp
with the canonical declaration by adding an `#include` for validation.h and
removing the extern; specifically, remove the local extern RecursiveMutex
cs_main; and ensure the file includes validation.h so cs_main (and its
RecursiveMutex type) is referenced from the authoritative declaration rather
than a local extern.

@UdjinM6
Copy link

UdjinM6 commented Jan 14, 2026

build fails

knst and others added 2 commits January 14, 2026 23:39
…it` in headers and force to check all headers

b0e9169 clang-tidy: Force to check all headers (Hennadii Stepanov)
96ee992 clang-tidy: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov)

Pull request description:

  This PR:
  - fixes the only [remained](bitcoin#26705 (comment)) check in headers, i.e., `modernize-use-default-member-init`
  - forces `clang-tidy` check all headers

  Closes bitcoin#26703.

ACKs for top commit:
  MarcoFalke:
    review ACK b0e9169 🍹

Tree-SHA512: 4d33fe873094914541ae81968cdb4e7a7a01b3fdd4f25bc6daa8a53f45dab80565a5b3607ddc338f122369ca5a0a2d0d09c8e78cabe1beb6bd50c115bc5c5210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants