backport: bitcoin#24364, #25676, #25737, #26095, #26132, #26198, #26234, #26235, #26236, #26254, #26524

[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 (for repository code-owners and collaborators only)

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

This pull request reorganizes and refactors several components across the codebase. Key changes include: moving ParseISO8601DateTime from the global util namespace to the wallet namespace and replacing Boost dependency with a local implementation; refactoring locking primitives in sync.h with template parameter renaming and macro adjustments; introducing a new UniValue::type_error exception type and updating RPC handlers to throw it; reorganizing index default constants from validation.h to header-specific files; renaming wallet transaction rebroadcast timing from nNextResend to atomic m_next_resend; removing explicit RPC type checks in favor of inline extraction; and updating test expectations for RPC type error codes from -1 to -3. Numerous typo corrections and header cleanups are also included.

Estimated code review effort

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

Areas requiring extra attention:

• src/sync.h — Extensive refactoring of locking template primitives (UniqueLock, MaybeCheckNotHeld, macro expansions). Template parameter renaming from Mutex to MutexType and base class derivation changes require careful verification of type safety and macro expansion behavior across all locking constructs.
• RPC error handling pipeline — Changes to type-error signaling across src/rpc/server.cpp, src/univalue/, and multiple RPC handlers (fees.cpp, mempool.cpp, coins.cpp, spend.cpp) that remove explicit type checks and rely on inline extraction. Verify that error reporting remains correct.
• ParseISO8601DateTime namespace relocation — Function moved from src/util/time.{h,cpp} to src/wallet/rpc/util.{h,cpp} with Boost dependency implications. Ensure all call sites are correctly updated (particularly src/wallet/test/fuzz/parse_iso8601.cpp and src/test/fuzz/util.cpp which now use hard-coded timestamps instead).
• Test expectation updates — Repetitive changes across multiple functional test files updating expected RPC error codes from -1 to -3. While mechanically similar, verify consistency across all test files.
• Wallet member variable atomicity — Replacement of int64_t nNextResend with std::atomic<int64_t> m_next_resend in CWallet. Review thread-safety implications and access patterns in src/wallet/wallet.cpp.
• Index default constant relocation — DEFAULT_TXINDEX, DEFAULT_COINSTATSINDEX, DEFAULT_BLOCKFILTERINDEX moved from validation.h to their respective header files. Verify no downstream includes are broken and that new DEFAULT_SYNC_MEMPOOL behaves as intended.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.32% 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 clearly identifies this as a backport of specific Bitcoin Core commits, which matches the extensive code changes across multiple files shown in the changeset.
Description check	✅ Passed	The pull request description clearly states the purpose: 'Regular backports from Bitcoin Core v25' and confirms testing was performed.

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

🧹 Nitpick comments (1)

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

15-33: ParseISO8601DateTime implementation looks correct; consider explicit STL includes.

The Boost-based parsing logic and error handling (returning 0 on parse failure or pre-epoch timestamps) are consistent and thread-safe thanks to function-local statics; this matches the historical behavior of Bitcoin’s ISO8601 parsing.

One minor thing to double-check: this function uses std::locale and std::istringstream, but this file does not explicitly include <locale> or <sstream>. If those types are only coming in via transitive includes, it’s a bit brittle and may diverge from upstream if they add the direct includes there. It might be worth confirming this matches the exact include set from the corresponding Bitcoin Core file, and adding the missing headers here if upstream has them as well, to avoid future IWYU or toolchain issues.

📜 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 7211d23.

📒 Files selected for processing (50)

• ci/dash/lint-tidy.sh (1 hunks)
• ci/lint/04_install.sh (1 hunks)
• ci/lint/06_script.sh (1 hunks)
• ci/test/04_install.sh (1 hunks)
• doc/release-notes-empty-template.md (1 hunks)
• src/Makefile.test.include (2 hunks)
• src/bench/nanobench.h (1 hunks)
• src/bitcoind.cpp (1 hunks)
• src/index/blockfilterindex.h (1 hunks)
• src/index/coinstatsindex.h (1 hunks)
• src/index/txindex.h (1 hunks)
• src/node/caches.cpp (1 hunks)
• src/node/miner.cpp (1 hunks)
• src/rpc/fees.cpp (0 hunks)
• src/rpc/mempool.cpp (1 hunks)
• src/rpc/server.cpp (1 hunks)
• src/sync.h (4 hunks)
• src/test/fuzz/util.cpp (1 hunks)
• src/test/util_tests.cpp (1 hunks)
• src/test/validation_chainstatemanager_tests.cpp (1 hunks)
• src/univalue/include/univalue.h (1 hunks)
• src/univalue/lib/univalue.cpp (1 hunks)
• src/util/time.cpp (0 hunks)
• src/util/time.h (0 hunks)
• src/validation.h (1 hunks)
• src/wallet/rpc/coins.cpp (1 hunks)
• src/wallet/rpc/spend.cpp (1 hunks)
• src/wallet/rpc/util.cpp (1 hunks)
• src/wallet/rpc/util.h (1 hunks)
• src/wallet/test/fuzz/coinselection.cpp (1 hunks)
• src/wallet/test/fuzz/parse_iso8601.cpp (2 hunks)
• src/wallet/test/rpc_util_tests.cpp (1 hunks)
• src/wallet/wallet.cpp (1 hunks)
• src/wallet/wallet.h (1 hunks)
• src/zmq/zmqabstractnotifier.h (1 hunks)
• src/zmq/zmqnotificationinterface.cpp (1 hunks)
• src/zmq/zmqnotificationinterface.h (1 hunks)
• src/zmq/zmqpublishnotifier.cpp (1 hunks)
• src/zmq/zmqpublishnotifier.h (1 hunks)
• src/zmq/zmqrpc.cpp (1 hunks)
• test/README.md (1 hunks)
• test/functional/mining_prioritisetransaction.py (1 hunks)
• test/functional/rpc_blockchain.py (2 hunks)
• test/functional/rpc_help.py (1 hunks)
• test/functional/rpc_rawtransaction.py (4 hunks)
• test/functional/wallet_basic.py (1 hunks)
• test/functional/wallet_hd.py (1 hunks)
• test/functional/wallet_multiwallet.py (1 hunks)
• test/lint/lint-locale-dependence.py (1 hunks)
• test/lint/spelling.ignore-words.txt (1 hunks)

💤 Files with no reviewable changes (3)

• src/util/time.cpp
• src/rpc/fees.cpp
• src/util/time.h

🧰 Additional context used

📓 Path-based instructions (8)

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

• ci/lint/06_script.sh
• ci/lint/04_install.sh
• ci/test/04_install.sh
• doc/release-notes-empty-template.md
• ci/dash/lint-tidy.sh

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/wallet/test/rpc_util_tests.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/bitcoind.cpp
• src/bench/nanobench.h
• src/wallet/test/fuzz/coinselection.cpp
• src/zmq/zmqpublishnotifier.cpp
• src/zmq/zmqnotificationinterface.h
• src/test/fuzz/util.cpp
• src/node/caches.cpp
• src/rpc/server.cpp
• src/wallet/wallet.h
• src/rpc/mempool.cpp
• src/zmq/zmqrpc.cpp
• src/zmq/zmqabstractnotifier.h
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/util.cpp
• src/node/miner.cpp
• src/index/blockfilterindex.h
• src/test/util_tests.cpp
• src/index/coinstatsindex.h
• src/wallet/rpc/util.h
• src/index/txindex.h
• src/univalue/lib/univalue.cpp
• src/wallet/wallet.cpp
• src/zmq/zmqnotificationinterface.cpp
• src/zmq/zmqpublishnotifier.h
• src/univalue/include/univalue.h
• src/wallet/rpc/spend.cpp
• src/validation.h
• src/wallet/test/fuzz/parse_iso8601.cpp
• src/sync.h

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

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests in src/test/ and src/wallet/test/ must use Boost::Test framework

Files:

• src/wallet/test/rpc_util_tests.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/wallet/test/fuzz/coinselection.cpp
• src/test/fuzz/util.cpp
• src/test/util_tests.cpp
• src/wallet/test/fuzz/parse_iso8601.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

Wallet implementation must use Berkeley DB and SQLite

Files:

• src/wallet/test/rpc_util_tests.cpp
• src/wallet/test/fuzz/coinselection.cpp
• src/wallet/wallet.h
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/util.h
• src/wallet/wallet.cpp
• src/wallet/rpc/spend.cpp
• src/wallet/test/fuzz/parse_iso8601.cpp

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/bench/nanobench.h

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

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/node/caches.cpp
• src/node/miner.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_hd.py
• test/functional/mining_prioritisetransaction.py
• test/functional/wallet_basic.py
• test/functional/wallet_multiwallet.py
• test/functional/rpc_rawtransaction.py
• test/functional/rpc_help.py
• test/functional/rpc_blockchain.py

src/{crc32c,dashbls,gsl,immer,leveldb,minisketch,secp256k1,univalue,crypto/{ctaes,x11}}/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/univalue/lib/univalue.cpp
• src/univalue/include/univalue.h

🧠 Learnings (27)

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

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-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/test/rpc_util_tests.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/bitcoind.cpp
• test/functional/wallet_hd.py
• src/rpc/server.cpp
• src/wallet/wallet.h
• src/rpc/mempool.cpp
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/util.cpp
• src/wallet/rpc/util.h
• test/functional/mining_prioritisetransaction.py
• src/univalue/lib/univalue.cpp
• test/functional/wallet_basic.py
• src/wallet/wallet.cpp
• test/functional/wallet_multiwallet.py
• src/Makefile.test.include
• test/functional/rpc_rawtransaction.py
• test/functional/rpc_help.py
• src/wallet/rpc/spend.cpp
• test/functional/rpc_blockchain.py
• src/wallet/test/fuzz/parse_iso8601.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/rpc_util_tests.cpp
• src/test/util_tests.cpp
• src/Makefile.test.include
• src/wallet/test/fuzz/parse_iso8601.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/test/rpc_util_tests.cpp
• test/functional/wallet_hd.py
• src/wallet/rpc/coins.cpp
• test/functional/wallet_basic.py
• test/functional/wallet_multiwallet.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/validation_chainstatemanager_tests.cpp
• test/functional/wallet_hd.py
• test/README.md
• test/functional/mining_prioritisetransaction.py
• test/functional/wallet_basic.py
• test/functional/wallet_multiwallet.py
• test/functional/rpc_rawtransaction.py
• test/functional/rpc_blockchain.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/node/chainstate.{cpp,h} : Chainstate initialization must be separated into dedicated src/node/chainstate.* files

Applied to files:

• src/test/validation_chainstatemanager_tests.cpp
• src/zmq/zmqpublishnotifier.cpp
• src/node/caches.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/bench/**/*.{cpp,h} : Performance benchmarks in src/bench/ must use the nanobench library

Applied to files:

• src/bench/nanobench.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/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs

Applied to files:

• src/wallet/test/fuzz/coinselection.cpp
• src/wallet/rpc/coins.cpp
• src/Makefile.test.include

📚 Learning: 2025-01-02T08:33:26.751Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: ci/test/00_setup_env_mac.sh:11-11
Timestamp: 2025-01-02T08:33:26.751Z
Learning: The removal of DMG support in the macOS packaging process eliminates the need for python-based scripts or python3-setuptools in the build environment. The PACKAGES variable in ci/test/00_setup_env_mac.sh is vestigial due to the use of a general-purpose Docker container defined in contrib/containers/ci/Dockerfile.

Applied to files:

• ci/test/04_install.sh

📚 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/zmq/zmqpublishnotifier.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/zmq/zmqpublishnotifier.cpp
• src/zmq/zmqnotificationinterface.h

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features

Applied to files:

• src/zmq/zmqpublishnotifier.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/zmq/zmqpublishnotifier.cpp
• src/zmq/zmqnotificationinterface.h
• src/node/caches.cpp
• src/wallet/rpc/coins.cpp
• src/wallet/wallet.cpp
• src/Makefile.test.include
• src/validation.h
• src/wallet/test/fuzz/parse_iso8601.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/zmq/zmqpublishnotifier.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

Applied to files:

• src/zmq/zmqpublishnotifier.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h} : Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Applied to files:

