backport: bitcoin#25526, #26067, #26109, #26142, #26277, #26286, #26313, #26675, #27177, #32765

[knst]

What was done?

Regular backports from Bitcoin Core v25 + extra backport-fix for feature_bip68_sequence.py.

How Has This Been Tested?

Run unit & functional tests

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

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

Coordinated edits across wallet, networking, tests, build, docs, and tooling. Key factual changes: bitcoin-wallet init now returns std::optional instead of bool; several keypool TopUp calls moved or removed in scriptpubkeyman/wallet; net_processing now explicitly initializes peer TxRelay state and propagates relay flags; test utilities add test/util/txmempool.{h,cpp} and move TestMemPoolEntryHelper FromTx implementations there with many tests updated to include the header; test framework wallet selects mature UTXOs by block height; Makefile source grouping expanded; minor UI/doc wording edits; contrib/devtools/gen-manpages.py tightened subprocess.run to use check=True.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–75 minutes

Areas needing extra attention:

• src/bitcoin-wallet.cpp: new std::optional return semantics and callers handling that signal.
• src/wallet/{scriptpubkeyman.cpp,wallet.cpp,wallet.h}: keypool TopUp relocation/removal and removed nAccountingEntryNumber.
• src/net_processing.cpp: explicit TxRelay initialization and propagation to peer objects.
• test/util/txmempool.{h,cpp} and src/test/util/setup_common.{h,cpp}: API movement for TestMemPoolEntryHelper and impact on tests.
• src/Makefile.am and src/Makefile.test_util.include: large source list/grouping changes affecting build.
• Functional tests and test_framework/wallet: UTXO maturity change and script/test adjustments (feature_bip68_sequence, mempool tests).

Possibly related PRs

• backport: merge bitcoin#24587, #24637, #24623, #23075, #24817, #25087, #24839, #20456, #26640, #26892, #26280, #26657, #26923, partial bitcoin#19937 (test backports: part 3) #6726 — Modifies test/functional/feature_bip68_sequence; likely related test-area changes.
• backport: merge bitcoin#25218, #25721, #25656, #25616, #26005, #24855, #18554, #17204, #20562, #21166, #25044, #25364, #25525, #25512, #24678, #26747, partial bitcoin#22154, #24584 (wallet backports: part 8) #6823 — Touches wallet subsystem and test utilities (TestMemPoolEntryHelper), strongly related to the test util reorganization here.

Suggested reviewers

• UdjinM6
• PastaPastaPasta
• kwvg

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title lists Bitcoin Core PR numbers that were backported, accurately reflecting the changeset's main purpose of applying upstream backports.
Description check	✅ Passed	The PR description is clearly related to the changeset, describing Bitcoin Core v25 backports and an additional backport-fix for a test file.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/rpc/net.cpp (1)

127-139: Minor redundancy around fStateStats guard for relaytxes

Inside the for (const CNodeStats& stats : vstats) loop, fStateStats is already checked and continued on when false (Line 204), so the additional if (fStateStats) around obj.pushKV("relaytxes", ...) is redundant. It’s harmless, but if you ever touch this code outside of strict backports, you could simplify by dropping the inner if and writing relaytxes unconditionally within this block.

Otherwise, the updated help text and field wiring remain consistent with the existing data flow.

Also applies to: 222-247

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 199ac79 and 46e4bfb.

📒 Files selected for processing (30)

• contrib/devtools/gen-manpages.py (1 hunks)
• doc/design/libraries.md (1 hunks)
• src/Makefile.am (8 hunks)
• src/Makefile.test_util.include (2 hunks)
• src/bitcoin-wallet.cpp (3 hunks)
• src/init.cpp (1 hunks)
• src/net_processing.cpp (2 hunks)
• src/qt/forms/optionsdialog.ui (1 hunks)
• src/qt/optionsdialog.cpp (1 hunks)
• src/rpc/net.cpp (3 hunks)
• src/test/blockencodings_tests.cpp (1 hunks)
• src/test/evo_deterministicmns_tests.cpp (1 hunks)
• src/test/fuzz/tx_pool.cpp (1 hunks)
• src/test/fuzz/validation_load_mempool.cpp (1 hunks)
• src/test/mempool_tests.cpp (1 hunks)
• src/test/miner_tests.cpp (1 hunks)
• src/test/policyestimator_tests.cpp (1 hunks)
• src/test/sigopcount_tests.cpp (1 hunks)
• src/test/util/setup_common.cpp (1 hunks)
• src/test/util/setup_common.h (2 hunks)
• src/test/util/txmempool.cpp (1 hunks)
• src/test/util/txmempool.h (1 hunks)
• src/test/util_tests.cpp (1 hunks)
• src/wallet/scriptpubkeyman.cpp (3 hunks)
• src/wallet/wallet.cpp (2 hunks)
• src/wallet/wallet.h (0 hunks)
• test/functional/feature_bip68_sequence.py (10 hunks)
• test/functional/mempool_compatibility.py (1 hunks)
• test/functional/mempool_persist.py (1 hunks)
• test/functional/test_framework/wallet.py (2 hunks)

