Reviewing files that changed from the base of the PR and between 9ef4783 and 98e358d.

• .cirrus.yml (2 hunks)
• contrib/devtools/symbol-check.py (1 hunks)
• contrib/guix/libexec/prelude.bash (1 hunks)
• contrib/guix/manifest.scm (1 hunks)
• contrib/guix/patches/gcc-remap-guix-store.patch (1 hunks)
• contrib/guix/patches/winpthreads-remap-guix-store.patch (1 hunks)
• src/qt/bitcoin.cpp (3 hunks)
• src/qt/optionsdialog.cpp (1 hunks)
• src/rpc/mempool.cpp (3 hunks)
• src/univalue/include/univalue.h (2 hunks)
• src/util/asmap.cpp (1 hunks)
• src/util/bip32.cpp (1 hunks)
• src/util/bip32.h (0 hunks)
• src/util/bytevectorhash.cpp (1 hunks)
• src/util/bytevectorhash.h (1 hunks)
• src/util/error.cpp (1 hunks)
• src/util/hasher.cpp (1 hunks)
• src/util/hasher.h (1 hunks)
• src/util/message.cpp (1 hunks)
• src/util/message.h (1 hunks)
• src/util/moneystr.cpp (1 hunks)
• src/util/readwritefile.cpp (1 hunks)
• src/util/serfloat.h (1 hunks)
• src/util/spanparsing.cpp (1 hunks)
• src/util/strencodings.cpp (1 hunks)
• src/util/strencodings.h (1 hunks)
• src/util/syserror.cpp (1 hunks)
• src/util/thread.cpp (1 hunks)
• src/util/time.cpp (1 hunks)
• src/util/time.h (1 hunks)
• src/util/translation.h (1 hunks)
• src/util/vector.h (1 hunks)
• src/wallet/coinselection.cpp (0 hunks)
• src/wallet/rpc/transactions.cpp (3 hunks)
• src/wallet/test/coinselector_tests.cpp (2 hunks)
• test/functional/feature_maxuploadtarget.py (2 hunks)
• test/functional/mempool_limit.py (1 hunks)
• test/functional/mining_prioritisetransaction.py (1 hunks)
• test/functional/test_framework/util.py (1 hunks)
• test/functional/wallet_importdescriptors.py (1 hunks)

• src/util/bip32.h
• src/wallet/coinselection.cpp

• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: win64-build / Build source

contrib/guix/manifest.scm (2)

131-133: LGTM: Consistent patch wrapper implementation.

The new winpthreads-patches function follows the established pattern used by other patch wrapper functions in this file (gcc-mingw-patches, binutils-mingw-patches). The implementation correctly applies the new patch to ensure reproducible builds.

---

139-141: LGTM: Proper integration of winpthreads patches.

The changes correctly integrate the winpthreads-patches wrapper into the MinGW cross-toolchain build process. This ensures the patch is consistently applied during the pthreads libc component build.

contrib/guix/patches/gcc-remap-guix-store.patch (1)

14-14: LGTM: Consistent implementation for GCC reproducible builds.

This patch applies the same reproducible build strategy used in the winpthreads patch, ensuring consistency across the toolchain. The modification correctly preserves the existing -fexceptions flag while adding the dynamically generated -ffile-prefix-map flags.

The approach effectively removes architecture-dependent Guix store paths from debug symbols, contributing to reproducible builds.

src/util/syserror.cpp (1)

12-13: Explicitly include <string> for std::string usage.

Returning std::string from SysErrorString requires the <string> header; this addition makes the dependency explicit and avoids relying on transitive includes.

src/util/moneystr.cpp (1)

13-13: Add <cstdint> to support fixed-width integer types.

The functions in this file use int64_t for calculations; including <cstdint> ensures those type definitions are available in a portable, standardized way.

src/util/time.h (1)

12-12: Modernize header: include <cstdint> instead of C-style headers.

Multiple functions and templates in this file return or manipulate int64_t; adding <cstdint> makes the dependency on fixed-width integer types explicit and consistent with C++ conventions.

src/util/spanparsing.cpp (1)

