Reviewing files that changed from the base of the PR and between 0e9105e and 8b14cd6.

• configure.ac (1 hunks)
• src/Makefile.test.include (1 hunks)
• src/bench/ellswift.cpp (1 hunks)
• src/bitcoin-tx.cpp (1 hunks)
• src/bitcoind.cpp (1 hunks)
• src/compat/byteswap.h (2 hunks)
• src/compat/endian.h (1 hunks)
• src/crypto/chacha20.h (1 hunks)
• src/crypto/common.h (1 hunks)
• src/crypto/hkdf_sha256_32.h (1 hunks)
• src/crypto/hmac_sha256.h (1 hunks)
• src/crypto/hmac_sha512.h (1 hunks)
• src/crypto/pkcs5_pbkdf2_hmac_sha512.h (1 hunks)
• src/crypto/poly1305.h (1 hunks)
• src/crypto/ripemd160.h (1 hunks)
• src/crypto/sha1.h (1 hunks)
• src/crypto/sha256.h (1 hunks)
• src/crypto/sha256_sse4.cpp (1 hunks)
• src/crypto/sha3.h (1 hunks)
• src/crypto/sha512.h (1 hunks)
• src/dbwrapper.h (8 hunks)
• src/httpserver.cpp (1 hunks)
• src/i2p.cpp (1 hunks)
• src/ipc/interfaces.cpp (1 hunks)
• src/ipc/process.cpp (1 hunks)
• src/llmq/blockprocessor.cpp (3 hunks)
• src/llmq/instantsend.cpp (3 hunks)
• src/llmq/signing.cpp (5 hunks)
• src/memusage.h (1 hunks)
• src/prevector.h (6 hunks)
• src/qt/utilitydialog.cpp (1 hunks)
• src/random.cpp (1 hunks)
• src/random.h (2 hunks)
• src/serialize.h (4 hunks)
• src/span.h (3 hunks)
• src/streams.h (1 hunks)
• src/support/cleanse.h (1 hunks)
• src/test/argsman_tests.cpp (1 hunks)
• src/test/blockchain_tests.cpp (1 hunks)
• src/test/bswap_tests.cpp (1 hunks)
• src/test/crypto_tests.cpp (0 hunks)
• src/test/fuzz/integer.cpp (0 hunks)
• src/test/raii_event_tests.cpp (1 hunks)
• src/test/util/wallet.cpp (1 hunks)
• src/test/util_tests.cpp (0 hunks)
• src/util/asmap.cpp (3 hunks)
• src/util/bip32.cpp (1 hunks)
• src/util/readwritefile.cpp (1 hunks)
• src/util/url.cpp (1 hunks)
• src/wallet/bdb.cpp (3 hunks)
• src/wallet/bdb.h (1 hunks)
• src/wallet/crypter.cpp (1 hunks)
• src/wallet/sqlite.cpp (3 hunks)

• src/test/fuzz/integer.cpp
• src/test/crypto_tests.cpp
• src/test/util_tests.cpp

• src/qt/utilitydialog.cpp
• src/memusage.h
• src/util/readwritefile.cpp
• src/random.cpp
• src/util/url.cpp
• src/bitcoind.cpp
• src/test/util/wallet.cpp
• src/util/bip32.cpp
• src/crypto/pkcs5_pbkdf2_hmac_sha512.h
• src/test/raii_event_tests.cpp
• src/crypto/sha512.h
• src/support/cleanse.h
• src/crypto/chacha20.h
• src/crypto/ripemd160.h
• src/streams.h
• src/ipc/interfaces.cpp
• src/httpserver.cpp
• src/test/blockchain_tests.cpp
• src/crypto/sha256.h
• src/bitcoin-tx.cpp
• src/crypto/sha3.h
• src/crypto/sha256_sse4.cpp
• src/crypto/sha1.h
• src/ipc/process.cpp
• src/crypto/hmac_sha512.h
• src/crypto/hkdf_sha256_32.h
• src/crypto/hmac_sha256.h

src/test/argsman_tests.cpp (4)

49-80: Validate locking usage in ReadConfigString.
The explicit lock around m_settings.ro_config and m_config_sections adjustments is good. Ensure that all usage of m_settings variables is also consistently guarded by cs_args in other methods to prevent potential data races.

---

359-411: Commend concurrency approach with cs_args.
These tests demonstrate consistent usage of LOCK(testArgs.cs_args) while modifying shared data, which helps mitigate concurrency issues.

---

578-599: Highlight section-only configuration handling.
Connectivity among SetNetworkOnlyArg calls and subsequent network selections is well tested. The approach ensures that section-specific arguments do not leak into global scope.

---