• src/node/caches.cpp

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

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite

Applied to files:

• src/wallet/wallet.h
• src/Makefile.test.include

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

• src/wallet/rpc/coins.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/rpc/coins.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/rpc/coins.cpp
• src/node/miner.cpp

📚 Learning: 2025-10-03T11:29:36.358Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6838
File: src/governance/signing.cpp:84-85
Timestamp: 2025-10-03T11:29:36.358Z
Learning: After bitcoin#25153 (included in dash#6775), UniValue uses the templated getInt<T>() API instead of the legacy get_int() and get_int64() methods. For example, use jproposal["start_epoch"].getInt<int64_t>() to retrieve int64_t values from UniValue objects.

Applied to files:

• src/wallet/rpc/util.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/governance/**/*.{cpp,h} : Governance implementation must support governance objects (proposals, triggers, superblock management) and on-chain voting with tallying

Applied to files:

• src/zmq/zmqpublishnotifier.h

📚 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.test.include

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

📚 Learning: 2025-11-25T10:53:37.523Z

Learnt from: knst
Repo: dashpay/dash PR: 7008
File: src/masternode/sync.h:17-18
Timestamp: 2025-11-25T10:53:37.523Z
Learning: In Dash Core, `DEFAULT_SYNC_MEMPOOL` belongs with node synchronization logic (currently in src/masternode/sync.h) rather than with validation logic in src/validation.h, even though other mempool-related constants are in validation.h. The constant is conceptually tied to sync functionality, not validation.

