wallet: remove outdated `RewriteDB` calls from SPKM & `DBErrors::NEED_REWRITE` enum value #34301

pull rkrux wants to merge 2 commits into bitcoin:master from rkrux:rewritedb changing 4 files +0 −22
  1. rkrux commented at 10:11 AM on January 15, 2026: contributor

    ISTM that there is no implementation left of the RewriteDB method in any of the SPKMs, and thus, its call sites can be removed safely.

    Also remove DBErrors::NEED_REWRITE enum value as its usage is outdated now.

    <!-- *** 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 added the label Wallet on Jan 15, 2026
  3. DrahtBot commented at 10:11 AM on January 15, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34301.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK achow101, furszy, sedited

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    No conflicts as of last run.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. achow101 commented at 9:09 PM on January 15, 2026: member

    Concept ACK

    I think this can actually extend a bit futher into getting rid of DBErrors::NEED_REWRITE entirely. This only exists for wallets created in 0.4.x which had the bug that the database was not rewritten. However, given that BDB is no longer supported at all and migration is explicitly a rewrite anyways, there's actually no situation in which NEED_REWRITE being returned is ever doing anything useful.

    The only places we should ever need WalletDatabase::Rewrite are in wallet encryption.

  5. rkrux renamed this:
    wallet: remove unimplemented `RewriteDB` calls from SPKM
    wallet: remove outdated `RewriteDB` calls from SPKM & `DBErrors::NEED_REWRITE` enum value
    on Jan 16, 2026
  6. rkrux commented at 11:02 AM on January 16, 2026: contributor

    I think this can actually extend a bit futher into getting rid of DBErrors::NEED_REWRITE entirely.

    Thanks, I have added a commit addressing this point.

  7. achow101 commented at 9:01 PM on January 19, 2026: member

    ACK 5c802c73c0d40cf327bbe3333853b9ebfb5b7563

  8. DrahtBot added the label Needs rebase on Feb 4, 2026
  9. wallet: remove unimplemented `RewriteDB` calls from SPKM
    ISTM that there is no implementation left of the `RewriteDB` method
    in any of the SPKMs, and thus, its call sites can be removed safely.
    61039d72a5
  10. wallet: remove `DBErrors::NEED_REWRITE` enum value
    As highlighted in the PR review comment, this enum seems no longer
    required as the specific issue it solves for involves BDB based wallets
    that can't be loaded anymore outside the context of wallet migration,
    which rewrites the database anyway.
    c6a6435ced
  11. rkrux force-pushed on Feb 5, 2026
  12. DrahtBot removed the label Needs rebase on Feb 5, 2026
  13. achow101 commented at 6:38 PM on February 5, 2026: member

    ACK c6a6435cedac01a6800cac9285e2c599290e2e0d

  14. furszy commented at 11:00 PM on March 2, 2026: member

    ACK c6a6435cedac01a6800cac9285e2c599290e2e0d

  15. sedited approved
  16. sedited commented at 12:51 PM on March 11, 2026: contributor

    ACK c6a6435cedac01a6800cac9285e2c599290e2e0d

  17. sedited merged this on Mar 11, 2026
  18. sedited closed this on Mar 11, 2026


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-17 03:12 UTC

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