backport: bitcoin#25285, #25298, #25331, #25445, #25638, #25691, #25699, #25809, #25976, #26079

[knst]

What was done?

Regular backports from Bitcoin Core v24

Backport bitcoin#25331 is partial due to several missing usages of CHashWritter in non-existing yet taproot code.
Though, it's time to do it, because most of backports prior to 25331 is done and it helps to reduce conflicts for further backports that may use HashWritter instead.

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

The changes introduce a new AutoFile base class and adjust CAutoFile to derive from it (streams.h), replace many CAutoFile usages with AutoFile, and change multiple function signatures and call sites to accept AutoFile&. CHashWriter usages are replaced with HashWriter in many hashing paths and corresponding APIs (hash.*, muhash, interpreter, node, wallet). RPC handlers and tests standardize null returns from NullUniValue to UniValue::VNULL. Build files add BITCOIN_TEST_NAME/BITCOIN_TEST_BIN and the Windows installer packaging now includes the testnet binary. Documentation and several tests were also adjusted (OpenBSD/osx docs, test expectations).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Rationale: Changes are widespread but follow consistent, repetitive patterns (CAutoFile→AutoFile, CHashWriter→HashWriter, NullUniValue→UniValue::VNULL) with a central structural addition (AutoFile/CAutoFile redesign). Review requires checking many call sites for correct type/signature updates and ensuring serialization/hashing semantics preserved, but individual edits are low-complexity and homogeneous.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The pull request title "backport: bitcoin#25285, bitcoin#25298, bitcoin#25331, bitcoin#25445, bitcoin#25638, bitcoin#25691, bitcoin#25699, bitcoin#25809, bitcoin#25976, bitcoin#26079" accurately reflects that the changeset consists of backports from Bitcoin Core PRs, as confirmed by the raw_summary showing extensive changes aligned with these upstream backports (CAutoFile→AutoFile refactoring, CHashWriter→HashWriter replacements, NullUniValue→UniValue::VNULL standardization, and build system updates). However, the title is primarily a list of PR reference numbers rather than a descriptive summary of the actual changes, making it vague and generic—it does not clearly convey to a teammate scanning the commit history what the substantive modifications accomplish or the key technical improvements being introduced.	Consider revising the title to highlight the primary technical changes, such as "Refactor file I/O and hash writer classes (backport bitcoin v24)" or "Replace CAutoFile/CHashWriter with AutoFile/HashWriter (backport bitcoin v24)" to provide clearer context about what the backports accomplish beyond merely listing PR numbers.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The pull request description is directly related to the changeset. It explains that this is a regular backport from Bitcoin Core v24, notes that the backport of bitcoin#25331 is partial due to missing taproot code, provides the rationale for proceeding with the partial backport, confirms testing via unit and functional tests, and clarifies that there are no breaking changes. This information meaningfully describes the nature and intent of the changes present in the changeset.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8e276e and 312e438.

📒 Files selected for processing (1)

