backport: bitcoin#25784, 25878, 25865, 25879

[vijaydasmp]

Bitcoin Backporting

[coderabbitai]

Walkthrough

The pull request introduces multiple changes across the codebase. In the core logic, the concatenation of block filter type names in blockfilter.cpp is simplified by replacing the string stream approach with a utility Join function, which now resides in the modified util/string.h file. The Join function itself has been generalized to work with any container using a range-based loop. In the wallet modules, the method for initializing wallet flags has been refactored: the previous AddWalletFlags method has been replaced with InitWalletFlags, which now includes additional assertions and validations. In addition, several functional tests related to wallet operations have been updated to include a whitelisting argument ("[email protected]") in their setup parameters, and wallet encryption tests have moved from range assertions to strict equality checks for unlock times. These changes update both functionality and testing configurations as documented in the pull request.

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1930572 and 8f73b41.

📒 Files selected for processing (14)

• src/blockfilter.cpp (2 hunks)
• src/test/util_tests.cpp (1 hunks)
• src/util/string.h (1 hunks)
• src/wallet/wallet.cpp (2 hunks)
• src/wallet/wallet.h (1 hunks)
• test/functional/wallet_abandonconflict.py (1 hunks)
• test/functional/wallet_balance.py (1 hunks)
• test/functional/wallet_basic.py (1 hunks)
• test/functional/wallet_encryption.py (2 hunks)
• test/functional/wallet_groups.py (1 hunks)
• test/functional/wallet_hd.py (1 hunks)
• test/functional/wallet_importdescriptors.py (1 hunks)
• test/functional/wallet_listreceivedby.py (1 hunks)
• test/functional/wallet_listsinceblock.py (1 hunks)

🔇 Additional comments (20)

src/util/string.h (3)

60-67: Documentation accurately reflects the enhanced functionality.

The updated documentation clearly describes the generic container support and provides accurate parameter descriptions.

---

68-79: Well-designed template implementation with improved efficiency.

The changes improve the Join function in several ways:

1. Generic container support through template parameter C
2. More efficient implementation using range-based for loop
3. Cleaner control flow with boolean flag instead of index checking

---

81-85: Consistent implementation of the simplified overload.

The overload correctly maintains consistency with the primary implementation.

test/functional/wallet_encryption.py (2)

78-84: Improved test determinism with mocktime.

The changes enhance test reliability by:

1. Using mocktime instead of real time
2. Using exact equality assertions instead of range checks

---

87-91: Consistent time checking approach.

The same pattern of using mocktime and exact equality is maintained for the second test case.

src/blockfilter.cpp (2)

15-15: Appropriate header inclusion for string utilities.

The addition of string.h header supports the use of the Join function.

---

182-182: Elegant simplification of ListBlockFilterTypes.

The implementation is improved by:

1. Using the generic Join function instead of manual string concatenation
2. More maintainable and readable code
3. Efficient lambda usage to extract filter type names

test/functional/wallet_listreceivedby.py (1)

20-21: Enhanced test reliability with whitelist configuration.

The addition of the whitelist configuration improves test reliability by speeding up transaction relay and mempool synchronization between nodes.

test/functional/wallet_groups.py (1)

28-30: LGTM! Well-documented peer whitelisting.

The addition of peer whitelisting with clear documentation improves test performance by speeding up transaction relay and mempool synchronization.

test/functional/wallet_abandonconflict.py (1)

27-29: LGTM! Consistent peer whitelisting implementation.

The addition of peer whitelisting matches the pattern seen in other test files and will help improve test performance.

test/functional/wallet_balance.py (1)

56-58: LGTM! Consistent peer whitelisting implementation.

The addition of peer whitelisting follows the same pattern as other test files and will enhance test performance.

test/functional/wallet_hd.py (1)

22-24: LGTM! Consistent peer whitelisting implementation.

The addition of peer whitelisting maintains consistency with other test files and will improve test performance.

test/functional/wallet_listsinceblock.py (1)

25-26: LGTM! Test configuration updated to improve transaction relay.

The addition of the whitelist argument will help speed up transaction relay and mempool synchronization during test execution.

test/functional/wallet_importdescriptors.py (1)

37-39: LGTM! Test configuration updated to improve transaction relay.

The addition of the whitelist argument using a loop is a clean implementation that will help speed up transaction relay and mempool synchronization during test execution.

test/functional/wallet_basic.py (1)

30-30: LGTM! Test configuration updated to improve transaction relay.

The addition of the whitelist argument in both descriptor and non-descriptor paths will help speed up transaction relay and mempool synchronization during test execution.

Also applies to: 34-34

src/wallet/wallet.h (1)

1429-1432: Verify the initialization requirements for wallet flags.

The new InitWalletFlags method has strict requirements:

1. The flags parameter must be uninitialized (or 0)
2. Only known flags may be present

Please ensure that all callers of this method comply with these requirements to prevent potential issues.

Run the following script to verify the usage of InitWalletFlags:

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Description: Check all callers of InitWalletFlags to ensure they pass uninitialized flags.

# Search for InitWalletFlags usage
rg -A 5 $'InitWalletFlags'

# Search for known flags definition to cross-reference
rg -A 10 $'KNOWN_WALLET_FLAGS'

Length of output: 2885

---

Wallet flag initialization conformity verified.

The implementation in CWallet::InitWalletFlags in wallet.cpp enforces the preconditions:

• It asserts that the wallet flags are uninitialized (m_wallet_flags must be 0).
• It asserts that only known flags are provided by comparing (flags & KNOWN_WALLET_FLAGS) with flags.

Additionally, the usage sites (e.g., walletInstance->InitWalletFlags) appear to comply with these requirements.

src/test/util_tests.cpp (2)

248-260: LGTM! Test cases updated to use explicit container types.

The test cases for the Join function have been improved by using explicit container types (std::vector and std::list), which better demonstrates the generic container support. The test coverage is comprehensive, covering empty containers, single elements, and multiple elements with both basic joining and transformation operations.

---

2755-2799: LGTM! Comprehensive test coverage for binary file operations.

The new test cases for ReadBinaryFile and WriteBinaryFile provide thorough coverage of file operations, including:

• Full file reads
• Partial file reads
• Error handling for non-existent files
• File writing and content verification

The tests properly use RAII for file handling and clean up temporary files appropriately.

src/wallet/wallet.cpp (2)

1770-1784: LGTM! The new InitWalletFlags method improves wallet initialization safety.

The new method adds important assertions to ensure wallet flags are initialized correctly:

• Verifies that only known wallet flags are being set
• Ensures flags are only set during initial wallet creation
• Validates that current flags are blank before initialization

---

4781-4782: LGTM! Proper integration of the new wallet flags initialization.

The new InitWalletFlags method is correctly called during first run wallet initialization.

[coderabbitai]

🛠️ Refactor suggestion

Add validation against KNOWN_WALLET_FLAGS.

Consider adding validation in the implementation to ensure that:

1. The input flags are a subset of KNOWN_WALLET_FLAGS
2. The flags are uninitialized (0) before initialization

Example validation:

void CWallet::InitWalletFlags(uint64_t flags)
{
    if (m_wallet_flags != 0) {
        throw std::logic_error("InitWalletFlags called with initialized flags");
    }
    if ((flags & ~KNOWN_WALLET_FLAGS) != 0) {
        throw std::logic_error("InitWalletFlags called with unknown flags");
    }
    m_wallet_flags = flags;
}

[UdjinM6]

pls see 21290b7 and ed06d32