test, wallet: overhaul wallet_reindex test to wallet_birthtime #34879

pull rkrux wants to merge 2 commits into bitcoin:master from rkrux:wallet-birthtime changing 3 files +90 −93
  1. rkrux commented at 12:57 PM on March 20, 2026: contributor

    The need for this change was ideated during the review of PR 34857.

    • The node doesn't need to be restarted (with reindex or without) because rescanblockchain RPC already updates the wallet birthtime, which is the property that needs to be tested - this allows us to rename the test class to highlight the change in intention of the test.
    • Generate 9 blocks fewer to fund the miner wallet.
    • Generate 30 blocks fewer to accommodate for the wallet scan start time while importing descriptor with "now" timestamp.
    • Use common bumpmocktime helper instead of custom advance_time function.
    • Unload miner wallet before 20 blocks generation to avoid notifications from being processed by that wallet.
    • Change the order of arguments in assert_equal for consistency.
    • Add few constant and verbose comments for test clarity.

    Note: This overhaul helps in reducing the time taken by the test from 2s to 0s as observed in few runs.

    <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

    <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

    <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

  2. DrahtBot commented at 12:57 PM on March 20, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. rkrux commented at 11:54 AM on March 21, 2026: contributor

    It appears that this overhaul helps in reducing the tests time as well from 2s to 0s, which is nice to see.

    Few runs that I checked: wallet_reindex test time in macOS native job in other PR1 and PR2 - 2 sec. wallet_birthtime test time in macOS native job in this PR - 0 sec.

  4. w0xlt commented at 8:04 AM on March 22, 2026: contributor

    I’ll take a closer look, but what’s the motivation for those changes?

  5. rkrux commented at 8:34 AM on March 22, 2026: contributor

    I’ll take a closer look, but what’s the motivation for those changes?

    As mentioned in the last line of the PR description, it started from the review of PR #34857 here: #34857 (review)

  6. rkrux commented at 8:38 AM on March 22, 2026: contributor

    Thanks for asking, made me realise that the motivation was obscured away at the end of the PR description. I updated the PR description to highlight the motivation at the start.

  7. DrahtBot added the label Needs rebase on Mar 24, 2026
  8. test, wallet: overhaul wallet_reindex test
    Changes done:
    - The node doesn't need to be restarted (with reindex or without) because
    rescanblockchain RPC already updates the wallet birthtime, which is the property
    that needs to be tested - this allows us to rename the test class to highlight
    the change in intention of the test.
    - Generate 9 blocks fewer to fund the miner wallet.
    - Generate 30 blocks fewer to accommodate for the wallet scan start time while
    importing descriptor with "now" timestamp.
    - Use common bumpmocktime helper instead of custom advance_time function.
    - Unload miner wallet before 20 blocks generation to avoid notifications from
    being processed by that wallet.
    - Change the order of arguments in assert_equal for consistency.
    - Add few constants and verbose comments for test clarity while removing
    comments that allude to legacy wallets.
    ea2e314e8b
  9. test, wallet: rename wallet_reindex to wallet_birthtime
    This rename follows the change in intention of the test caused by the preceding
    commit.
    fbbbe75ba9
  10. rkrux force-pushed on Mar 24, 2026
  11. DrahtBot removed the label Needs rebase on Mar 24, 2026
Contributors

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-24 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me