Applied to files:

• src/validation.h

📚 Learning: 2025-01-14T09:06:19.717Z

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

Applied to files:

• src/wallet/test/fuzz/parse_iso8601.cpp

📚 Learning: 2025-09-09T21:36:11.833Z

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

Applied to files:

• src/sync.h

🧬 Code graph analysis (12)

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

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

• ParseISO8601DateTime (21-33)
• ParseISO8601DateTime (21-21)

test/functional/wallet_hd.py (1)

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

• assert_raises_rpc_error (132-148)

src/rpc/server.cpp (1)

src/rpc/request.cpp (3)

• request (31-31)
• JSONRPCError (56-62)
• JSONRPCError (56-56)

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

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

• ParseISO8601DateTime (21-33)
• ParseISO8601DateTime (21-21)

test/functional/mining_prioritisetransaction.py (1)

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

• assert_raises_rpc_error (132-148)

src/univalue/lib/univalue.cpp (1)

src/script/miniscript.h (1)

• typ (153-153)

test/functional/wallet_basic.py (1)

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

• assert_raises_rpc_error (132-148)

src/wallet/wallet.cpp (1)

src/util/time.cpp (2)

• GetTime (63-63)
• GetTime (63-63)

test/functional/rpc_rawtransaction.py (1)

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