9-10: Include <algorithm> and <cstddef> for std::equal and size_t.

The parsing logic relies on std::equal (from <algorithm>) and size_t (from <cstddef>). Adding these headers prevents potential missing‐include errors and aligns with the project's modernization of utility modules.

src/util/serfloat.h (1)

8-8: Replace C header with <cstdint> to provide uint64_t.

Encoding and decoding functions use uint64_t; including the C++ <cstdint> header ensures the fixed-width integer type is available and follows modern C++ header conventions.

src/util/translation.h (1)

11-11: LGTM! Header modernization improves dependency clarity.

The change from <utility> to <string> is appropriate since the file extensively uses std::string but doesn't use utility functions like std::pair or std::move. This reduces unnecessary dependencies.

src/util/readwritefile.cpp (1)

10-10: Essential header addition for std::min usage.

The <algorithm> include is necessary for the std::min call on line 24. This ensures proper compilation and resolves the dependency.

src/util/thread.cpp (1)

12-12: Essential header addition for std::function usage.

The <functional> include is necessary for the std::function<void()> parameter type used in the TraceThread function on line 14.

src/util/error.cpp (1)

10-11: Essential header additions for standard library usage.

Both header additions are necessary:

• <cassert> is required for the assert(false) call on line 38
• <string> is required for the std::string parameters used throughout the file (lines 41, 46, 51, 56)

These direct includes improve dependency clarity and ensure compilation correctness.

src/util/asmap.cpp (2)

10-10: Essential header addition for serialization functionality.

The <serialize.h> include is necessary for the SER_DISK and CLIENT_VERSION constants used on line 201.

---

16-17: Essential header additions for standard library usage.

Both additions are necessary:

• <cstdio> is required for C-style file operations (FILE, fopen, fseek, ftell) used on lines 200, 206-209
• <utility> is required for std::pair used in the vector declaration on line 137

src/util/time.cpp (1)

6-23: Excellent header organization and modernization.

The header reorganization is well-structured:

• Conditional config include follows standard patterns
• All added standard headers are properly utilized in the code:
  • <chrono> for time operations throughout
  • <ctime> for time-related functions
  • <locale> for ParseISO8601DateTime locale handling
  • <sstream> for stringstream operations in parsing
  • <string> for std::string usage

This modernization improves code clarity and ensures proper dependencies.

src/rpc/mempool.cpp (3)

396-396: Excellent addition of incremental relay fee information.

Adding the incrementalrelayfee field provides valuable information about the minimum fee rate increment for mempool limiting and BIP 125 replacement. The implementation correctly uses the global ::incrementalRelayFee variable and follows the same pattern as other fee fields in the function.

---

405-405: Good formatting improvement.

The help text formatting cleanup improves readability.

---

418-418: Comprehensive documentation for the new field.

The help text clearly describes the purpose and units of the incrementalrelayfee field, making it easy for users to understand its role in mempool limiting and BIP 125 replacement.

src/util/hasher.cpp (1)

5-7: Necessary header additions for cryptographic functionality.

The added headers are correctly required for the code:

• <crypto/siphash.h> is needed for CSipHasher used in SaltedSipHasher::operator()
• <span.h> is required for the Span<const unsigned char> parameter in the same function

These additions ensure proper compilation and access to cryptographic utilities.

src/util/vector.h (1)

10-10: Ensure <utility> is included for std::forward usage
The added <utility> header correctly provides std::forward and std::move required by the Vector variadic template.

src/util/bytevectorhash.cpp (1)

9-9: Add <vector> to support std::vector type usage
Including <vector> ensures the operator() method signature referencing std::vector<unsigned char> compiles without errors.

src/util/message.cpp (4)

6-12: Reorganize and include necessary project headers
The added includes (<hash.h>, <key.h>, <key_io.h>, <pubkey.h>, <script/standard.h>, <serialize.h>, <uint256.h>) satisfy the dependencies for message signing and verification functionality.

---

14-14: Include <util/strencodings.h> for Base64 routines
This header is required for DecodeBase64 and EncodeBase64 used in MessageVerify and MessageSign.