717-915: Confirm stability of hash-based test validation.
A hash is used to detect unintended behavior changes. If this test starts failing, carefully inspect all new code or updates to ArgsManager logic to confirm whether the difference is a legitimate break or expected behavior.

src/Makefile.test.include (1)

76-76: Great addition to the test suite.
Adding test/argsman_tests.cpp expands coverage for ArgsManager behavior. No issues found here.

src/compat/byteswap.h (8)

8-9: No issues with the standard library includes.
This addition of <cstdint> and conditional <cstdlib> under _MSC_VER is appropriate and portable.

---

14-16: Clarifying C++23 upgrade path.
Commenting about future replacement with std::byteswap is clear and helpful.

---

18-34: Robust macro handling for built-in byte-swap functions.
Conditionally defining bitcoin_builtin_bswap* ensures optimized paths where supported. Implementation meets portability needs.

---

38-39: Consistent BSWAP_CONSTEXPR definition.
Defining BSWAP_CONSTEXPR as constexpr when not on MSVC is a sound approach to leverage compile-time evaluation.

---

41-41: Fallback for MSVC.
Overriding BSWAP_CONSTEXPR for MSVC ensures compatibility with compiler constraints.

---

44-51: internal_bswap_16 implementation.
Correctly swaps 16-bit values using either built-ins or a simple shift-and-mask fallback.

---

53-61: internal_bswap_32 implementation.
Logic is standard and efficient, handling 32-bit swaps with built-ins or a clearly structured fallback.

---

63-77: internal_bswap_64 implementation.
64-bit swap approach is consistent with the rest, with built-in usage or a reliable fallback.

src/compat/endian.h (13)

10-10: Including <bit> header for endian checks.
This is essential for using std::endian.

---

13-17: htobe16_internal: Proper conditional swap.
Uses compile-time endianness checks to swap only when needed.

---

18-22: htole16_internal: Modern approach.
Mirrors the logic for htobe16_internal and ensures appropriate swaps.

---

23-27: be16toh_internal: Correct condition.
Implementation parallels htobe16_internal logic.

---

28-32: le16toh_internal: Straightforward fallback.
Consistent usage of internal_bswap_16 ensures clarity.

---

33-37: htobe32_internal: Reliable endianness handling.
Follows the same pattern for 32-bit.

---

38-42: htole32_internal: Maintains uniform approach.
Conditional swap logic is consistent with the rest.

---

43-47: be32toh_internal: Cohesive design.
Properly reuses internal_bswap_32 for big- to host-endian conversion.

---

48-52: le32toh_internal: Clear implementation.
Implementation is in line with the established pattern.

---

53-57: htobe64_internal: Full 64-bit support.
Leverages internal_bswap_64 for little-endian hosts.

---

58-62: htole64_internal: Matches the other conversions.
Remains straightforward and robust with compile-time checks.

---

63-67: be64toh_internal: Minimal and consistent.
Applies the same technique for big to host conversion.

---

68-72: le64toh_internal: Final 64-bit function.
Ensures complete coverage of little to host scenario.

src/test/bswap_tests.cpp (1)

19-21: Switch to internal_bswap_* in tests.
Tests verify correct swapping with the new function names. Implementation is sound.

src/bench/ellswift.cpp (1)

20-25:

Details

❓ Verification inconclusive

LGTM! Improved byte span handling.

The changes to use MakeByteSpan and MakeWritableByteSpan improve type safety and readability.

Let's verify that these span utility functions are consistently used across the codebase:

---

🏁 Script executed:

#!/bin/bash
# Search for any remaining instances of old span handling patterns
rg "AsBytes\(Span" 
rg "BytePtr\("

Length of output: 1304

---

LGTM – Improved byte span handling in src/bench/ellswift.cpp.

The update to use MakeByteSpan and MakeWritableByteSpan indeed improves type safety and enhances readability in the benchmark code. Note, however, that legacy patterns using AsBytes(Span{…}) still appear in several other parts of the codebase (for example in src/bls/bls.h, src/pubkey.h, src/serialize.h, and src/test/key_tests.cpp). Please verify if these should be updated in tandem or addressed in a separate migration step.

src/crypto/common.h (2)

10-12: LGTM! Modernized header includes.

Replaced C-style headers with their C++ counterparts, improving consistency with modern C++ practices.

---

15-83:

Details

✅ Verification successful

Verify endian conversion function changes.

The switch to internal versions of endian conversion functions (*_internal) is consistent throughout the file. This change improves encapsulation and maintainability.

Let's verify that all endian conversion functions use the internal versions consistently:

---

🏁 Script executed:

#!/bin/bash
# Search for any remaining non-internal endian conversion function calls
rg "htobe|htole|be.*toh|le.*toh" --type cpp | grep -v "_internal"

Length of output: 1390

---

Endianness Conversion in src/crypto/common.h Verified

The verification confirms that all functions in src/crypto/common.h consistently use the internal versions (e.g., le16toh_internal, be32toh_internal, etc.). No non-internal calls were detected in this file; the non-internal matches found are in unrelated parts of the codebase and don’t affect this change.

• ReadLE16, ReadLE32, ReadLE64, WriteLE16, WriteLE32, WriteLE64, ReadBE16, ReadBE32, ReadBE64, WriteBE32, and WriteBE64 all correctly use the respective _internal functions.

src/crypto/poly1305.h (1)

11-11: LGTM! Modernized header include.

Replaced C-style stdlib.h with C++ cstdlib, consistent with modern C++ practices.

src/wallet/crypter.cpp (1)

114-114: LGTM! Removed redundant type cast.

Simplified the Encrypt call by removing the unnecessary cast, as vchPlaintext is already of type CKeyingMaterial.

src/wallet/bdb.h (1)

171-190: LGTM! Good use of RAII pattern for Dbt management.

The SafeDbt class is a well-designed RAII wrapper that ensures proper memory management of Dbt objects. The class provides:

• Automatic cleanup in the destructor
• Clear interface for data access
• Proper encapsulation of the underlying Dbt object

src/util/asmap.cpp (2)

12-12: LGTM! Good use of C++20 header.

Adding the <bit> header provides access to modern bit manipulation utilities.

---

111-111: Good modernization using std::bit_width.

Replacing CountBits with std::bit_width is a good improvement:

• Uses standard library functionality
• More efficient implementation
• Better maintainability

src/random.h (2)

14-14: LGTM! Good addition of C++20 header.

Adding the <bit> header provides access to modern bit manipulation utilities.

---

200-200: Good modernization using std::bit_width.

The update to use std::bit_width in the randrange function is a good improvement:

• Uses modern C++20 features
• More maintainable code
• Consistent with other similar changes in the codebase

src/span.h (3)

9-11: LGTM! Better organization of include directives.

The includes are now better organized and use the modern C++ header versions.

---

250-250: Good improvement in type safety with explicit casts.

The direct use of reinterpret_cast in AsBytes and AsWritableBytes makes the casting intentions more explicit and improves type safety.

Also applies to: 255-255

---

270-273: Good improvement in UCharCast implementations.

The updated UCharCast functions with explicit reinterpret_cast:

• Make casting intentions clearer
• Improve type safety
• Are more consistent with modern C++ practices

src/i2p.cpp (1)

357-357:

Details

✅ Verification successful

Verify byte order handling in I2P destination data.

The change from be16toh to be16toh_internal maintains the same functionality but uses an internal implementation. Since this affects network protocol data handling, ensure the byte order conversion remains consistent.

Run the following script to verify the byte order handling:

---

🏁 Script executed:

#!/bin/bash
# Description: Verify byte order handling in I2P destination data.

# Test: Search for all usages of byte order conversion functions.
rg -A 5 $'be16toh|be16toh_internal'

# Test: Search for any related test files.
fd -t f "test.*i2p"

Length of output: 1238

---

Byte Order Conversion Consistency Verified

The usage of be16toh_internal in src/i2p.cpp is in line with the project's internal conversion implementation as defined in src/compat/endian.h. The grep results confirm that this internal function is consistently used across related modules (e.g., serialization and crypto), and its behavior matches that of the original be16toh.

• Location Verified:

  • src/i2p.cpp uses be16toh_internal(cert_len) for converting certificate length.
  • The same internal function appears in src/serialize.h and src/crypto/common.h, ensuring consistent conversion throughout the codebase.

• Next Steps:

  • Confirm that any network protocol data handling relying on these conversions is covered by existing tests.
  • Maintain vigilance in future changes to ensure the internal function continues to meet the necessary byte order handling across all modules.

src/prevector.h (3)

50-50: LGTM! Improved safety with default member initialization.

Default initializing pointer members to nullptr prevents undefined behavior from uninitialized pointers.

Also applies to: 82-82, 102-102, 135-135

---

56-57: LGTM! Updated iterator traits for C++20 compatibility.

The changes:

• Added element_type to comply with C++20 iterator requirements
• Updated iterator_category to use std::contiguous_iterator_tag for more precise iterator categorization

---

68-72: LGTM! Enhanced const correctness and operator overloading.

The changes:

• Added const qualifiers to arithmetic operators
• Added friend declarations for proper operator overloading

src/wallet/sqlite.cpp (1)

28-32: LGTM! Improved code reusability and type safety.