💤 Files with no reviewable changes (1)

• src/wallet/wallet.h

🧰 Additional context used

📓 Path-based instructions (6)

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/qt/optionsdialog.cpp
• src/test/evo_deterministicmns_tests.cpp
• src/test/util_tests.cpp
• src/init.cpp
• src/test/util/setup_common.h
• src/test/fuzz/tx_pool.cpp
• src/wallet/scriptpubkeyman.cpp
• src/test/mempool_tests.cpp
• src/test/miner_tests.cpp
• src/test/util/txmempool.h
• src/test/util/txmempool.cpp
• src/wallet/wallet.cpp
• src/test/policyestimator_tests.cpp
• src/test/blockencodings_tests.cpp
• src/rpc/net.cpp
• src/test/sigopcount_tests.cpp
• src/test/fuzz/validation_load_mempool.cpp
• src/bitcoin-wallet.cpp
• src/net_processing.cpp
• src/test/util/setup_common.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

GUI implementation in src/qt/ must use Qt 5

Files:

• src/qt/optionsdialog.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/evo_deterministicmns_tests.cpp
• src/test/util_tests.cpp
• src/test/util/setup_common.h
• src/test/fuzz/tx_pool.cpp
• src/test/mempool_tests.cpp
• src/test/miner_tests.cpp
• src/test/util/txmempool.h
• src/test/util/txmempool.cpp
• src/test/policyestimator_tests.cpp
• src/test/blockencodings_tests.cpp
• src/test/sigopcount_tests.cpp
• src/test/fuzz/validation_load_mempool.cpp
• src/test/util/setup_common.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/scriptpubkeyman.cpp
• src/wallet/wallet.cpp

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests in test/functional/ must be written in Python (minimum version specified in .python-version) and depend on dashd and dash-node

Files:

• test/functional/mempool_persist.py
• test/functional/feature_bip68_sequence.py
• test/functional/mempool_compatibility.py
• test/functional/test_framework/wallet.py

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• doc/design/libraries.md
• contrib/devtools/gen-manpages.py

🧠 Learnings (29)

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

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

Learnt from: knst
Repo: dashpay/dash PR: 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.

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

📚 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/qt/**/*.{cpp,h} : GUI implementation in src/qt/ must use Qt 5

Applied to files:

• src/qt/optionsdialog.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/evo_deterministicmns_tests.cpp
• src/test/util_tests.cpp
• src/test/fuzz/tx_pool.cpp
• src/test/mempool_tests.cpp
• src/test/miner_tests.cpp
• src/Makefile.test_util.include
• src/Makefile.am
• src/test/util/txmempool.h
• src/test/policyestimator_tests.cpp
• src/test/blockencodings_tests.cpp
• src/test/sigopcount_tests.cpp
• src/test/fuzz/validation_load_mempool.cpp
• src/test/util/setup_common.cpp

📚 Learning: 2025-01-07T18:50:44.838Z

Learnt from: knst
Repo: dashpay/dash 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.

Applied to files:

• src/test/evo_deterministicmns_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/evo_deterministicmns_tests.cpp
• src/test/util/setup_common.h
• src/test/fuzz/tx_pool.cpp
• src/test/miner_tests.cpp
• src/test/util/setup_common.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/evo_deterministicmns_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,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/evo_deterministicmns_tests.cpp
• src/test/util/setup_common.h
• src/test/fuzz/tx_pool.cpp
• src/wallet/scriptpubkeyman.cpp
• src/test/mempool_tests.cpp
• src/test/miner_tests.cpp
• src/Makefile.test_util.include
• src/Makefile.am
• src/test/util/txmempool.h
• src/test/util/txmempool.cpp
• src/test/policyestimator_tests.cpp
• src/test/blockencodings_tests.cpp
• src/test/fuzz/validation_load_mempool.cpp
• src/net_processing.cpp
• src/test/util/setup_common.cpp
• test/functional/test_framework/wallet.py

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/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/evo_deterministicmns_tests.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/test/evo_deterministicmns_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/evo_deterministicmns_tests.cpp
• src/Makefile.am
• doc/design/libraries.md

📚 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.h
• src/Makefile.am