---

16-17: Include <cassert> and <optional> for assertions and optional types
<cassert> is needed for the assert(false) in SigningResultString, and <optional> supports the optional return type from DecodeBase64.

---

19-19: Add <variant> for std::variant utilities
The std::get_if<PKHash> usage in MessageVerify relies on <variant>.

src/util/bytevectorhash.h (1)

8-9: Modernize headers: use <cstdint> and <cstddef>
Switching to the C++ standard headers for fixed-width and size types improves portability and adherence to modern C++ practices.

src/util/message.h (1)

13-13: Forward-declare CKey to reduce header coupling
The class CKey; declaration removes the need for including <key.h>, streamlining dependencies in the header.

src/util/strencodings.h (1)

17-17: Header additions are well-justified and necessary.

Each new include serves a specific purpose:

• <cstddef> for size_t usage throughout the file
• <string_view> for the extensive use of std::string_view in function signatures
• <system_error> for std::errc error handling in LocaleIndependentAtoi (lines 128, 136)
• <type_traits> for std::is_integral<T>::value in template constraints (lines 117, 178)

This modernizes the header to explicitly declare its dependencies.

Also applies to: 22-24

test/functional/mining_prioritisetransaction.py (1)

17-17: Test parameter update aligns with coordinated changes across test suite.

Replacing -acceptnonstdtxn=1 with -datacarriersize=100000 is more targeted and appropriate:

• The test uses gen_return_txouts() which creates OP_RETURN outputs that may require larger data carrier size
• This change is more specific than broadly accepting all non-standard transactions
• Consistent with similar updates in other functional tests mentioned in the AI summary

The test functionality should remain intact while using more precise node configuration.

test/functional/mempool_limit.py (1)

26-26: Consistent test parameter modernization.

The same beneficial change as in mining_prioritisetransaction.py:

• Replaces broad non-standard transaction acceptance with specific data carrier size limit
• Supports the test's use of gen_return_txouts() for creating large data outputs
• Part of coordinated modernization across the functional test suite

This maintains test functionality while using more precise node configuration.

test/functional/wallet_importdescriptors.py (1)

433-433: LGTM! Improved synchronization approach.

Replacing self.generate(self.nodes[0], 6) with self.sync_all() is a good improvement. The sync_all() method ensures both block and mempool synchronization across all nodes, which is more comprehensive and reliable for multi-node wallet tests.

src/util/hasher.h (3)

8-8: LGTM! Adding necessary cryptographic header.

The inclusion of <crypto/common.h> provides required cryptographic functionality and aligns with the header modernization effort.

---

13-14: LGTM! Modernizing to C++ standard headers.

Replacing C-style headers with <cstdint> and <cstring> follows C++ best practices and ensures consistent type definitions across the codebase.

---

16-16: LGTM! Proper forward declaration.

The forward declaration of the Span template class is appropriate to avoid circular dependencies while allowing the hasher classes to work with span types.

test/functional/feature_maxuploadtarget.py (2)

49-50: LGTM! Coordinated test parameter updates.

The changes properly update the upload target to 800MB and add the data carrier size parameter. These modifications align with similar updates in other functional tests and maintain test consistency.

---

103-103: LGTM! Test logic correctly updated.

The max_bytes_per_day calculation has been properly updated to reflect the new 800MB upload target, ensuring the test logic remains consistent with the node configuration.

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

114-115: LGTM! Well-designed template method declaration.

The templated push_backV method provides flexible iterator-based insertion of ranges into UniValue arrays, enhancing the API's usability and supporting move semantics.

---

185-191: LGTM! Correct and efficient implementation.

The template implementation is well-designed:

• Properly validates the UniValue type before proceeding
• Uses efficient values.insert(values.end(), first, last) for range insertion
• Returns appropriate boolean status
• Supports move semantics when used with move iterators

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

610-612: LGTM! Large OP_RETURN output implementation looks correct.

The change from multiple small outputs to a single large OP_RETURN output is well-implemented. The math checks out: 67,437 bytes of data plus overhead equals 67,456 bytes total serialized size, achieving the ~66k vbytes target mentioned in the comment.

