backport: bitcoin#26120, #26192, #26378, #26643, #26752, #26758, #26771, #26810, #26824, #26867

[knst]

What was done?

Regular backports from Bitcoin Core v25

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

This PR makes multiple coordinated changes across build config, docs, static analysis, networking, wallet, and various source files. Notable edits: include build_bitcoin_util in the configure.ac gating; add clang-tidy check performance-no-automatic-move; remove mempool_sequence from Chain::Notifications and CWallet transaction callbacks and update all call sites and tests; change several function parameter types from pointers to const references in net processing; adjust local variable constness in many files; refactor miner grind_task to return found nonce via an out parameter; add a pre-check to prevent loading a wallet with a duplicate name; and update multiple documentation links and minor test/script tweaks.

Sequence Diagram(s)

(skip)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• UdjinM6
• kwvg

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.39% 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 PR title accurately summarizes the main change: backporting 10 specific Bitcoin Core commits to the develop branch.
Description check	✅ Passed	The PR description clearly indicates this is a backport of Bitcoin Core v25 commits, directly related to the substantial changeset affecting build configuration, documentation, code refactoring, and wallet functionality.

✨ Finishing touches

• 📝 Generate docstrings

---

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)

doc/build-windows.md (1)

9-9: Microsoft Learn URLs verified as accessible

All three Microsoft Learn URLs are accessible and correctly point to the intended documentation:

• Line 9: WSL about page ✓
• Line 21: WSL install page ✓
• Line 37: WSL/Windows interoperability archive ✓

The secondary note about "here" as link text at line 21 remains a minor documentation style issue that can be addressed separately if desired.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a2943e and becb647.

📒 Files selected for processing (26)

• configure.ac
• contrib/debian/copyright
• doc/build-windows.md
• doc/fuzzing.md
• doc/managing-wallets.md
• src/.clang-tidy
• src/addrman.cpp
• src/bitcoin-cli.cpp
• src/bitcoin-util.cpp
• src/interfaces/chain.h
• src/net_processing.cpp
• src/node/interfaces.cpp
• src/primitives/transaction.h
• src/rpc/util.cpp
• src/test/fuzz/tx_pool.cpp
• src/test/fuzz/txorphan.cpp
• src/test/util/setup_common.cpp
• src/validation.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/wallet.cpp
• src/wallet/spend.cpp
• src/wallet/test/coinselector_tests.cpp
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp
• src/wallet/wallet.h
• test/functional/wallet_multiwallet.py

💤 Files with no reviewable changes (1)

• contrib/debian/copyright

🧰 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/primitives/transaction.h
• src/test/util/setup_common.cpp
• src/bitcoin-cli.cpp
• src/wallet/test/coinselector_tests.cpp
• src/rpc/util.cpp
• src/interfaces/chain.h
• src/wallet/wallet.h
• src/wallet/rpc/wallet.cpp
• src/wallet/test/wallet_tests.cpp
• src/addrman.cpp
• src/wallet/wallet.cpp
• src/net_processing.cpp
• src/wallet/spend.cpp
• src/wallet/rpc/util.cpp
• src/bitcoin-util.cpp
• src/validation.cpp
• src/test/fuzz/tx_pool.cpp
• src/test/fuzz/txorphan.cpp
• src/node/interfaces.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:

• doc/fuzzing.md
• doc/managing-wallets.md
• doc/build-windows.md

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/util/setup_common.cpp
• src/wallet/test/coinselector_tests.cpp
• src/wallet/test/wallet_tests.cpp
• src/test/fuzz/tx_pool.cpp
• src/test/fuzz/txorphan.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/test/coinselector_tests.cpp
• src/wallet/wallet.h
• src/wallet/rpc/wallet.cpp
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp
• src/wallet/spend.cpp
• src/wallet/rpc/util.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/wallet_multiwallet.py

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

📄 CodeRabbit inference engine (CLAUDE.md)

NodeContext must be extended with Dash-specific managers for each subsystem during initialization

Files:

• src/node/interfaces.cpp

🧠 Learnings (17)

📓 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: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

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: 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: 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,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing

📚 Learning: 2025-11-14T23:17:08.495Z

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6976
File: src/stacktraces.cpp:475-482
Timestamp: 2025-11-14T23:17:08.495Z
Learning: In C++ code reviews, always check for opportunities to use standard algorithms (std::any_of, std::all_of, std::none_of, std::find_if, std::transform, std::for_each, etc.) instead of manual for-loops, especially when the loop body performs simple checks, transformations, or accumulations.

Applied to files:

• src/primitives/transaction.h
• src/.clang-tidy

📚 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/primitives/transaction.h
• src/interfaces/chain.h
• src/wallet/wallet.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/bitcoin-cli.cpp
• src/rpc/util.cpp
• src/wallet/wallet.h
• doc/managing-wallets.md
• test/functional/wallet_multiwallet.py
• src/wallet/rpc/wallet.cpp
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp
• src/wallet/rpc/util.cpp
• src/validation.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/wallet/test/coinselector_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/interfaces/chain.h
• src/wallet/wallet.h
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp
• src/net_processing.cpp
• src/wallet/spend.cpp
• src/validation.cpp
• src/test/fuzz/tx_pool.cpp
• src/node/interfaces.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/interfaces/chain.h
• src/net_processing.cpp
• src/wallet/spend.cpp
• src/validation.cpp
• src/node/interfaces.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/wallet/wallet.h
• test/functional/wallet_multiwallet.py
• src/wallet/rpc/wallet.cpp
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp
• src/wallet/rpc/util.cpp

📚 Learning: 2025-01-14T08:37:16.955Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.

Applied to files:

• src/wallet/wallet.h
• src/wallet/test/wallet_tests.cpp
• src/wallet/wallet.cpp

📚 Learning: 2025-12-17T13:58:26.891Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.

Applied to files:

• src/wallet/wallet.h
• doc/managing-wallets.md
• src/wallet/wallet.cpp

📚 Learning: 2025-06-09T16:43:20.996Z

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

Applied to files:

• test/functional/wallet_multiwallet.py
• src/wallet/test/wallet_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/wallet/test/wallet_tests.cpp
• configure.ac

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

📚 Learning: 2025-02-06T14:34:30.466Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Applied to files:

• src/wallet/wallet.cpp

📚 Learning: 2025-07-14T10:11:05.011Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6754
File: contrib/containers/guix/docker-compose.yml:18-19
Timestamp: 2025-07-14T10:11:05.011Z
Learning: In the Guix build process for Dash Core, the `guix.sigs` directory requires write access as signatures are written to it during the build process, and `dash-detached-sigs` may be updated with `git pull` operations, so both directories need rw permissions in the Docker volume mounts.

Applied to files:

• doc/build-windows.md

📚 Learning: 2025-08-19T14:57:31.801Z

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Applied to files:

• src/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/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/wallet/spend.cpp

🧬 Code graph analysis (7)

src/bitcoin-cli.cpp (2)

src/qt/rpcconsole.cpp (1)

• reply (456-456)

src/rpc/request.cpp (1)

• reply (40-40)

src/wallet/test/coinselector_tests.cpp (1)

src/wallet/spend.cpp (2)

• SelectCoins (515-694)
• SelectCoins (515-515)

src/rpc/util.cpp (9)

src/rpc/quorums.cpp (2)

• ret (78-78)
• ret (144-144)

src/rpc/blockchain.cpp (8)

• ret (342-342)
• ret (388-388)
• ret (434-434)
• ret (1191-1191)
• ret (1335-1335)
• ret (1940-1940)
• ret (2242-2242)
• ret (2665-2665)

src/wallet/rpc/coins.cpp (1)

• ret (420-420)

src/wallet/rpc/transactions.cpp (2)

• ret (145-145)
• ret (684-684)

src/rest.cpp (1)

• ret (583-583)

src/rpc/node.cpp (2)

• ret (181-181)
• ret (187-187)

src/qt/rpcconsole.cpp (2)

• request (421-475)
• request (421-421)

src/rpc/request.cpp (1)

• request (31-31)

src/rpc/server.cpp (1)

• request (552-552)

src/wallet/wallet.h (1)

src/wallet/wallet.cpp (4)

• transactionAddedToMempool (1332-1341)
• transactionAddedToMempool (1332-1332)
• transactionRemovedFromMempool (1343-1382)
• transactionRemovedFromMempool (1343-1343)

test/functional/wallet_multiwallet.py (2)

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