• assert_raises_rpc_error (132-148)

test/functional/rpc_help.py (1)

src/rpc/server.cpp (4)

• help (82-143)
• help (82-82)
• help (145-175)
• help (145-145)

src/wallet/test/fuzz/parse_iso8601.cpp (1)

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

• ParseISO8601DateTime (21-33)
• ParseISO8601DateTime (21-21)

src/sync.h (2)

src/validation.cpp (2)

• LOCKS_EXCLUDED (3415-3436)
• LOCKS_EXCLUDED (3438-3444)

src/validationinterface.cpp (1)

• WAIT_LOCK (83-83)

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

src/test/validation_chainstatemanager_tests.cpp (1)

342-342: Verify that the comment typo correction originates from the backported Bitcoin Core PR.

Per the learnings from previous backport PRs, typos in comments should be kept as-is to reduce merge conflicts in future backports, unless they are part of the original upstream Bitcoin Core PR being backported. Clarify whether the "verfiy" → "verify" correction at Line 342 is from the original Bitcoin Core commit or a Dash-specific change.

If this correction is part of the original Bitcoin Core backport PR, it should be kept. If it's Dash-specific, it should be reverted to match upstream and avoid future merge conflicts per learnings from PR #6871.

src/node/miner.cpp (1)

431-437: Unified insert/modify pattern for mapModifiedTx is correct