🧰 Tools

🪛 Pylint (3.3.7)

[refactor] 611-611: Consider using a generator instead 'sum(len(txout.serialize()) for txout in txouts)'

(R1728)

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

331-331: Good documentation update.

The parameter documentation correctly reflects the change from a specific UniValue reference to a generic vector container.

---

335-336: Excellent template generalization.

Converting ListTransactions to a template function allows it to work with different container types while maintaining type safety. This makes the function more flexible and reusable.

---

523-523: Good performance optimization with vector collection.

Using std::vector<UniValue> to collect transactions before moving them to the final result is more efficient than repeatedly calling push_back on a UniValue array, especially for large transaction lists.

---

545-547: Efficient bulk transfer preserving chronological order.

The implementation correctly reverses the collected transactions (newest to oldest → oldest to newest) using move iterators and the new push_backV method for efficient bulk transfer. The range calculation (txs_rev_it - nFrom - nCount, txs_rev_it - nFrom) properly handles the skip and count parameters.

src/qt/bitcoin.cpp (4)

101-105: LGTM: Proper Qt 5/6 compatibility for meta type registration.

The conditional compilation correctly handles the API change where qRegisterMetaTypeStreamOperators was removed in Qt 6 in favor of the simpler qRegisterMetaType.

---

144-148: LGTM: Correct handling of deprecated QLibraryInfo API.

The change properly addresses the deprecation of QLibraryInfo::location() in Qt 6, replacing it with QLibraryInfo::path().

---

150-152: LGTM: Improved code style with consistent bracing.

Adding braces around single-line if statements improves code readability and consistency.

Also applies to: 155-157, 160-162, 165-167

---

529-533: LGTM: Appropriate high-DPI handling for Qt versions.

The high-DPI attributes are correctly limited to Qt versions below 6.0.0, as these settings became default behavior in Qt 6.

src/qt/optionsdialog.cpp (1)

399-411: LGTM: Enhanced user communication in reset dialog.

The restructured dialog message provides better user guidance by clearly explaining:

1. The restart requirement
2. Where settings will be backed up
3. What action is being requested

This is a significant improvement over the previous simple concatenation approach.

.cirrus.yml (2)

30-36: LGTM: Good centralization of common CI settings.

The new main_template effectively consolidates timeout, ccache, and CI script settings that are shared across multiple tasks.

---

54-60: LGTM: Platform-specific template improves organization.

The macos_native_task_template appropriately encapsulates macOS-specific setup steps like clang version checking and Homebrew package installation.

🧰 Tools

🪛 YAMLlint (1.37.1)

[warning] 55-55: too many spaces before colon

(colons)

---

[warning] 60-60: too many spaces before colon

(colons)

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

209-210:

✅ Verification successful

Verify the Branch and Bound algorithm change is intentional.

The test expectation for selecting 5 cents has changed from {4 cents, 1 cent} to {3 cents, 2 cents}. Both combinations equal 5 cents, but this change suggests the algorithm now prefers different coin combinations when multiple zero-waste solutions exist.

Please verify that this change aligns with the intended algorithmic behavior:

---

🏁 Script executed:

#!/bin/bash
# Description: Search for the Branch and Bound implementation to understand the algorithmic change
# Expected: Find the implementation and any changes to early exit behavior

echo "Searching for Branch and Bound coin selection implementation..."
rg -A 10 -B 5 "SelectCoinsBnB|zero.?waste|early.?exit" --type cpp

echo -e "\nSearching for BnB algorithm changes..."
ast-grep --pattern 'bool SelectCoinsBnB($$$) {
  $$$
}'

echo -e "\nSearching for related test documentation or comments..."
rg -A 5 -B 5 "early.*exit|zero.*waste.*solution" --type cpp

Length of output: 23041

---

🏁 Script executed:

#!/bin/bash
# Display the BnB recursion and early-exit logic in SelectCoinsBnB
echo "=== BnbSearch definition ==="
rg -n -C5 "BnbSearch" src/wallet/coinselection.cpp

