init: correct first-run disk space estimate #35948

pull l0rinc wants to merge 1 commits into bitcoin:master from l0rinc:l0rinc/init-first-run-disk-space changing 2 files +5 −4
  1. l0rinc commented at 10:35 PM on August 10, 2026: contributor

    Problem: The assumed blockchain size is maintained in GiB, but the first-run disk-space warning labels it as GB. For pruned nodes, the warning also displays the full-chain estimate even though the check uses the lower of the prune target and that estimate. The chainparams API comments likewise describe both assumed sizes as GB.

    Fix: Label the warning as GiB, display the same rounded-up GiB estimate that the disk-space check uses, and document both assumed sizes as GiB.

    This PR revives the first-run warning fixes from #29678.

  2. DrahtBot commented at 10:35 PM on August 10, 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/35948.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK murchandamus, jeanpablojp, achow101
    Stale ACK mercie-ux

    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

    Reviewers, this pull request conflicts with the following ones:

    • #33324 (blocks: add resumable reobfuscation for existing block files by l0rinc)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. mercie-ux commented at 6:28 AM on August 12, 2026: none

    ACK fc4342822b

    Traced the 856 in chainparams.cpp through both the old and new math. Found a real 63 GB gap between what the check silently required and what the GUI ever showed. Confirmed.

  4. DrahtBot added the label Needs rebase on Aug 14, 2026
  5. achow101 commented at 10:36 PM on August 17, 2026: member

    Rather than changing the units that we use in the code, why not change the units as displayed to the user? It shouldn't be that hard to change the units in startup messages since those are just strings.

    As one of the people who do/review the chainparams updates, I strongly prefer to use GiB rather than GB since du uses the *iB units by default. Most of these updates are done by looking at commands used in previous PRs, none of which have used *iB units, so I would expect this to be a little bit confusing. Also *B units is actually (annoyingly) ambiguous since some places use it to mean multiples of 1024 (i.e. same as *iB) while others use it as multiples of 1000. The *iB units are actually unambiguous.

  6. l0rinc renamed this:
    init: correct first-run disk space estimates
    init: correct first-run disk space estimate
    on Aug 17, 2026
  7. init: correct first-run disk space estimate
    The assumed blockchain size is maintained in GiB, but the first-run warning labels it as GB.
    For pruned nodes, the disk-space check uses the lower prune target while the warning displays the full-chain size.
    
    Label the value as GiB and round the checked byte estimate up to GiB so the warning describes the selected storage mode.
    
    Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
    Co-authored-by: Ava Chow <github@achow101.com>
    42215b8581
  8. l0rinc force-pushed on Aug 17, 2026
  9. l0rinc commented at 11:24 PM on August 17, 2026: contributor

    I assumed the original change had a good reason for moving these values to GB, but keeping them in GiB makes this change a lot simpler! Thanks for the review, pushed and added you as a co-author.

  10. DrahtBot removed the label Needs rebase on Aug 18, 2026
  11. murchandamus commented at 10:29 PM on August 21, 2026: member

    ACK 42215b85811a4950265fa2d02b22e83046ed95d5

    Thanks for picking this up. — I’m not sure the co-authorship is needed here, your code is not reusing any of the lines from the original, you’re just proposing an alternative fix to the identified problem.

  12. jeanpablojp commented at 10:13 PM on August 22, 2026: contributor

    tACK 42215b85811a4950265fa2d02b22e83046ed95d5

    Ran the pruned case on a 585 MiB volume, -prune=550 makes master warn about 856 GB where this branch says 1 GiB.

  13. fanquake commented at 9:15 AM on August 24, 2026: member

    cc @hebasto @GBKS given comments in #29678.

  14. achow101 commented at 9:22 PM on August 24, 2026: member

    ACK 42215b85811a4950265fa2d02b22e83046ed95d5

    Going to wait for a UI person's opinion before merging though.

  15. fanquake commented at 10:53 PM on August 24, 2026: member

    before merging though.

    Given this was changed to change strings, and we past string/translation freeze, I don't think it can be merged anyways?

  16. achow101 commented at 11:09 PM on August 24, 2026: member

    Given this was changed to change strings, and we past string/translation freeze

    oh right

  17. maflcko added this to the milestone 33.0 on Aug 25, 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-09-04 08:51 UTC

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