Using mit = mapModifiedTx.insert(modEntry).first; followed by mapModifiedTx.modify(mit, update_for_parent_inclusion(it)); correctly handles both cases (new and existing entries) given boost::multi_index semantics, and keeps descendant accounting consistent while simplifying the logic. No issues from a correctness or concurrency perspective, and it aligns with the upstream backport behavior.

src/bench/nanobench.h (1)

901-901: LGTM! Typo fix improves documentation.

The correction from "detials" to "details" in the comment is a straightforward documentation improvement that enhances readability.

ci/lint/04_install.sh (1)

36-36: LGTM! Straightforward dependency bump for codespell.

ci/lint/06_script.sh (1)

41-42: LGTM! Proper CI identity setup before GPG operations.

ci/test/04_install.sh (1)

86-97: LGTM! Clean consolidation of pip install logic with OS-specific routing.

src/zmq/zmqabstractnotifier.h (1)

8-8: LGTM!

The addition of <cstdint> is appropriate for the fixed-width integer types (uint64_t, uint32_t) used in method signatures and member variables throughout this header.

src/zmq/zmqnotificationinterface.h (1)

8-15: LGTM!

The added includes and forward declaration properly support the types used in the interface methods. The <primitives/transaction.h> provides CTransactionRef, <cstdint> supports fixed-width integer types like int64_t and uint64_t, and the CBlock forward declaration is used in method parameters.

src/zmq/zmqpublishnotifier.h (1)

10-16: LGTM!

The added includes and forward declaration are appropriate. <cstddef> provides size_t used in line 36, <cstdint> provides uint32_t used in line 26, and the CTransaction forward declaration supports method parameters without requiring the full header inclusion.

src/zmq/zmqnotificationinterface.cpp (1)

7-23: LGTM!

The added include directives provide necessary dependencies for the implementation. These are standard header hygiene improvements with no functional changes to the code.

src/zmq/zmqrpc.cpp (1)

14-17: LGTM!

The forward declaration of JSONRPCRequest instead of including the full header is a good practice to reduce coupling. The added standard library includes support types used in the implementation.

src/zmq/zmqpublishnotifier.cpp (1)

9-17: LGTM on other include additions!

The added project includes, standard library headers, and forward declaration for Consensus::Params are appropriate for supporting the implementation's dependencies. These are standard header hygiene improvements.

Also applies to: 20-26, 33-46

src/Makefile.test.include (2)

208-223: Wire new wallet RPC util unit test into test suite

wallet/test/rpc_util_tests.cpp is correctly added to BITCOIN_TESTS under ENABLE_WALLET alongside other wallet tests, so it will only build and run when wallets are enabled. This is consistent with the existing layout and has no Makefile issues.

---

233-236: Centralize wallet fuzz sources and move parse_iso8601 into wallet scope

Defining FUZZ_WALLET_SRC to include wallet/test/fuzz/coincontrol.cpp, wallet/test/fuzz/coinselection.cpp, and the new wallet/test/fuzz/parse_iso8601.cpp and then reusing it in test_fuzz_fuzz_SOURCES cleanly scopes these fuzz targets to ENABLE_WALLET without duplicating entries. This also properly relocates the parse_iso8601 fuzzer into the wallet fuzz set.

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