echo -e "\n=== Perfect-match bailout (zero-waste) ==="
rg -n -C5 "if (curr_value ==.*selection_target)" src/wallet/coinselection.cpp

echo -e "\n=== Any 'found_solution' or early-exit flag ==="
rg -n -C5 "found" src/wallet/coinselection.cpp

Length of output: 2073

---

Exhaustive BnB search: test expectations updated accordingly.

The Branch-and-Bound implementation no longer bails out on the first zero-waste match—instead it explores all perfect-match combinations and picks the one with minimal waste according to its tie-breaker. That change explains why:

• For a 5 ¢ target, {3 ¢, 2 ¢} is now chosen over {4 ¢, 1 ¢}
• For a 10 ¢ target, {4 ¢, 3 ¢, 2 ¢, 1 ¢} is now chosen instead of combinations with fewer coins

The updated tests accurately reflect the intended, exhaustive minimization of the waste metric.

---

236-237:

✅ Verification successful

Confirm the coin selection optimality for the 10-cent test case.

The expected result now selects {4, 3, 2, 1} cents (4 coins) instead of using the available 5-cent coin, which could produce fewer coins like {5, 4, 1} or {5, 3, 2} (3 coins each). Using more coins generally increases transaction size and fees.

This selection pattern suggests the algorithm may be optimizing for criteria other than minimizing coin count. Please verify this is the intended behavior and aligns with the coin selection strategy:

---

🏁 Script executed:

#!/bin/bash
# Description: Examine the coin selection criteria and optimization goals
# Expected: Find documentation or code comments explaining the selection strategy

echo "Searching for coin selection optimization criteria..."
rg -A 10 -B 5 "optimization|minimize|criteria|strategy|waste.*calculation" src/wallet/coinselection.cpp

echo -e "\nSearching for comments about coin selection preferences..."
rg -A 5 -B 5 "prefer|optimal|better.*solution" src/wallet/coinselection.cpp

Length of output: 3643

---

🏁 Script executed:

#!/bin/bash
echo "Context around the 10-cent test case:"
rg -C 10 "10 \* CENT" src/wallet/test/coinselector_tests.cpp

echo -e "\nDefinition of add_coin helper:"
rg -C 5 "static void add_coin" src/wallet/test/coinselector_tests.cpp

Length of output: 8574

---

BnB selects by minimized waste, not coin count
The Branch & Bound implementation in src/wallet/coinselection.cpp (and documented in Murch’s thesis) optimizes the waste metric—
waste = (selectionTotal – target) + inputs × (currentFeeRate – longTermFeeRate)—
not the raw number of coins. It explores all combinations and, on ties, keeps the last zero-waste solution found. In the 10 cent test, {4, 3, 2, 1} yields the same zero waste as {5, 4, 1} or {5, 3, 2} but is encountered last, so its selection is intentional and correct. No change required.

Reviewing files that changed from the base of the PR and between 98e358d and ff68e99.

• contrib/devtools/symbol-check.py (1 hunks)
• contrib/guix/libexec/prelude.bash (1 hunks)
• contrib/guix/manifest.scm (1 hunks)
• contrib/guix/patches/gcc-remap-guix-store.patch (1 hunks)
• contrib/guix/patches/winpthreads-remap-guix-store.patch (1 hunks)
• src/qt/bitcoin.cpp (3 hunks)
• src/univalue/include/univalue.h (2 hunks)
• src/util/asmap.cpp (1 hunks)
• src/util/bip32.cpp (1 hunks)
• src/util/bip32.h (0 hunks)
• src/util/bytevectorhash.cpp (1 hunks)
• src/util/bytevectorhash.h (1 hunks)
• src/util/error.cpp (1 hunks)
• src/util/hasher.cpp (1 hunks)
• src/util/hasher.h (1 hunks)
• src/util/message.cpp (1 hunks)
• src/util/message.h (1 hunks)
• src/util/moneystr.cpp (1 hunks)
• src/util/readwritefile.cpp (1 hunks)
• src/util/serfloat.h (1 hunks)
• src/util/spanparsing.cpp (1 hunks)
• src/util/strencodings.cpp (1 hunks)
• src/util/strencodings.h (1 hunks)
• src/util/syserror.cpp (1 hunks)
• src/util/thread.cpp (1 hunks)
• src/util/time.cpp (1 hunks)
• src/util/time.h (1 hunks)
• src/util/translation.h (1 hunks)
• src/util/vector.h (1 hunks)
• src/wallet/coinselection.cpp (0 hunks)
• src/wallet/rpc/transactions.cpp (3 hunks)
• src/wallet/test/coinselector_tests.cpp (2 hunks)
• test/functional/feature_maxuploadtarget.py (2 hunks)
• test/functional/mempool_limit.py (1 hunks)
• test/functional/mining_prioritisetransaction.py (1 hunks)
• test/functional/test_framework/util.py (1 hunks)
• test/functional/wallet_importdescriptors.py (1 hunks)