• assert_raises_rpc_error (132-148)

src/wallet/rpc/wallet.cpp (2)

• loadwallet (504-556)
• loadwallet (504-504)

src/wallet/spend.cpp (1)

src/primitives/transaction.h (1)

• CalculateOutputValue (210-213)

src/bitcoin-util.cpp (1)

src/chain.h (1)

• nBits (190-190)

🪛 markdownlint-cli2 (0.18.1)

doc/build-windows.md

21-21: Link text should be descriptive

(MD059, descriptive-link-text)

⏰ 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_tsan-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: Lint / Run linters

🔇 Additional comments (36)

src/bitcoin-util.cpp (3)

80-103: LGTM: Thread-safe refactoring eliminates data races.

The changes correctly address TSAN concerns by passing header by value (giving each thread its own copy) and using found.exchange(true) to ensure only one thread writes to proposed_nonce. The atomic exchange operation guarantees that only the first thread to find a valid nonce will store its result, preventing concurrent writes.

---

120-125: LGTM: Correct thread creation with new signature.

The proposed_nonce variable is properly initialized and passed by reference to worker threads. The thread creation correctly passes header by value (each thread gets its own copy) and uses std::ref for the shared found and proposed_nonce variables, matching the updated grind_task signature.

---

130-135: LGTM: Safe read after thread synchronization.

The code correctly waits for all threads to complete via join() before reading proposed_nonce. This establishes a happens-before relationship that ensures the main thread sees the write performed by the winning worker thread, with no data race.

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

204-204: LGTM! Const removal enables move optimization.

Removing const from the local variable allows the compiler to apply move semantics when returning tx. The variable is used read-only after construction, so behavior is unchanged.

src/.clang-tidy (1)

10-10: LGTM! Static analysis addition for move semantics.

Adding the performance-no-automatic-move check enforces better move semantics optimization across the codebase. This check complements the const-removal changes seen in other files of this PR.

Also applies to: 23-23

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

499-499: LGTM! Explicit type annotation for clarity.

Making the type explicit as CBlock instead of using auto ensures proper copy elision and aligns with the move semantics improvements in this PR.

src/addrman.cpp (3)

1194-1201: LGTM! Const removal enables move semantics.

Removing const from the local return value holder allows the compiler to apply move semantics when returning. This is a performance optimization with no behavior change.

---

1203-1210: LGTM! Const removal enables move semantics.

Removing const from the local return value holder allows the compiler to apply move semantics when returning. This is a performance optimization with no behavior change.

---

1212-1219: LGTM! Const removal enables move semantics.

Removing const from the local return value holder allows the compiler to apply move semantics when returning. This is a performance optimization with no behavior change.

doc/fuzzing.md (1)

282-283: Microsoft documentation URL and Ubuntu capitalization verified as correct.

The URL has been properly updated to learn.microsoft.com and "Ubuntu" is correctly capitalized. The Microsoft Learn URL is valid and points to the correct .NET Core installation documentation for Linux.

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

76-76: LGTM: Cosmetic const removal

The removal of const from new_tx is a cosmetic change with no functional impact, as the variable is not reassigned after initialization. This change is part of the upstream backport and aligns with broader const-correctness adjustments in the PR.

src/bitcoin-cli.cpp (1)

855-855: LGTM: Consistent const removal

Removing const from the reply variable is consistent with similar changes throughout the PR. The variable is not modified after assignment, so this is purely a cosmetic change with no functional impact.

configure.ac (1)

1589-1589: LGTM: Correct addition of dash-util target

Adding build_bitcoin_util to the gating condition is correct and necessary. This ensures that when dash-util is built, Boost dependencies are properly enabled. The change aligns with the PR's inclusion of bitcoin#26824, which fixes configure.ac for bitcoin-util builds.

src/rpc/util.cpp (1)

536-536: LGTM: Consistent const removal in RPC handler

The removal of const from the ret variable is consistent with the broader pattern in this PR. The variable is not modified after assignment from m_fun, making this a cosmetic change with no functional impact. This aligns with the upstream backport.

src/wallet/test/coinselector_tests.cpp (1)

956-956: LGTM: Minor const-correctness adjustment.

The removal of const from the local variable is consistent with the broader const-correctness refactoring in this PR. The variable is not modified after initialization, so this change has no functional impact.