251-312: sendmany: verbose argument and parameter indexing are consistent

The new verbose RPC argument and its use (request.params[11]) line up correctly with the argument list and mirror the sendtoaddress behavior. Help text is clear and consistent with other RPCs.

---

369-449: FundTransaction options validation via RPCTypeCheckObj looks correct

The switch to RPCTypeCheckObj with an explicit key/type schema preserves the legacy bool-only fallback, supports both camelCase/snake_case aliases, and cleanly feeds into SetFeeEstimateMode. The subsequent options.exists(...) checks align with the declared schema, and no invalid combinations or missing defaults are apparent.

src/wallet/test/fuzz/coinselection.cpp (1)

34-34: LGTM!

Typo correction in comment is accurate.

test/lint/spelling.ignore-words.txt (1)

4-25: LGTM!

Updates to the spelling ignore list are appropriate for maintaining lint checks across backported changes.

src/index/coinstatsindex.h (1)

14-15: LGTM!

Relocating the default constant to the appropriate header improves modularity and organization.

doc/release-notes-empty-template.md (1)

22-22: LGTM!

Correct capitalization of macOS improves documentation consistency.

test/lint/lint-locale-dependence.py (1)

257-257: LGTM!

Typo corrections improve the quality of the error message.

src/bitcoind.cpp (1)

225-225: LGTM!

Typo correction in error message is accurate.

src/index/blockfilterindex.h (1)

15-16: LGTM!

Relocating the default constant to the appropriate header improves code organization.

src/wallet/wallet.cpp (1)

1951-1954: LGTM!

The variable rename from nNextResend to m_next_resend follows proper naming conventions for member variables. The resend timing logic (1-3 hour randomization, first-run skip, broadcast flag check) is preserved correctly.

src/rpc/mempool.cpp (1)

632-635: Backport change looks correct.

The removal of explicit per-argument type validation in favor of direct get_array() extraction aligns with the broader pattern in this backport. The get_array() method will throw UniValue::type_error if the parameter is not an array, maintaining type safety. The empty-check on line 633 provides proper validation of the array contents.

src/index/txindex.h (1)

10-11: Constant relocation looks good.

Moving DEFAULT_TXINDEX from validation.h to the index-specific header is a cleaner organization that places the default near its relevant class. The static constexpr ensures proper internal linkage.

src/node/caches.cpp (1)

7-7: Include update correctly follows constant relocation.

The include change from validation.h to index/txindex.h properly tracks the relocation of DEFAULT_TXINDEX. The file only needs this constant (used at line 20), so the more specific include is appropriate.

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

315-315: RPC parameter handling change aligns with backport pattern.

The direct get_array() call replaces explicit type validation. The null check at line 307 handles the omitted parameter case, and get_array() will throw UniValue::type_error if the parameter is provided but has the wrong type.

src/wallet/wallet.h (1)

277-278: Thread-safety improvement for resend timing.

The change from int64_t nNextResend to std::atomic<int64_t> m_next_resend properly addresses potential data races. The wallet's resend logic can be triggered from the scheduler thread while other threads may access this value. This is consistent with m_best_block_time (line 283) which is also atomic for similar reasons.

src/validation.h (1)

81-85: Constants reorganized per upstream backport.

The removal of index-related defaults (moved to their respective headers) and addition of DEFAULT_SYNC_MEMPOOL aligns with the Bitcoin Core v25 backport. The new constant is logically placed near DEFAULT_PERSIST_MEMPOOL which controls related mempool behavior.

src/sync.h (5)

118-118: Type alias renamed to lowercase.

The UniqueLock → unique_lock rename follows standard library naming conventions (std::unique_lock). This is a cosmetic change that maintains API compatibility.

---

175-180: Template simplification looks correct.