• test/functional/rpc_blockchain.py (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be written in Python and placed in test/functional/

Files:

• test/functional/rpc_blockchain.py

🧠 Learnings (1)

📓 Common learnings

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

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

🧬 Code graph analysis (1)

test/functional/rpc_blockchain.py (3)

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

• dashhash (92-93)

src/rpc/blockchain.cpp (2)

• getblock (913-1032)
• getblock (913-913)

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

• assert_equal (69-74)

🔇 Additional comments (3)

test/functional/rpc_blockchain.py (3)

38-38: LGTM! Import is correctly used.

The dashhash import is properly utilized in the new test helper function at line 497.

---

495-497: Excellent validation logic for block hash verification.

The function correctly validates that the hex-encoded block data hashes to the expected blockhash by:

1. Extracting the 80-byte block header ([:160])
2. Hashing with Dash's X11 algorithm (dashhash)
3. Reversing byte order (block hashes are stored reversed)
4. Comparing with the expected blockhash

This provides good test coverage for the getblock RPC with verbosity 0.

---

533-539: Good test coverage for verbosity parameter variants.

The tests appropriately verify both integer and boolean forms of the verbosity parameter:

• Lines 534-535: Test verbosity 0 and False (both return hex block)
• Line 539: Test verbosity True (equivalent to 1, returns JSON without fees)

This ensures the RPC handles parameter type flexibility correctly.

---

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/script/interpreter.cpp (1)

1469-1498: Restore sha256d hashing for witness preimages

GetPrevoutsSHA256, GetSequencesSHA256, and GetOutputsSHA256 now call GetSHA256(), which produces a single-round SHA256. Prior to this change CHashWriter::GetHash() returned the standard Bitcoin double-SHA (sha256d), matching BIP143/BIP341 requirements for the witness digest. Switching to a single-round hash alters the serialized preimage and breaks signature verification consensus-wide. Please keep these helpers returning the sha256d result (e.g., by using GetHash() on HashWriter).

-    return ss.GetSHA256();
+    return ss.GetHash();

Apply the same fix to the other two helpers.

src/node/coinstats.cpp (1)

59-60: Restore height/coinbase encoding in serialized hash.

Dropping the parentheses makes the ternary operate on the whole sum, so we now serialize only 0 or 1 for each output cluster. That completely changes the legacy hash used by CoinStats snapshots/assumeutxo validation, breaking snapshot creation and verification on this branch.

-            ss << VARINT(it->second.nHeight * 2 + it->second.fCoinBase ? 1u : 0u);
+            ss << VARINT(it->second.nHeight * 2 + (it->second.fCoinBase ? 1u : 0u));

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee41b18 and d8e276e.

📒 Files selected for processing (57)

• Makefile.am (2 hunks)
• configure.ac (2 hunks)
• doc/build-openbsd.md (1 hunks)
• doc/build-osx.md (0 hunks)
• share/setup.nsi.in (1 hunks)
• src/crypto/muhash.cpp (2 hunks)
• src/flat-database.h (2 hunks)
• src/hash.cpp (1 hunks)
• src/hash.h (1 hunks)
• src/index/blockfilterindex.cpp (4 hunks)
• src/index/txindex.cpp (1 hunks)
• src/net.cpp (1 hunks)
• src/node/blockstorage.cpp (1 hunks)
• src/node/coinstats.cpp (3 hunks)
• src/node/interfaces.cpp (8 hunks)
• src/node/txreconciliation.cpp (1 hunks)
• src/policy/fees.cpp (6 hunks)
• src/policy/fees.h (2 hunks)
• src/rpc/blockchain.cpp (10 hunks)
• src/rpc/blockchain.h (1 hunks)
• src/rpc/coinjoin.cpp (7 hunks)
• src/rpc/evo.cpp (5 hunks)
• src/rpc/governance.cpp (4 hunks)
• src/rpc/masternode.cpp (2 hunks)
• src/rpc/mining.cpp (2 hunks)
• src/rpc/net.cpp (8 hunks)
• src/rpc/node.cpp (4 hunks)
• src/rpc/rawtransaction.cpp (5 hunks)
• src/script/interpreter.cpp (4 hunks)
• src/streams.h (3 hunks)
• src/test/flatfile_tests.cpp (3 hunks)
• src/test/fuzz/autofile.cpp (1 hunks)
• src/test/fuzz/parse_univalue.cpp (1 hunks)
• src/test/fuzz/policy_estimator.cpp (1 hunks)
• src/test/fuzz/policy_estimator_io.cpp (1 hunks)
• src/test/fuzz/util.h (1 hunks)
• src/test/fuzz/utxo_snapshot.cpp (1 hunks)
• src/test/util/chainstate.h (3 hunks)
• src/test/validation_chainstatemanager_tests.cpp (2 hunks)
• src/util/asmap.cpp (1 hunks)
• src/util/message.cpp (1 hunks)
• src/validation.cpp (4 hunks)
• src/validation.h (2 hunks)
• src/wallet/dump.cpp (2 hunks)
• src/wallet/rpc/addresses.cpp (12 hunks)
• src/wallet/rpc/backup.cpp (23 hunks)
• src/wallet/rpc/coins.cpp (8 hunks)
• src/wallet/rpc/encrypt.cpp (7 hunks)
• src/wallet/rpc/signmessage.cpp (1 hunks)
• src/wallet/rpc/spend.cpp (8 hunks)
• src/wallet/rpc/transactions.cpp (8 hunks)
• src/wallet/rpc/wallet.cpp (13 hunks)
• test/functional/feature_addrman.py (1 hunks)
• test/functional/feature_dbcrash.py (1 hunks)
• test/functional/feature_fee_estimation.py (1 hunks)
• test/functional/rpc_blockchain.py (3 hunks)
• test/functional/rpc_mempool_info.py (0 hunks)

💤 Files with no reviewable changes (2)

• test/functional/rpc_mempool_info.py
• doc/build-osx.md

🧰 Additional context used

📓 Path-based instructions (4)

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

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/wallet/rpc/backup.cpp
• src/rpc/masternode.cpp
• src/rpc/governance.cpp
• src/node/blockstorage.cpp
• src/rpc/evo.cpp
• src/node/txreconciliation.cpp
• src/rpc/coinjoin.cpp
• src/hash.cpp
• src/hash.h
• src/test/fuzz/policy_estimator_io.cpp
• src/rpc/node.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/wallet/rpc/spend.cpp
• src/rpc/mining.cpp
• src/wallet/rpc/signmessage.cpp
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/transactions.cpp
• src/test/fuzz/policy_estimator.cpp
• src/wallet/rpc/encrypt.cpp
• src/flat-database.h
• src/wallet/rpc/addresses.cpp
• src/test/flatfile_tests.cpp
• src/rpc/blockchain.h
• src/wallet/rpc/wallet.cpp
• src/policy/fees.cpp
• src/script/interpreter.cpp
• src/test/fuzz/util.h
• src/net.cpp
• src/rpc/net.cpp
• src/crypto/muhash.cpp
• src/util/asmap.cpp
• src/test/fuzz/autofile.cpp
• src/node/interfaces.cpp
• src/rpc/rawtransaction.cpp
• src/policy/fees.h
• src/index/txindex.cpp
• src/wallet/dump.cpp
• src/index/blockfilterindex.cpp
• src/validation.cpp
• src/node/coinstats.cpp
• src/validation.h
• src/test/fuzz/parse_univalue.cpp
• src/util/message.cpp
• src/test/fuzz/utxo_snapshot.cpp
• src/rpc/blockchain.cpp
• src/test/util/chainstate.h
• src/streams.h

src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests

Files:

• src/test/fuzz/policy_estimator_io.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/test/fuzz/policy_estimator.cpp
• src/test/flatfile_tests.cpp
• src/test/fuzz/util.h
• src/test/fuzz/autofile.cpp
• src/test/fuzz/parse_univalue.cpp
• src/test/fuzz/utxo_snapshot.cpp
• src/test/util/chainstate.h

test/functional/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Functional tests should be written in Python and placed in test/functional/

Files:

• test/functional/feature_addrman.py
• test/functional/rpc_blockchain.py
• test/functional/feature_fee_estimation.py
• test/functional/feature_dbcrash.py

doc/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the doc directory (documentation)

Files:

• doc/build-openbsd.md

🧠 Learnings (4)

📓 Common learnings

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

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

📚 Learning: 2025-08-08T07:01:47.332Z

Learnt from: knst
PR: dashpay/dash#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/rpc/backup.cpp
• src/rpc/masternode.cpp
• src/rpc/governance.cpp
• src/rpc/evo.cpp
• src/rpc/coinjoin.cpp
• src/rpc/node.cpp
• src/wallet/rpc/spend.cpp
• src/rpc/mining.cpp
• src/wallet/rpc/signmessage.cpp
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/rpc/encrypt.cpp
• src/wallet/rpc/addresses.cpp
• src/wallet/rpc/wallet.cpp
• src/rpc/net.cpp
• src/rpc/blockchain.cpp

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

Learnt from: kwvg
PR: dashpay/dash#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/rpc/backup.cpp
• src/rpc/governance.cpp
• src/wallet/rpc/spend.cpp
• src/wallet/rpc/coins.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/rpc/wallet.cpp

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

Learnt from: kwvg
PR: dashpay/dash#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:

• configure.ac

🧬 Code graph analysis (15)

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

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

• wallet (301-301)

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

• wallet (18-45)

src/rpc/masternode.cpp (1)

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

• wallet (301-301)

src/rpc/governance.cpp (1)

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

• wallet (301-301)

src/node/txreconciliation.cpp (1)

src/hash.cpp (2)

• TaggedHash (85-92)
• TaggedHash (85-85)

src/hash.h (1)

src/hash.cpp (2)

• TaggedHash (85-92)
• TaggedHash (85-85)

src/test/validation_chainstatemanager_tests.cpp (1)

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

• auto_infile (44-51)

test/functional/rpc_blockchain.py (2)

test/functional/test_framework/messages.py (2)

• dashhash (92-93)
• hash256 (89-90)

src/rpc/blockchain.cpp (2)

• getblock (913-1032)
• getblock (913-913)

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

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

• wallet (18-45)

src/flat-database.h (3)

src/fs.h (1)

• fsbridge (206-241)

src/policy/fees.cpp (2)

• fileout (166-166)
• filein (172-172)

src/addrdb.cpp (2)

• fileout (58-58)
• filein (117-117)

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

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

• EnsureLegacyScriptPubKeyMan (96-106)
• EnsureLegacyScriptPubKeyMan (96-96)

src/policy/fees.cpp (1)

src/flat-database.h (2)

• fileout (54-72)
• filein (81-156)

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

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

• open (480-519)
• open (480-480)

src/streams.h (1)

• AutoFile (493-580)

src/policy/fees.h (2)

src/streams.h (3)

• AutoFile (493-580)
• Write (455-470)
• Read (407-426)

src/policy/fees.cpp (10)

• Write (408-416)
• Write (408-408)
• Write (929-952)
• Write (929-929)
• fileout (166-166)
• Read (418-472)
• Read (418-418)
• Read (954-1014)
• Read (954-954)
• filein (172-172)

src/index/blockfilterindex.cpp (3)

src/flat-database.h (2)

• filein (81-156)
• fileout (54-72)

src/policy/fees.cpp (2)

• filein (172-172)
• fileout (166-166)

src/addrdb.cpp (2)

• filein (117-117)
• fileout (58-58)

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

src/streams.h (1)

• AutoFile (493-580)

src/node/utxo_snapshot.h (1)

• SnapshotMetadata (15-34)

🪛 Flake8 (7.3.0)

test/functional/rpc_blockchain.py

[error] 43-43: 'test_framework.script.hash256' imported but unused

(F401)

🔇 Additional comments (28)

test/functional/feature_dbcrash.py (1)

195-200: Good practice: trailing comma improves diff clarity.

The addition of the trailing comma after fee_per_output=FEE // 3, is idiomatic Python and makes the diff cleaner when adding or modifying parameters in the future. No semantic impact on test behavior.

src/rpc/rawtransaction.cpp (1)

151-152: Documentation improvements for RPC result descriptions.

These updates enhance the clarity of RPC help documentation by providing more specific descriptions for script-related fields (signature scripts, public key scripts, and redeem scripts). The changes are purely documentation—no functional or behavioral changes. This aligns well with the PR objective of backporting Bitcoin Core documentation corrections.

Also applies to: 165-165, 167-167, 1036-1037, 1051-1052, 1095-1096

doc/build-openbsd.md (1)

39-39: Correct OpenBSD package manager syntax.

Removing "install" from the pkg_add command fixes invalid OpenBSD syntax. The correct form is pkg_add <package> without an "install" argument.

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

30-30: LGTM! Consistent with null representation standardization.

The change from the previous null representation to UniValue{} aligns with the PR's objective to standardize null/empty UniValue representations across the codebase. Since valid is set to false before this return and triggers an early exit on line 34, the constructed value is never used, making this change safe and purely for consistency.

src/node/interfaces.cpp (1)

831-831: LGTM! Consistent refactoring to use accessor method.

The changes consistently replace direct member access (m_node.chainman->) with the chainman() accessor method throughout the ChainImpl class. This improves encapsulation and maintains null safety via the Assert() in the accessor. The refactoring is part of the Bitcoin Core v24 backport and follows the same pattern already used elsewhere in the file.

Also applies to: 841-841, 849-849, 886-886, 921-922, 927-927, 933-934, 944-946, 953-955, 1058-1058, 1080-1080, 1133-1133

src/rpc/masternode.cpp (2)

138-138: LGTM! Null return standardization.

The change from NullUniValue to UniValue::VNULL correctly standardizes the null return value when no wallet is available. This is consistent with the broader refactoring to use explicit UniValue::VNULL across RPC handlers.

---

245-245: LGTM! Null return standardization.

The change from NullUniValue to UniValue::VNULL correctly standardizes the null return value when the chain tip is unavailable. This appropriately handles edge cases such as initial blockchain synchronization or empty chain states.

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

254-254: LGTM! Consistent null-return standardization across wallet RPC handlers.

The replacement of NullUniValue with UniValue::VNULL is applied uniformly across all wallet RPC handlers in this file. This standardizes the null response representation across the RPC surface as part of the broader refactor described in the PR.

Also applies to: 301-301, 494-494, 610-610, 749-749, 822-822, 839-839, 867-867

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

390-403: LGTM! Clean refactor aligning with AutoFile migration.

The removal of the m_fuzzed_data_provider member and simplification of the open() method to return AutoFile directly is consistent with the repository-wide transition from CAutoFile to AutoFile. The class now correctly relies solely on FuzzedFileProvider for file operations.

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

39-39: LGTM! Consistent with wallet RPC null-return standardization.

The change aligns with the PR-wide effort to standardize null responses using UniValue::VNULL across wallet RPC handlers.

src/util/asmap.cpp (1)

201-201: LGTM! Clean migration to AutoFile.

The transition from CAutoFile with serialization parameters to AutoFile with brace initialization is consistent with the repository-wide IO wrapper refactor. The behavioral semantics remain unchanged.

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

23-23: LGTM! Consistent AutoFile adoption in fuzz tests.

The change from CAutoFile to AutoFile with brace initialization aligns with the PR-wide IO wrapper refactor and matches the pattern in related fuzz test files.

test/functional/feature_addrman.py (1)

97-97: LGTM! Test expectation updated for renamed error message.

The update from CAutoFile::read to AutoFile::read in the error message pattern correctly reflects the class name change from the IO wrapper refactor.

src/wallet/dump.cpp (1)

44-44: LGTM! Modern initialization style for HashWriter.

The transition to value initialization (HashWriter hasher{}) from explicit constructor parameters is a clean modernization that aligns with the broader HashWriter refactor pattern in this PR.

Also applies to: 135-135

configure.ac (1)

34-34: LGTM! Build configuration addition for test binary packaging.

The addition of BITCOIN_TEST_NAME and its substitution follows standard autoconf patterns and enables test binary packaging in the Windows installer, as described in the AI summary.

Also applies to: 1936-1936

src/node/txreconciliation.cpp (1)

18-18: LGTM: Consistent type refactoring.

The replacement of CHashWriter with HashWriter aligns with the repository-wide refactor. The usage pattern at line 28 with copy construction from this constant remains valid.

src/node/blockstorage.cpp (1)

508-508: LGTM: Correct replacement of hash writer type.

The migration from CHashWriter to HashWriter with default construction is consistent with the broader refactor. The subsequent usage with streaming operators and GetHash() remains valid.

src/flat-database.h (2)

52-54: LGTM: Consistent file I/O wrapper refactoring.

The migration from CAutoFile to AutoFile with brace-initialization is correct. The serialization type and version parameters are removed here but maintained in the CDataStream usage at line 45, preserving the overall serialization semantics.

---

79-81: LGTM: Consistent file I/O wrapper refactoring.

The migration from CAutoFile to AutoFile aligns with the repository-wide refactor. Serialization parameters are appropriately managed by the CDataStream at line 108.

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

20-20: LGTM: Correct test adaptation to new type.

The migration to AutoFile with brace-initialization is consistent with the refactor. The removal of GetType() and GetVersion() calls (mentioned in the summary) is correct since AutoFile doesn't expose serialization type/version information.

src/test/flatfile_tests.cpp (1)

47-89: LGTM: Consistent test suite adaptation.

All CAutoFile to AutoFile replacements with brace-initialization are mechanically correct and consistent. The test logic remains unchanged, and all file operations continue to work as expected with the new wrapper type.

share/setup.nsi.in (1)

83-83: LGTM: Test binary packaging added to installer.

The addition of @BITCOIN_TEST_NAME@@EXEEXT@ to the Windows installer is consistent with the corresponding build system changes in Makefile.am. The placement in the daemon directory aligns with other utility binaries.

src/index/blockfilterindex.cpp (1)

169-239: LGTM: Consistent file I/O wrapper refactoring.

All migrations from CAutoFile to AutoFile with brace-initialization are mechanically correct across CommitInternal, ReadFilterFromDisk, and WriteFilterToDisk. The file operations and error handling remain unchanged and valid.

Makefile.am (2)

25-25: LGTM: Test binary variable definition.

The addition of BITCOIN_TEST_BIN follows the established pattern for other binary variables and enables consistent test binary packaging for the Windows installer.

---

81-81: LGTM: Test binary packaging integration.

The stripping and copying of BITCOIN_TEST_BIN to the release directory is consistent with the handling of other binaries and aligns with the corresponding changes in share/setup.nsi.in.

src/policy/fees.h (1)

22-22: LGTM! AutoFile refactoring is consistent.

The forward declaration and method signatures have been correctly updated from CAutoFile to AutoFile, aligning with the broader I/O wrapper refactoring introduced in src/streams.h. The corresponding implementation in src/policy/fees.cpp has been updated accordingly.

Also applies to: 221-226

src/rpc/net.cpp (1)

93-93: LGTM! Standardized null return representation.

All RPC handlers correctly updated from NullUniValue to UniValue::VNULL for consistent null representation across the codebase. No functional changes.

Also applies to: 343-343, 359-359, 469-469, 807-807, 875-875, 896-896, 1082-1082

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

36-36: LGTM! Standardized null returns across wallet RPC handlers.

All wallet RPC handlers correctly updated to return UniValue::VNULL instead of NullUniValue, maintaining consistent null representation. The early-return checks for null wallet pointers and void RPC completions are unchanged in behavior.

Also applies to: 74-74, 107-107, 124-124, 156-156, 223-223, 279-279, 302-302, 327-327, 334-334, 474-474, 584-584, 647-647

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Remove unused import flagged by flake8

hash256 is imported but never used, and the flake8 report (F401) shows the lint job will fail until it’s removed. Please drop the import (or use it) so the test suite passes.

-from test_framework.script import hash256

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	from_hex,
	msg_block,
	)
	from test_framework.p2p import P2PInterface
	from test_framework.script import hash256
	from test_framework.test_framework import BitcoinTestFramework
	from_hex,
	msg_block,
	)
	from test_framework.p2p import P2PInterface
	from test_framework.test_framework import BitcoinTestFramework

🧰 Tools

🪛 Flake8 (7.3.0)

[error] 43-43: 'test_framework.script.hash256' imported but unused

(F401)

🤖 Prompt for AI Agents

In test/functional/rpc_blockchain.py around lines 39 to 44, the import list
includes hash256 which is not used; remove hash256 from the import statement to
satisfy flake8 F401 (or alternatively use it where intended). Update the import
line so only used symbols are imported to eliminate the unused-import linter
error.