• src/util/bip32.h
• src/wallet/coinselection.cpp

• src/util/time.h
• src/util/spanparsing.cpp
• src/util/translation.h
• src/util/vector.h
• src/util/strencodings.h
• src/util/bytevectorhash.cpp
• src/util/serfloat.h
• src/util/thread.cpp
• src/util/error.cpp
• src/util/moneystr.cpp
• src/util/message.cpp
• src/util/readwritefile.cpp

• src/util/syserror.cpp
• test/functional/mempool_limit.py
• src/util/bytevectorhash.h
• src/util/message.h
• src/util/bip32.cpp
• src/util/strencodings.cpp
• src/util/hasher.cpp
• src/util/asmap.cpp
• src/util/hasher.h
• contrib/guix/libexec/prelude.bash
• test/functional/wallet_importdescriptors.py
• test/functional/mining_prioritisetransaction.py
• test/functional/feature_maxuploadtarget.py
• contrib/guix/patches/winpthreads-remap-guix-store.patch
• src/util/time.cpp
• contrib/guix/patches/gcc-remap-guix-store.patch
• src/univalue/include/univalue.h
• contrib/guix/manifest.scm
• src/qt/bitcoin.cpp
• src/wallet/rpc/transactions.cpp
• src/wallet/test/coinselector_tests.cpp

• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_nowallet-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: linux64_fuzz-build / Build source
• GitHub Check: arm-linux-build / Build source

contrib/devtools/symbol-check.py (1)

255-255:

❓ Verification inconclusive

Verify the updated LLVM linker version.

The version update from [18, 1, 6] to [18, 1, 8] appears to align with build environment updates mentioned in the PR context.

Please verify that this version matches the actual LLVM linker version used in your macOS build environment:

---

🏁 Script executed:

#!/bin/bash
# Description: Verify the LLVM linker version used in macOS builds
# Expected: Version should match [18, 1, 8]

# Check if ld64.lld is available and get its version
if command -v ld64.lld >/dev/null 2>&1; then
    echo "ld64.lld version:"
    ld64.lld --version
elif command -v lld >/dev/null 2>&1; then
    echo "lld version:"
    lld --version
else
    echo "LLVM linker not found in PATH"
fi

# Check Xcode's linker version if on macOS
if [[ "$OSTYPE" == "darwin"* ]]; then
    echo "Xcode linker version:"
    xcrun --show-sdk-path
    xcrun --sdk macosx --show-sdk-version
fi

Length of output: 142

---

Verify macOS LLVM linker version and simplify return statement

Please confirm that the updated LLVM linker version ([18, 1, 8]) matches the one shipped with your macOS build tools, and streamline the conditional in check_MACHO_lld.

• In contrib/devtools/symbol-check.py at line 255, simplify:

-    if binary.build_version.tools[0].version == [18, 1, 8]:
-        return True
-    return False
+    return binary.build_version.tools[0].version == [18, 1, 8]

• Run the version check on a macOS machine (where ld64.lld or lld is in PATH or via xcrun) to ensure it reports 18.1.8.

🧰 Tools

🪛 Ruff (0.11.9)

255-257: Return the condition directly

Inline condition

(SIM103)

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

605-606: Comment updates look good.