📚 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/util/setup_common.h
• src/test/miner_tests.cpp
• src/Makefile.am
• 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/llmq/**/*.{cpp,h} : InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus

Applied to files:

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

Applied to files:

• src/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/{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/util/setup_common.h
• src/test/fuzz/tx_pool.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/test/util/setup_common.h
• src/test/miner_tests.cpp
• src/Makefile.test_util.include
• src/Makefile.am
• src/bitcoin-wallet.cpp
• src/test/util/setup_common.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/test/fuzz/tx_pool.cpp
• src/test/miner_tests.cpp
• src/test/policyestimator_tests.cpp
• src/test/blockencodings_tests.cpp
• src/test/sigopcount_tests.cpp

📚 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/wallet/scriptpubkeyman.cpp
• src/test/util/txmempool.cpp
• src/wallet/wallet.cpp
• src/bitcoin-wallet.cpp
• test/functional/test_framework/wallet.py

📚 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/miner_tests.cpp
• test/functional/mempool_compatibility.py
• test/functional/test_framework/wallet.py

📚 Learning: 2025-07-23T09:30:34.631Z

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.

Applied to files:

• src/Makefile.am
• doc/design/libraries.md

📚 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/Makefile.am
• doc/design/libraries.md

📚 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/**/*.{cpp,h,hpp,cc} : Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Applied to files:

• src/Makefile.am

📚 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/Makefile.am

📚 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/Makefile.am

📚 Learning: 2025-02-14T15:19:17.218Z

Learnt from: kwvg
Repo: dashpay/dash 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.

Applied to files:

• src/wallet/wallet.cpp
• src/bitcoin-wallet.cpp

📚 Learning: 2025-08-10T13:52:46.289Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6666
File: test/functional/rpc_netinfo.py:73-81
Timestamp: 2025-08-10T13:52:46.289Z
Learning: In the Bitcoin/Dash test framework, `self.node.extra_args` contains the original startup arguments for a TestNode and is not modified by `test.restart_node()`. Each restart with new `extra_args` doesn't update the stored `self.node.extra_args`, so copying from it always provides a clean slate without accumulated modifications from previous restarts.

Applied to files:

• test/functional/feature_bip68_sequence.py

📚 Learning: 2025-06-06T11:53:09.094Z

Learnt from: kwvg
Repo: dashpay/dash 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.

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/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/** : Do not make changes to vendored dependencies: src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue} or src/crypto/{ctaes,x11}

Applied to files:

• doc/design/libraries.md

📚 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: The univalue library (in src/univalue/) is no longer a vendored external dependency but is now part of the Bitcoin Core codebase and can be modified as needed during backports.

Applied to files:

• doc/design/libraries.md

🧬 Code graph analysis (7)

src/qt/optionsdialog.cpp (1)

src/node/interfaces.cpp (1)

• ENABLE_EXTERNAL_SIGNER (570-591)

src/test/util/setup_common.h (2)

src/validation.cpp (1)

• CChainState (1648-1660)

src/validation.h (1)

• CChainState (475-601)

src/wallet/scriptpubkeyman.cpp (1)

src/script/descriptor.cpp (1)

• dest (693-703)

test/functional/mempool_persist.py (1)

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

• start_node (635-644)

src/test/util/txmempool.h (2)

src/test/util/txmempool.cpp (4)

• FromTx (31-34)
• FromTx (31-31)
• FromTx (36-40)
• FromTx (36-36)

src/test/fuzz/tx_pool.cpp (4)

• tx (67-70)
• tx (67-67)
• tx (72-75)
• tx (72-72)

src/test/util/txmempool.cpp (2)

src/util/time.h (1)

• TicksSinceEpoch (86-89)

src/test/util/txmempool.h (3)

• time (21-21)
• nHeight (22-22)
• spendsCoinbase (23-36)

src/bitcoin-wallet.cpp (2)

src/util/system.cpp (4)

• HelpRequested (838-841)
• HelpRequested (838-838)
• CheckDataDirOption (901-905)
• CheckDataDirOption (901-901)

src/script/descriptor.cpp (1)

• nullopt (656-656)

🪛 LanguageTool

doc/design/libraries.md

[style] ~88-~88: As a shorter alternative for ‘able to’, consider using “can still”.
Context: ...implementation details. But wallet code is still able to call node code indirectly through the `...

(BE_ABLE_TO)

🪛 markdownlint-cli2 (0.18.1)

doc/design/libraries.md

88-88: Emphasis style
Expected: asterisk; Actual: underscore

(MD049, emphasis-style)

---

88-88: Emphasis style
Expected: asterisk; Actual: underscore

(MD049, emphasis-style)

🪛 Ruff (0.14.6)

test/functional/feature_bip68_sequence.py

148-148: Standard pseudo-random generators are not suitable for cryptographic purposes

(S311)

contrib/devtools/gen-manpages.py

39-39: subprocess call: check for execution of untrusted input

(S603)

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

• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: Lint / Run linters
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
• GitHub Check: x86_64-pc-linux-gnu / Build depends
• GitHub Check: arm-linux-gnueabihf / Build depends

🔇 Additional comments (40)

src/qt/forms/optionsdialog.ui (1)

431-435: External signer tooltip now correctly uses a %1 placeholder

The tooltip text for externalSignerPath is now parameterized with %1, which aligns with optionsdialog.cpp formatting it via .arg(PACKAGE_NAME). This is consistent with other UI strings in this dialog and looks correct.

src/init.cpp (1)

2488-2494: BadPortWarning text change is safe and formatting remains correct

The updated warning string is purely textual and keeps the same %s/%u formatting and context in BadPortWarning. No functional impact or formatting issues.

src/qt/optionsdialog.cpp (1)

142-148: ENABLE_EXTERNAL_SIGNER logic around the external signer tooltip is now correct

Switching to #ifdef ENABLE_EXTERNAL_SIGNER and formatting the tooltip with PACKAGE_NAME matches the new %1 placeholder in the .ui file. The non-ENABLE_EXTERNAL_SIGNER case still disables the field with a clear tooltip. This fixes the previous inverted behavior without introducing new dependencies.

doc/design/libraries.md (1)

88-95: Updated dependency documentation aligns with intended module boundaries

The added explanation about wallet↔node interaction via interfaces::Chain/ChainClient and the clarification that libdash_consensus and libbitcoin_util are standalone dependencies are consistent with the dependency graph and upstream design. No action needed.

contrib/devtools/gen-manpages.py (1)

36-42: Version probing now fails fast on broken binaries

Adding check=True to the subprocess.run([abspath, "--version"], …) call means the script will now abort if a target binary exits with a non-zero status, instead of silently continuing. This is appropriate for tooling that expects working binaries and matches the upstream behavior.

src/net_processing.cpp (2)

286-293: Clarified m_relay_txs documentation in TxRelay is fine

The updated comment accurately reflects that m_relay_txs controls whether we relay transactions to this peer, matching its usage throughout the file (VERSION handler, FILTER* handlers, SendMessages). No functional issues here.

---

3815-3830: Tx relay initialization in VERSION handler is consistent and safe

The new/updated logic that calls peer->SetTxRelay() only for non-block-only, non-feeler peers when fRelay is true or we advertise NODE_BLOOM, and then sets tx_relay->m_relay_txs (plus pfrom.m_relays_txs when fRelay), aligns with the intended semantics:

• Block-relay-only and feeler peers never get a tx-relay-capable TxRelay (GetTxRelay() stays nullptr), so they won’t receive tx announcements.
• Peers that might enable relay later via BIP37 filters are initialized correctly via the NODE_BLOOM check.
• All later uses of GetTxRelay() check for nullptr, and code that must always have a TxRelay uses GetInvRelay(), so there’s no new null-dereference or race.

No changes suggested.

src/Makefile.am (1)

145-176: Makefile reorganization and module grouping look consistent

The new per-module section headers (# node #, # zmq #, # wallet #, # wallet tool #, # crypto #, # consensus #, # common #, # util #, # cli #) and associated source groupings for the various libbitcoin_* archives are structurally sound and keep the same sources wired into the build. No inconsistencies or obvious linkage issues stand out.

I’d keep this as-is for parity with upstream.

Also applies to: 468-685, 687-848, 850-891, 893-938, 940-999, 1000-1010

src/bitcoin-wallet.cpp (1)

52-94: Refactored WalletAppInit exit semantics look correct

Switching WalletAppInit to return std::optional<int> with explicit EXIT_SUCCESS/EXIT_FAILURE codes, and having MAIN_FUNCTION early-return when the optional is engaged, yields clearer and more robust control flow:

• Argument parse errors, missing command, and invalid datadir now map to EXIT_FAILURE.
• -help/-version flows exit cleanly with EXIT_SUCCESS.
• Normal initialization continues on std::nullopt.

This aligns with upstream and should not change behavior beyond making exit codes more explicit.

Also applies to: 106-107

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

1-38: New TestMemPoolEntryHelper header is consistent and usable

The extracted TestMemPoolEntryHelper matches the expected CTxMemPoolEntry construction pattern and provides the usual fluent setters plus both FromTx overloads. Default member initialization for fee, time, height, and coinbase flag is sensible; tests that care about sigops correctly call SigOps() before FromTx().

Looks good as a shared test utility.

src/test/util/setup_common.cpp (1)

37-37: Include of test/util/txmempool.h is appropriate

Adding the #include <test/util/txmempool.h> here is the right way to pick up the relocated TestMemPoolEntryHelper declarations after moving them out of setup_common. No issues spotted.

src/test/util_tests.cpp (1)

26-28: Adding <cmath> include is safe and resolves potential math dependencies

The extra <cmath> include is benign and ensures math functions used in this test file are properly declared, which is important with stricter C++20 compilers. No further changes needed.

src/test/miner_tests.cpp (1)

14-14: Miner tests correctly updated to include test/util/txmempool.h

Including the new txmempool test utility header here is required to keep TestMemPoolEntryHelper available after the refactor. The change is minimal and aligns with the new helper location.

src/test/fuzz/tx_pool.cpp (1)

13-13: Fuzz target include of test/util/txmempool.h is harmless

Bringing in test/util/txmempool.h here is consistent with other tests. Even if TestMemPoolEntryHelper isn’t used yet in this fuzz target, the include is safe and keeps things ready for future reuse.

src/test/evo_deterministicmns_tests.cpp (1)

23-23: Include of txmempool test utilities is correct

This file uses TestMemPoolEntryHelper, so adding <test/util/txmempool.h> is the right way to pick up its declaration after the refactor. No behavior change beyond satisfying the new header dependency.

src/test/blockencodings_tests.cpp (1)

10-10: Header addition matches TestMemPoolEntryHelper usage

TestMemPoolEntryHelper is used in this file, so including <test/util/txmempool.h> is required after moving the helper out of setup_common. This aligns with the rest of the test suite.

src/test/sigopcount_tests.cpp (1)

5-5: Additional coins.h include is harmless and future‑proof

Including <coins.h> here is safe and consistent with upstream’s tendency to make dependencies explicit. It doesn’t change behavior and can help prevent build issues if the test is later extended.

src/test/policyestimator_tests.cpp (1)

7-7: Correctly wiring in txmempool test helper

This test constructs mempool entries via TestMemPoolEntryHelper, so adding <test/util/txmempool.h> is required after the helper was moved. No other logic changes; fee estimation behavior remains the same.

src/test/fuzz/validation_load_mempool.cpp (1)

10-10: Extra txmempool test include is benign

Including <test/util/txmempool.h> here has no behavioral impact and keeps this fuzz harness consistent with others after the helper refactor. Safe to keep as‑is.

src/test/mempool_tests.cpp (1)

6-6: Include change correctly follows TestMemPoolEntryHelper move

Given the extensive use of TestMemPoolEntryHelper in this file, adding <test/util/txmempool.h> is the right adjustment after relocating the helper into its own header. No functional changes to the mempool tests.

test/functional/mempool_persist.py (1)

180-183: Starting node2 in test_persist_unbroadcast is consistent with MiniWallet usage

By the time test_persist_unbroadcast() is called, only node1 is running. Starting node0 and now node2 ensures the MiniWallet’s backing node (index 2) is alive again while still keeping node0 completely disconnected (no connect_nodes calls), so the getpeerinfo()/p2ps zero‑peer assertions remain valid. This aligns with the upstream MiniWallet‑based test flow.

test/functional/mempool_compatibility.py (1)

58-63: Stopping node1 before swapping mempool.dat may cause test failure on Windows

Functionally, stopping both old_node (0) and new_node (1) before moving mempool.dat is correct: it ensures no running process uses the destination datadir during the rename, and self.start_node(1) truly restarts a stopped node.

However, this introduces a cross-platform issue: after self.stop_node(1), node1 writes its own mempool.dat. On POSIX systems, os.rename(old_node_mempool, new_node_mempool) silently overwrites the destination. On Windows, os.rename() raises FileExistsError if the destination already exists, which would cause this test to fail on Windows CI.

Since this is a backport, verify that upstream Bitcoin/Dash handles this scenario—either through os.replace(), pre-removing the destination file, or confirming Windows CI is excluded from running this test.

test/functional/feature_bip68_sequence.py (10)

19-33: LGTM! Clean replacement of DUMMY_P2SH_SCRIPT.

The import additions and the computed SCRIPT_SH_OP_TRUE constant are well-structured. Computing the P2SH script once at module level is efficient and provides a proper spendable output script for the BIP68 sequence lock tests.

---

145-148: LGTM! Robustness improvement.

Dynamically limiting the number of inputs to min(10, available_utxos) prevents potential index errors when fewer UTXOs are available. This is a good defensive programming practice for the test.

---

303-307: Approve the workaround with tracking note.

This workaround addresses a Dash-specific behavior where get_utxo() may return the same transaction twice. The comment clearly documents this as a difference from Bitcoin Core and includes a TODO for future removal.

The workaround is appropriate for maintaining test compatibility while the underlying behavior difference exists.

---

312-313: LGTM! Useful debug logging.

The debug logging for tx5 and its inputs aids troubleshooting without affecting test logic.

---

97-109: LGTM! Proper P2SH output and scriptSig construction.

The replacement of DUMMY_P2SH_SCRIPT with SCRIPT_SH_OP_TRUE and the explicit scriptSig construction at line 108 are correct. The nested CScript([CScript([OP_TRUE])]) properly pushes the redeemScript for P2SH spending.

---

199-199: LGTM!

Consistent replacement with SCRIPT_SH_OP_TRUE.

---

229-229: LGTM!

Consistent replacement with SCRIPT_SH_OP_TRUE.

---

246-247: LGTM!

Proper P2SH scriptSig and output construction consistent with earlier changes.

---

366-366: LGTM!

Consistent replacement with SCRIPT_SH_OP_TRUE.

---

381-382: LGTM!

Proper P2SH scriptSig and output construction consistent with earlier changes.

src/wallet/wallet.cpp (2)

594-604: HasWalletSpend now correctly defers to IsSpent() logic

Using IsSpent(COutPoint(txid, i)) instead of checking mapTxSpends directly aligns the spend check with wallet conflict/abandon logic and avoids treating conflicted/abandoned spends as active. This is a strict improvement and matches upstream semantics.

---

2480-2497: ReserveDestination now relies on ScriptPubKeyMan for keypool top‑up

ReserveDestination::GetReservedDestination no longer calls TopUp() directly and instead delegates to ScriptPubKeyMan::GetReservedDestination, which is responsible for ensuring the keypool is filled. This keeps keypool policy localized in the scriptpubkeyman implementations and matches the Bitcoin Core refactor; no issues spotted.

If you want extra assurance, you can grep for all GetReservedDestination implementations and confirm they each either top‑up internally or only derive keys from already-cached ranges.

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

188-213: Maturity-aware UTXO filtering is correct and matches COINBASE_MATURITY semantics

The new blocks_height‑based filter:

• Correctly treats UTXOs as mature when blocks_height - height >= COINBASE_MATURITY - 1 (i.e., confirmations ≥ COINBASE_MATURITY).
• Applies only to coinbase UTXOs, leaving non-coinbase behavior unchanged.
• Keeps explicit txid/vout selection behavior the same while making the default path avoid immature coinbases.

This is consistent with the consensus maturity rule and is appropriate for the functional test framework.

src/Makefile.test_util.include (1)

7-27: txmempool test util is correctly integrated into the test library

Adding test/util/txmempool.{h,cpp} to TEST_UTIL_H and libtest_util_a_SOURCES follows the existing pattern for test helpers, so the new helper will be built and available to tests without affecting production code.

Also applies to: 31-45

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

14-20: Header dependencies and forward declaration are appropriate

Including <primitives/transaction.h> and forward-declaring CChainState in this test utility header matches its public API (e.g. CreateAndProcessBlock and mempool-related helpers) without pulling in heavier validation headers. This keeps compile-time dependencies reasonable and should resolve any prior incomplete-type issues.

Also applies to: 33-35, 137-161

src/test/util/txmempool.cpp (1)

31-40: TestMemPoolEntryHelper::FromTx overloads correctly wrap CTxMemPoolEntry construction

Both FromTx helpers are straightforward:

• The CMutableTransaction overload delegates to the CTransactionRef overload via MakeTransactionRef.
• The CTransactionRef overload passes fee, wall-clock time (seconds since epoch from NodeSeconds), height, coinbase flag, sigops, and lockpoints into the CTxMemPoolEntry constructor as expected for tests.

This matches the usual pattern from upstream and should be safe for txmempool-based tests.

If you want to be extra cautious, confirm that your local CTxMemPoolEntry constructor still takes parameters in the order (tx, fee, time, height, spends_coinbase, sigop_count, lockpoints) and uses an integer time type compatible with TicksSinceEpoch<std::chrono::seconds>(...).

src/wallet/scriptpubkeyman.cpp (2)

20-34: Keypool top‑up is now handled inside LegacyScriptPubKeyMan

• LegacyScriptPubKeyMan::GetNewDestination() now calls TopUp() before taking cs_KeyStore, so the keypool is refilled on demand when a new external address is requested.
• LegacyScriptPubKeyMan::GetReservedDestination() similarly calls TopUp() before reserving a key from the pool, centralizing top‑up logic in the key manager rather than the wallet.

This matches the upstream refactor and makes keypool behavior more self-contained.

Because TopUp() itself locks cs_KeyStore, please confirm that cs_KeyStore is implemented as a recursive mutex (or otherwise supports nested locking) in your tree. If it’s ever converted to a non-recursive mutex, GetReservedDestination would need to be adjusted to avoid double-locking.

Also applies to: 298-313, 1442-1452

---

1810-1840: DescriptorScriptPubKeyMan destination derivation and keypool top‑up look correct

• DescriptorScriptPubKeyMan::GetNewDestination() now:
  • Calls TopUp() under cs_desc_man to ensure the descriptor keypool/range is extended.
  • Expands the descriptor at next_index, and uses ExtractDestination on the resulting script, returning a clear error if extraction fails.
  • Increments next_index and persists the descriptor state, as before.
• DescriptorScriptPubKeyMan::GetReservedDestination() now delegates to GetNewDestination() and returns the derived destination while exposing the just-used index, which is what ReserveDestination in the wallet expects.

The behavior aligns with Bitcoin Core’s descriptor wallet flow and keeps reservation and derivation logic in one place.

Given GetReservedDestination calls GetNewDestination, which also locks cs_desc_man, it’s worth double-checking that cs_desc_man is a recursive mutex in your codebase. If it’s not, this function should be adjusted to avoid taking the same lock twice.

Also applies to: 1933-1939, 1971-1983

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c72d86 and d0b344e.

📒 Files selected for processing (16)

• contrib/devtools/gen-manpages.py (1 hunks)
• src/Makefile.test_util.include (2 hunks)
• src/bitcoin-wallet.cpp (3 hunks)
• src/test/blockencodings_tests.cpp (1 hunks)
• src/test/evo_deterministicmns_tests.cpp (1 hunks)
• src/test/fuzz/tx_pool.cpp (1 hunks)
• src/test/fuzz/validation_load_mempool.cpp (1 hunks)
• src/test/mempool_tests.cpp (1 hunks)
• src/test/miner_tests.cpp (1 hunks)
• src/test/policyestimator_tests.cpp (1 hunks)
• src/test/sigopcount_tests.cpp (1 hunks)
• src/test/util/setup_common.cpp (1 hunks)
• src/test/util/setup_common.h (2 hunks)
• src/test/util/txmempool.cpp (1 hunks)
• src/test/util/txmempool.h (1 hunks)
• src/test/util_tests.cpp (1 hunks)

✅ Files skipped from review due to trivial changes (2)

• src/test/mempool_tests.cpp
• src/test/evo_deterministicmns_tests.cpp

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

• src/test/miner_tests.cpp
• src/test/fuzz/tx_pool.cpp
• src/test/util/txmempool.cpp
• src/test/util/setup_common.h
• src/test/fuzz/validation_load_mempool.cpp

🧰 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/test/blockencodings_tests.cpp
• src/test/util/setup_common.cpp
• src/test/policyestimator_tests.cpp
• src/bitcoin-wallet.cpp
• src/test/util/txmempool.h
• src/test/util_tests.cpp
• src/test/sigopcount_tests.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
• src/test/util/setup_common.cpp
• src/test/policyestimator_tests.cpp
• src/test/util/txmempool.h
• src/test/util_tests.cpp
• src/test/sigopcount_tests.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• contrib/devtools/gen-manpages.py

🧠 Learnings (9)

📓 Common learnings

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

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

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

Learnt from: kwvg
Repo: dashpay/dash 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.

📚 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/blockencodings_tests.cpp
• src/Makefile.test_util.include
• src/test/util/setup_common.cpp
• src/test/policyestimator_tests.cpp
• src/test/util/txmempool.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/blockencodings_tests.cpp
• src/Makefile.test_util.include
• src/test/util/setup_common.cpp
• src/test/policyestimator_tests.cpp
• src/test/util_tests.cpp
• src/test/sigopcount_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/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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/test/blockencodings_tests.cpp
• src/test/policyestimator_tests.cpp
• src/test/sigopcount_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/util/setup_common.cpp

📚 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/test/util/setup_common.cpp
• src/bitcoin-wallet.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/test/util/setup_common.cpp
• src/bitcoin-wallet.cpp

📚 Learning: 2025-02-14T15:19:17.218Z

Learnt from: kwvg
Repo: dashpay/dash 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.

Applied to files:

• src/bitcoin-wallet.cpp

🧬 Code graph analysis (2)

src/bitcoin-wallet.cpp (2)

src/util/system.cpp (4)

• HelpRequested (838-841)
• HelpRequested (838-838)
• CheckDataDirOption (901-905)
• CheckDataDirOption (901-901)

src/script/descriptor.cpp (1)

• nullopt (656-656)

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

src/test/util/txmempool.cpp (4)

• FromTx (31-34)
• FromTx (31-31)
• FromTx (36-40)
• FromTx (36-36)

🪛 Ruff (0.14.6)

contrib/devtools/gen-manpages.py

39-39: subprocess call: check for execution of untrusted input

(S603)

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

• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
• GitHub Check: Lint / Run linters
• GitHub Check: x86_64-pc-linux-gnu / Build depends
• GitHub Check: arm-linux-gnueabihf / Build depends

🔇 Additional comments (9)

src/bitcoin-wallet.cpp (2)

52-93: LGTM - Clean refactor from bool to std::optional for explicit exit code handling.

The change from bool to std::optional<int> allows differentiated exit codes:

• EXIT_FAILURE for actual errors (parse failure, missing args, invalid data dir)
• EXIT_SUCCESS for intentional help/version display
• std::nullopt to continue normal execution

The missing_args logic correctly shows usage to stdout first, then prints the error to stderr before returning failure - good UX for CLI tools.

Based on learnings, this is a Bitcoin Core backport and the implementation matches upstream.

---

106-106: Correct handling of optional return value.

The structured binding with conditional check is idiomatic C++17: if WalletAppInit returns an exit code (success or failure), propagate it immediately; otherwise continue execution.

src/Makefile.test_util.include (1)

7-8: LGTM - Standard build file updates for test utility refactor.

The additions of test/util/txmempool.h and test/util/txmempool.cpp to the build system are consistent with extracting TestMemPoolEntryHelper into dedicated files. The formatting changes improve consistency.

Also applies to: 10-27, 42-42

src/test/util_tests.cpp (1)

27-27: LGTM - Include is required for sqrt() usage.

The <cmath> header is needed for the sqrt() function call on line 472 in the util_seed_insecure_rand test case.

src/test/blockencodings_tests.cpp (1)

10-10: LGTM - Required include for TestMemPoolEntryHelper.

The include is necessary for the TestMemPoolEntryHelper usage throughout this test file (lines 59, 142, 212).

src/test/policyestimator_tests.cpp (1)

7-7: LGTM - Required include for TestMemPoolEntryHelper.

The include is necessary for the TestMemPoolEntryHelper usage on line 23.

src/test/sigopcount_tests.cpp (1)

5-5: LGTM - Include from Bitcoin Core backport.

The <coins.h> include addition matches the upstream Bitcoin Core changes. While not immediately used in the visible code, it may address implicit dependencies or prepare for related changes in this backport series.

Based on learnings, as per coding guidelines.

src/test/util/setup_common.cpp (1)

37-37: LGTM - Part of TestMemPoolEntryHelper refactor.

The include addition is necessary as part of moving TestMemPoolEntryHelper from setup_common.h to the dedicated test/util/txmempool.h header. The FromTx method implementations have been appropriately moved to src/test/util/txmempool.cpp.

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

1-38: LGTM - Clean extraction of test utility.

The TestMemPoolEntryHelper struct has been properly extracted from setup_common.h into a dedicated header. The design is clean:

• Appropriate includes and forward declarations
• Public data members with sensible defaults
• Fluent setter interface for test construction
• FromTx overloads for convenient entry creation
• Proper header guards and copyright

The commented TODO on lines 15-16 referencing upstream bitcoin#25290 is acceptable for a backport PR.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix exception handling to catch CalledProcessError.

Adding check=True at line 39 causes subprocess.run to raise subprocess.CalledProcessError on non-zero exit codes, but the exception handler only catches IOError. This will result in an unhandled exception crash instead of the intended error message.

Apply this diff to fix the exception handling:

     try:
         r = subprocess.run([abspath, "--version"], stdout=subprocess.PIPE, check=True, universal_newlines=True)
-    except IOError:
+    except (IOError, subprocess.CalledProcessError):
         print(f'{abspath} not found or not an executable', file=sys.stderr)
         sys.exit(1)

Note regarding static analysis: The Ruff S603 warning about untrusted input is a false positive here, as abspath is constructed from controlled sources (project-built binaries).

🧰 Tools

🪛 Ruff (0.14.6)

39-39: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents

In contrib/devtools/gen-manpages.py around lines 38-42, the try/except only
catches IOError but subprocess.run(..., check=True) will raise
subprocess.CalledProcessError on non-zero exit codes; change the exception
handler to catch subprocess.CalledProcessError (and still handle OSError/IOError
for missing executable) and keep the same stderr message and sys.exit(1);
reference subprocess.CalledProcessError (or import it) so the new except clause
handles both failure modes.

[PastaPastaPasta]

utACK f0fa6de

[UdjinM6]

utACK f0fa6de