The new SpanFromBlob function:

• Centralizes blob handling logic
• Uses std::byte and Span for safer binary data handling
• Maintains the same functionality while reducing code duplication

src/dbwrapper.h (1)

24-24: LGTM! Improved type safety with encapsulated pointer casting.

The new CharCast function:

• Encapsulates pointer casting logic
• Improves code readability
• Maintains consistency with std::byte usage across the codebase

src/wallet/bdb.cpp (3)

27-31: LGTM! The new SpanFromDbt function enhances safety.

The function provides a safer way to handle database data by using Span<const std::byte> instead of raw pointers, which helps prevent buffer overflows and improves type safety.

---

696-697: LGTM! Improved stream writing with SpanFromDbt.

The code now uses the safer SpanFromDbt function to write data to the stream, replacing the previous raw pointer usage.

Also applies to: 699-699

---

771-772: LGTM! Improved stream writing with SpanFromDbt and explicit clearing.

The code now:

1. Explicitly clears the value stream before writing
2. Uses the safer SpanFromDbt function to write data

src/llmq/signing.cpp (5)

172-173: LGTM! Consistent use of internal endian conversion function.

Changed to use htobe32_internal for time-related key generation, aligning with the codebase's move towards internal endian conversion functions.

---

211-212: LGTM! Consistent use of internal endian conversion function.

Changed to use htobe32_internal for time-related key generation in RemoveRecoveredSig, maintaining consistency with other changes.

---

250-252: LGTM! Consistent use of internal endian conversion function.

Changed to use be32toh_internal for time comparison in CleanupOldRecoveredSigs, maintaining consistency with other changes.

---

299-300: LGTM! Consistent use of internal endian conversion function.

Changed to use htobe32_internal for time-related key generation in WriteVoteForId, maintaining consistency with other changes.

---

324-326: LGTM! Consistent use of internal endian conversion function.

Changed to use be32toh_internal for time comparison in CleanupOldVotes, maintaining consistency with other changes.

src/llmq/blockprocessor.cpp (4)

214-215: LGTM! Consistent use of internal endian conversion function.

Changed to use htobe32_internal for height-related key generation in BuildInversedHeightKey, maintaining consistency with other changes.

---

220-221: LGTM! Consistent use of internal endian conversion function.

Changed to use htobe32_internal for height-related key generation in BuildInversedHeightKeyIndexed, maintaining consistency with other changes.

---

508-511: LGTM! Consistent use of internal endian conversion function.

Changed to use be32toh_internal for height comparison in GetMinedCommitmentsUntilBlock, maintaining consistency with other changes.

---

551-554: LGTM! Consistent use of internal endian conversion function.

Changed to use be32toh_internal for height comparison in GetLastMinedCommitmentsByQuorumIndexUntilBlock, maintaining consistency with other changes.

src/serialize.h (2)

63-64: LGTM! Consistent use of internal endian conversion functions.

Changed all endian conversion functions to use their internal counterparts in serialization and deserialization functions, maintaining consistency across the codebase:

• htole16 → htole16_internal
• htobe16 → htobe16_internal
• htole32 → htole32_internal
• htobe32 → htobe32_internal
• htole64 → htole64_internal
• le16toh → le16toh_internal
• be16toh → be16toh_internal
• le32toh → le32toh_internal
• be32toh → be32toh_internal
• le64toh → le64toh_internal

Also applies to: 68-69, 73-74, 78-79, 83-84, 96-97, 102-103, 108-109, 114-115, 120-121

---

658-663: LGTM! Consistent use of internal endian conversion functions.

Changed to use internal endian conversion functions in CustomUintFormatter for both serialization and deserialization:

• htobe64 → htobe64_internal
• htole64 → htole64_internal
• be64toh → be64toh_internal
• le64toh → le64toh_internal

Also applies to: 672-677

src/llmq/instantsend.cpp (3)

125-125: LGTM! Consistent use of internal byte order conversion function.

The change from htobe32 to htobe32_internal aligns with the codebase's move to use internal byte order conversion functions.

---

178-178: LGTM! Consistent use of internal byte order conversion function.

The change from be32toh to be32toh_internal maintains consistency with the internal byte order conversion functions.

---

222-222: LGTM! Consistent use of internal byte order conversion function.

The change from be32toh to be32toh_internal aligns with the codebase's standardization on internal byte order conversion functions.

configure.ac (1)

1038-1038: LGTM! Simplified header checks.

The changes appropriately:

1. Remove platform-specific endianness header checks, aligning with the move to internal byte order conversion functions
2. Add checks for essential system headers that are needed for various platform-specific functionality

This simplifies the configuration process while ensuring necessary system headers are available.