The updated comments accurately reflect the new implementation that generates a single large OP_RETURN output instead of multiple smaller ones.

---

610-611:

✅ Verification successful

Verify the data carrier size coordination with test configuration.

The refactoring from 128 small outputs to 1 large output (67,437 bytes) is well-implemented and maintains the expected total serialized size. However, this large OP_RETURN requires coordination with test configurations that set -datacarriersize=100000.

Please verify that all tests using this function have the appropriate data carrier size configuration:

---

🏁 Script executed:

#!/bin/bash
# Description: Check if tests using gen_return_txouts have proper datacarriersize configuration
# Expected: Tests should include -datacarriersize=100000 or similar large value

# Find all files that import or use gen_return_txouts
rg -l "gen_return_txouts"

# Check for datacarriersize configuration in those files
rg -A 5 -B 5 "datacarriersize" --type py

Length of output: 2535

---

All tests using gen_return_txouts include the correct -datacarriersize

We’ve verified that every test importing or calling gen_return_txouts sets -datacarriersize=100000, which safely accommodates the 67 437-byte OP_RETURN:

• test/functional/feature_maxuploadtarget.py
• test/functional/mining_prioritisetransaction.py
• test/functional/mempool_limit.py

No further changes are needed.

🧰 Tools

🪛 Pylint (3.3.7)

[refactor] 611-611: Consider using a generator instead 'sum(len(txout.serialize()) for txout in txouts)'

(R1728)

Reviewing files that changed from the base of the PR and between ff68e99 and 8b9d343.

• ci/dash/lint-tidy.sh (1 hunks)
• contrib/devtools/symbol-check.py (1 hunks)
• contrib/guix/libexec/prelude.bash (1 hunks)
• contrib/guix/manifest.scm (1 hunks)
• contrib/guix/patches/gcc-remap-guix-store.patch (1 hunks)
• contrib/guix/patches/winpthreads-remap-guix-store.patch (1 hunks)
• src/univalue/include/univalue.h (2 hunks)
• src/util/asmap.cpp (1 hunks)
• src/util/bip32.cpp (1 hunks)
• src/util/bip32.h (0 hunks)
• src/util/bytevectorhash.cpp (1 hunks)
• src/util/bytevectorhash.h (1 hunks)
• src/util/error.cpp (1 hunks)
• src/util/hasher.cpp (1 hunks)
• src/util/hasher.h (1 hunks)
• src/util/message.cpp (1 hunks)
• src/util/message.h (1 hunks)
• src/util/moneystr.cpp (1 hunks)
• src/util/readwritefile.cpp (1 hunks)
• src/util/serfloat.h (1 hunks)
• src/util/spanparsing.cpp (1 hunks)
• src/util/strencodings.cpp (1 hunks)
• src/util/strencodings.h (1 hunks)
• src/util/syserror.cpp (1 hunks)
• src/util/thread.cpp (1 hunks)
• src/util/time.cpp (1 hunks)
• src/util/time.h (1 hunks)
• src/util/translation.h (1 hunks)
• src/util/vector.h (1 hunks)
• src/wallet/rpc/transactions.cpp (3 hunks)
• src/wallet/test/coinselector_tests.cpp (2 hunks)
• test/functional/feature_maxuploadtarget.py (1 hunks)
• test/functional/mempool_limit.py (1 hunks)
• test/functional/mining_prioritisetransaction.py (1 hunks)
• test/functional/test_framework/util.py (1 hunks)

• src/util/bip32.h

• src/util/translation.h
• src/util/spanparsing.cpp
• src/util/error.cpp
• src/util/strencodings.h
• src/util/message.cpp
• ci/dash/lint-tidy.sh
• src/util/time.cpp