The UniqueLock template now takes a single MutexType parameter instead of separate Mutex and Base parameters. The base class is derived internally as MutexType::unique_lock. This simplification reduces template complexity while maintaining the same functionality through the private Base alias.

---

203-220: Constructor signatures updated consistently.

The constructors now take MutexType& and MutexType* instead of Mutex& and Mutex*, matching the renamed template parameter. The lock acquisition logic (Enter/TryEnter) and defer_lock semantics remain unchanged.

---

333-338: Generic MaybeCheckNotHeld templates replace per-type overloads.

The new generic templates handle GlobalMutex, RecursiveMutex, and SharedMutex uniformly with LOCKS_EXCLUDED annotation, while the Mutex-specific overloads (lines 330-331) retain EXCLUSIVE_LOCKS_REQUIRED(!cs) for stricter negative capability checking. This maintains the same thread safety analysis behavior with less code duplication.

---

340-347: Lock macros use explicit template parameters, not CTAD.

The ReadLock<decltype(cs)> usage (lines 346, 347) explicitly specifies the template parameter and is not class template argument deduction. Only UniqueLock in lines 340, 342, 345 may potentially use CTAD if deduction guides are defined, but ReadLock clearly relies on explicit decltype(cs) template specification. Clarify whether the simplification is about macro naming/structure rather than actual CTAD adoption, or update the code to consistently use CTAD across both UniqueLock and ReadLock if that is the intent.

test/functional/mining_prioritisetransaction.py (1)

123-123: LGTM! Test expectation updated for new type error code.

The expected error code correctly changed from -1 to -3, aligning with the introduction of RPC_TYPE_ERROR for type mismatches. This is consistent with other test updates in this backport PR.

test/functional/rpc_help.py (1)

97-97: LGTM! Consistent type error handling.

The error code update from -1 to -3 correctly reflects the new type error handling introduced in this backport.

src/wallet/test/fuzz/parse_iso8601.cpp (3)

8-8: LGTM! Correct header inclusion.

The new header correctly provides access to the wallet-namespaced ParseISO8601DateTime function.

---

24-24: LGTM! Proper namespace qualification.

The function call is correctly qualified with the wallet namespace, matching the refactored location of ParseISO8601DateTime.

---

32-32: LGTM! Consistent namespace usage.

Both calls to ParseISO8601DateTime are correctly qualified with the wallet namespace.

src/test/util_tests.cpp (1)

295-295: LGTM! Test name accurately reflects updated scope.

The test case name correctly updated to reflect that it now only tests formatting, as ParseISO8601DateTime moved to the wallet namespace with its own dedicated tests.

src/univalue/lib/univalue.cpp (1)

210-216: LGTM! Improved exception specificity.

The change from std::runtime_error to type_error enables proper distinction of type mismatches in RPC handlers, allowing them to be mapped to RPC_TYPE_ERROR (-3). The error message construction and logic remain unchanged.

test/functional/wallet_basic.py (1)

456-456: LGTM! Correct type error expectation.

The error code correctly updated to -3 for passing a string where a number is expected, consistent with the new type error handling throughout this backport.

test/functional/wallet_multiwallet.py (1)

356-356: LGTM! Proper error code for missing required parameter.

The error code correctly updated to -3 when unloadwallet is called without the required wallet_name argument, consistent with the new type error handling.

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

45-45: LGTM! Function properly declared in wallet namespace.

The ParseISO8601DateTime function declaration is correctly placed in the wallet namespace, matching the implementation in src/wallet/rpc/util.cpp. This refactoring appropriately moves wallet-specific ISO8601 parsing functionality out of the global util namespace.

test/README.md (1)

303-305: Clarified timeout-factor documentation looks correct

The description of --timeout-factor 0 and the example invocation align with the test framework behavior; no issues here.

src/rpc/server.cpp (1)

526-540: RPC now maps UniValue type errors to RPC_TYPE_ERROR as intended