src/wallet/rpc/util.cpp (1)

78-78: LGTM: Const-correctness alignment with return type.

Removing const from the local variable aligns with the function's return type and is consistent with the broader const-correctness refactoring in this PR. No functional impact.

src/primitives/transaction.h (2)

20-20: LGTM: Required include for std::accumulate.

The <numeric> header is correctly included to support the new CalculateOutputValue function template.

---

209-213: LGTM: Clean utility function using standard algorithms.

The CalculateOutputValue template function is well-implemented:

• Uses std::accumulate for a clean, idiomatic sum calculation
• Template design allows use with both CTransaction and CMutableTransaction
• Inline placement in the header is appropriate for this small utility
• Initial value of CAmount{0} is correct

src/wallet/test/wallet_tests.cpp (2)

1559-1559: LGTM: Test updated for new transactionAddedToMempool signature.

The call correctly reflects the removal of the mempool_sequence parameter from the transactionAddedToMempool method signature, as described in the PR's broader API changes.

---

1580-1580: LGTM: Test updated for new transactionAddedToMempool signature.

The call correctly reflects the removal of the mempool_sequence parameter from the transactionAddedToMempool method signature, consistent with the API changes throughout the PR.

src/wallet/rpc/wallet.cpp (1)

537-544: LoadWallet has internal protection via g_loading_wallet_set that prevents concurrent loading of the same wallet.

The pre-check prevents loading an already-loaded wallet, but a TOCTOU window exists between releasing the wallets_mutex lock (line 543) and calling LoadWallet (line 545). However, LoadWallet includes its own protection mechanism at lines 287-291:

auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(name));
if (!result.second) {
    error = Untranslated("Wallet already loading.");
    status = DatabaseStatus::FAILED_LOAD;
    return nullptr;
}

This prevents the primary race condition: concurrent loading attempts of the same wallet name will fail on the second attempt. The pre-check appears to serve as an early-validation optimization to provide a cleaner error message before attempting to load, rather than being the primary protection mechanism.

The secondary AddWallet check (line 121 in wallet.cpp) prevents duplicate wallet objects by identity, providing an additional layer of safety.

test/functional/wallet_multiwallet.py (1)

303-308: LGTM! Unified duplicate wallet error handling correctly implemented.

The test assertions now correctly expect the uniform error code -35 (RPC_WALLET_ALREADY_LOADED) with the message Wallet "w1" is already loaded. for both duplicate wallet loading scenarios. This aligns with the pre-load validation check added in src/wallet/rpc/wallet.cpp that detects already-loaded wallets before attempting to load them, providing consistent error reporting regardless of how the duplicate load is attempted (by wallet name or by file path).

doc/managing-wallets.md (1)

93-125: LGTM! Documentation improvements enhance clarity and accuracy.

The documentation updates correctly:

• Standardize ellipsis characters (… instead of ...) for GUI menu items
• Add the GUI restore option (Restore Wallet…) to complement the existing RPC guidance, providing users with more accessible wallet restoration methods
• Improve capitalization consistency for menu paths (Open Wallet vs Open wallet)

These changes align with the upstream Bitcoin Core documentation improvements and make the wallet management guide more comprehensive and consistent.

src/validation.cpp (2)

1401-1419: Non-const result binding in AcceptToMemoryPool is fine

Changing result to a non-const MempoolAcceptResult does not alter behavior (it’s only read and then returned) and aligns with typical patterns for returning local results by value. No issues here.

---

1430-1443: Dropping const from result in ProcessNewPackage is safe

Binding the lambda return to auto result instead of const auto is behaviorally equivalent here and keeps the value flexible for potential future moves or modifications. The surrounding control flow and uncache logic remain unchanged.

src/wallet/wallet.cpp (2)

262-262: Relaxed std::shared_ptr<CWallet> constness looks fine

Using non-const std::shared_ptr<CWallet> locals for wallet/walletInstance here is harmless and aligns with upstream, while the pointed-to CWallet remains mutable as before. No behavioral or lifetime regressions spotted.

Also applies to: 346-346, 2930-2931

---

1332-1341: Mempool notification refactor is consistent and lock-safe

The updated transactionAddedToMempool / transactionRemovedFromMempool signatures and implementations (no mempool_sequence) correctly:

• Lock cs_wallet before touching mapWallet and calling SyncTransaction.
• Use RefreshMempoolStatus for non-CONFLICT removals and SyncTransaction(TxStateInactive{}) for CONFLICT removals.
• Call transactionRemovedFromMempool(..., MemPoolRemovalReason::BLOCK) from blockConnected, which is safe with RecursiveMutex cs_wallet.

This matches the intended upstream behavior for these callbacks.

Also applies to: 1343-1382, 1389-1395

src/wallet/wallet.h (1)

646-647: Wallet mempool notification overrides correctly match interfaces::Chain::Notifications

The updated transactionAddedToMempool and transactionRemovedFromMempool overrides drop mempool_sequence and now match the revised interfaces::Chain::Notifications signatures, keeping the inheritance contract consistent.

Also applies to: 668-668

src/interfaces/chain.h (1)

264-265: Chain notifications API update is complete and consistent

The interfaces::Chain::Notifications interface has been properly updated:

• transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime)
• transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason)

All implementors match these new signatures:

• CWallet directly implements both methods without mempool_sequence
• NotificationsProxy in node/interfaces.cpp correctly adapts the underlying CValidationInterface (which retains mempool_sequence) to the new Notifications interface, properly stripping the parameter in the delegation calls

The implementation is consistent across the codebase.

src/wallet/spend.cpp (1)

11-11: Review comment is analyzing code that does not exist in this repository

The specific bugs and code patterns described do not match the current state of src/wallet/spend.cpp. Key mismatches:

• Line 11 contains #include <script/signingprovider.h>, not #include <primitives/transaction.h>
• No Assume() statements exist in the function (search returned no results)
• No current_fee variable exists anywhere in the wallet code (search across entire src/wallet/ returned no results)
• The claimed problematic lines (e.g., line 984 with Assume(recipients_sum + change_amount == output_value), line 1056 with the invariant check) do not exist at those locations
• Line 1000 claim is off by 8 lines; change_amount is actually declared at line 992

The actual implementation around these line ranges uses simpler logic with change_and_fee, fee_needed, and change_amount variables, with no Assume statements or fee invariant checks like those described. The code structure in this file differs from the code being analyzed in the review comment.

This suggests the reviewer was analyzing a different version of the code or upstream Bitcoin Core implementation rather than the current committed state of this PR.

Likely an incorrect or invalid review comment.

src/node/interfaces.cpp (3)

910-913: LGTM! Parameter removal correctly implemented.

The adapter pattern is properly maintained: mempool_sequence is received from CValidationInterface but intentionally discarded when forwarding to Chain::Notifications, removing an unused parameter from the wallet interface.

---

914-917: LGTM! Consistent parameter removal.

Correctly mirrors the pattern in TransactionAddedToMempool, maintaining consistency across both notification methods.

---

1301-1308: LGTM! Call site correctly updated.

The removal of the hardcoded mempool_sequence = 0 parameter confirms it was unused, and the update correctly aligns with the simplified interface.

src/net_processing.cpp (3)

773-779: Helper declarations now use non-null const CBlockIndex& parameters

Switching HeadersDirectFetchBlocks and UpdatePeerStateForReceivedHeaders from const CBlockIndex* to const CBlockIndex& is consistent with their implementations and call sites, and encodes the existing non-null expectation in the type system without changing behavior.
Based on learnings, this matches the upstream Bitcoin Core backport and keeps divergence minimal.

---

3138-3201: Pointer-to-reference refactor in HeadersDirectFetchBlocks is mechanically correct

All uses of pindexLast were correctly converted to last_header/&last_header (validity checks, nChainWork, GetBlockHash(), nHeight, and pprev access), with locking and control flow unchanged. This preserves the previous semantics while avoiding nullable parameters.

---

3203-3261: UpdatePeerStateForReceivedHeaders and its callers remain consistent after refactor

The function now takes const CBlockIndex& last_header and correctly uses last_header.GetBlockHash() and last_header.nChainWork where pindexLast was previously dereferenced. ProcessHeadersMessage passes *pindexLast into both UpdatePeerStateForReceivedHeaders and HeadersDirectFetchBlocks, matching the former non-null pointer assumptions and upstream behavior.

Also applies to: 3321-3325