• src/util/time.h
• src/util/hasher.cpp
• src/util/moneystr.cpp
• src/util/syserror.cpp
• src/util/serfloat.h
• src/util/readwritefile.cpp
• src/util/thread.cpp
• src/util/bytevectorhash.h
• src/util/bytevectorhash.cpp
• contrib/guix/libexec/prelude.bash
• src/util/asmap.cpp
• src/util/message.h
• src/util/bip32.cpp
• src/util/vector.h
• test/functional/mining_prioritisetransaction.py
• test/functional/mempool_limit.py
• src/util/strencodings.cpp
• contrib/guix/patches/winpthreads-remap-guix-store.patch
• src/util/hasher.h
• test/functional/feature_maxuploadtarget.py
• contrib/guix/manifest.scm
• contrib/guix/patches/gcc-remap-guix-store.patch
• src/wallet/test/coinselector_tests.cpp
• src/univalue/include/univalue.h
• src/wallet/rpc/transactions.cpp

• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: arm-linux-build / Build source

contrib/devtools/symbol-check.py (1)

255-257:

✅ Verification successful

Apply the static analysis suggestion for cleaner code.

The version update from [18, 1, 6] to [18, 1, 8] looks appropriate for a toolchain update. However, the function can be simplified by returning the condition directly.

Apply this diff to simplify the function:

 def check_MACHO_lld(binary) -> bool:
-    if binary.build_version.tools[0].version == [18, 1, 8]:
-        return True
-    return False
+    return binary.build_version.tools[0].version == [18, 1, 8]

Please verify that LLD version 18.1.8 is the correct version for your toolchain:

---

🌐 Web query:

What is the latest stable version of LLVM LLD linker 18.x series?

💡 Result:

Latest Stable Version of LLVM LLD Linker in the 18.x Series

The most recent stable version of the LLVM LLD linker in the 18.x series is 18.1.8. This version, along with its sources, pre-built binaries, and documentation, is available for download on the official LLVM release page as of June 2025[10].

Citations:

• 1: https://llvm.org
• 2: https://releases.llvm.org
• 3: https://maskray.me/blog/2024-02-18-lld-18-elf-changes
• 4: https://github.com/llvm/llvm-project
• 5: https://rocm.docs.amd.com/projects/llvm-project/en/docs-6.3.1/LLVM/lld/html/
• 6: https://releases.llvm.org/18.1.0/tools/lld/docs/ReleaseNotes.html
• 7: llvm/llvm-project#67779
• 8: https://rocm.docs.amd.com/projects/llvm-project/en/latest/LLVM/lld/html/ReleaseNotes.html
• 9: https://apt.llvm.org
• 10: https://releases.llvm.org/download.html

---

Simplify check_MACHO_lld to return the version check directly

Verified that LLVM LLD 18.1.8 is the latest stable 18.x release, so the function can be reduced to a single return statement.

• File: contrib/devtools/symbol-check.py
Function: check_MACHO_lld (around lines 255–257)

Apply this diff:

 def check_MACHO_lld(binary) -> bool:
-    if binary.build_version.tools[0].version == [18, 1, 8]:
-        return True
-    return False
+    return binary.build_version.tools[0].version == [18, 1, 8]

🧰 Tools

🪛 Ruff (0.11.9)

255-257: Return the condition directly

Inline condition

(SIM103)

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

610-610:

✅ Verification successful

Verify OP_RETURN size limits.

The function creates an OP_RETURN script with 67,437 bytes of data, which is significantly larger than typical Bitcoin/Dash OP_RETURN limits. Ensure this large size is supported by the test environment and doesn't violate protocol constraints.

---

🏁 Script executed:

#!/bin/bash
# Description: Check for OP_RETURN size limits in the codebase

# Search for OP_RETURN size constraints or limits
rg -A 5 -B 5 "OP_RETURN.*[Ll]imit|[Ll]imit.*OP_RETURN|MAX.*OP_RETURN|OP_RETURN.*MAX" --type cpp --type py

# Search for datacarriersize parameter usage
rg -A 3 -B 3 "datacarriersize" --type cpp --type py

Length of output: 9766

---

OP_RETURN size is covered by functional test overrides

The 67 437-byte OP_RETURN payload is accepted because functional tests override the default 83 byte limit via the -datacarriersize=100000 node argument. No changes are needed.

Key locations setting a higher limit:

• test/functional/mempool_limit.py
• test/functional/feature_maxuploadtarget.py
• test/functional/mining_prioritisetransaction.py