Catching UniValue::type_error before the generic std::exception and translating it to JSONRPCError(RPC_TYPE_ERROR, ...) cleanly separates type errors from miscellaneous failures and matches the updated tests expecting code -3. No issues spotted with the control flow.

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

1-24: ISO8601 wallet util tests are well-formed and use correct epochs

The Boost.Test suite exercises ParseISO8601DateTime with representative boundary/future dates and matches the intended “pre-epoch yields 0” behavior. Includes and namespace usage are consistent with the wallet RPC util.

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

323-329: Hard-coded time bounds keep ConsumeTime behavior while simplifying dependencies

Using the explicit epoch constants for 2000-01-01T00:00:01Z and 2100-12-31T23:59:59Z preserves the prior range and mocktime-0 avoidance, while decoupling fuzz utilities from the ISO8601 parser. Looks good.

test/functional/wallet_hd.py (1)

187-188: sethdseed type-error expectations updated to RPC_TYPE_ERROR

The two assertions for invalid sethdseed argument types now expect code -3 with type-mismatch messages, which matches the new UniValue::type_error → RPC_TYPE_ERROR handling. No issues.

test/functional/rpc_blockchain.py (3)

269-269: getchaintxstats nblocks type error now correctly uses code -3

The assertion for getchaintxstats with nblocks as a string now expects -3 and a type-mismatch message, aligning with the centralized type-error handling. Surrounding range checks remain unchanged.

---

273-273: getchaintxstats blockhash type error expectation aligned with new behavior

Passing blockhash=0 is now treated as a JSON type error with code -3 and an appropriate “number vs string” message, before falling through to length/format checks. This matches the updated RPC error classification.

---

566-567: getblock invalid verbosity type now mapped to RPC_TYPE_ERROR

Supplying "2" for verbosity now expects error code -3 with a string-vs-number type message, which is consistent with the new UniValue type_error handling.

test/functional/rpc_rawtransaction.py (4)

131-139: getrawtransaction type-error expectations correctly switched to RPC_TYPE_ERROR

The verbose parameter tests now treat invalid types ("True", "False", [], {}) as JSON type errors with code -3 and “not of expected type bool” messages, both for getrawtransaction and getrawtransactionmulti. This matches the new UniValue type_error mapping.

---

160-166: Blockhash type validation now reports a JSON type error before format checks

Passing blockhash=True is now expected to yield -3 and a bool-vs-string type message, with the existing length/hex/lookup validations preserved for string inputs. This is consistent and fine.

---

191-192: createrawtransaction input-type checks aligned with new type_error behavior

The invalid inputs cases (string instead of array, array containing string instead of object, null instead of string txid) now expect code -3 and detailed JSON type-mismatch messages, which matches the centralised type checking in the RPC layer.

---

214-231: createrawtransaction outputs/locktime type errors now consistently use code -3

outputs='foo' and locktime='foo' are now asserted as JSON type errors (-3) with array/number expectation messages, while non-type validations (-5/-8) remain unchanged. This brings these tests in line with the new RPC type-error mapping.

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

23-26: Manual verification required. While the technical analysis of the C++ access control issue is correct, the critical concern is that src/univalue/ is a vendored dependency per project guidelines and should not be modified. Verify whether this PR is introducing changes to vendored code (which would violate coding guidelines) or if this flag is being raised against pre-existing vendored code (which may require a different response).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove duplicate include.

The <streams.h> header is included twice consecutively at lines 18 and 19. Remove the duplicate.

Apply this diff to remove the duplicate:

 #include <serialize.h>
 #include <streams.h>
-#include <streams.h>
 #include <sync.h>

🤖 Prompt for AI Agents

In src/zmq/zmqpublishnotifier.cpp around lines 18 to 19, the header <streams.h>
is included twice; remove the duplicated line so the file contains only a single
#include <streams.h> at that location.

[UdjinM6]

utACK e9939a2 with a nit

[PastaPastaPasta]

utACK e9939a2