Bugfix: Correct first-run free space checks #29678

pull luke-jr wants to merge 4 commits into bitcoin:master from luke-jr:fix_init_lowdisk_warning_reqd changing 4 files +65 −4

luke-jr commented at 4:36 pm on March 19, 2024: member

It’s not clear what m_assumed_*_size are actually set based on, but historically it was in GB, not GiB, and that’s still used in the GUI which is more user-facing.

Could just as easily change the GUI if GiB is preferred.
DrahtBot commented at 4:36 pm on March 19, 2024: contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

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

Reviews

See the guideline for information on the review process.

Type Reviewers

Concept ACK hebasto, murchandamus, BrandonOdiwuor

Stale ACK sedited

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:
- #34436 (refactor + log: use new CeilDiv helper in UTXO Flushing warning by l0rinc)
- #34435 (refactor: use _MiB/_GiB consistently for byte conversions by l0rinc)
- #31974 (Drop testnet3 by Sjors)
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.

LLM Linter (✨ experimental)

Possible places where comparison-specific test macros should replace generic comparisons:
- [src/test/util_tests.cpp] BOOST_CHECK_THROW((void)CeilDiv(1ULL, 0ULL), NonFatalCheckError); -> Use BOOST_CHECK_EXCEPTION to also verify the failure message, e.g. BOOST_CHECK_EXCEPTION((void)CeilDiv(1ULL, 0ULL), NonFatalCheckError, HasReason(“expected failure message”));
- [src/test/util_tests.cpp] BOOST_CHECK_THROW((void)CeilDiv(size_t{1}, size_t{0}), NonFatalCheckError); -> Use BOOST_CHECK_EXCEPTION to also verify the failure message, e.g. BOOST_CHECK_EXCEPTION((void)CeilDiv(size_t{1}, size_t{0}), NonFatalCheckError, HasReason(“expected failure message”));
2026-01-29 07:26:52
Sjors commented at 5:37 pm on March 19, 2024: member

I’m not sure what the latest convention is, cc @hebasto. A few years ago #15163 made it so RPC, code and GUI all use KiB / MiB / GiB.

(though for pruning it seems the config file uses MiB, but the GUI converts it to GB - yet there are translated error strings using MiB, confusing…)
DrahtBot added the label Needs rebase on May 20, 2024
luke-jr force-pushed on May 28, 2024
DrahtBot removed the label Needs rebase on May 28, 2024
DrahtBot added the label Needs rebase on Jun 10, 2024
fanquake commented at 3:44 pm on June 25, 2024: member

@hebasto can you follow up given the gui / translation Qs here. This also needs a rebase.
hebasto commented at 5:56 pm on July 10, 2024: member

Indeed, there is inconsistency in GB/GiB unit usage.

For instance, the value of the m_assumed_blockchain_size variable in GiB is used with the “GB” units in the user-faced messages here: https://github.com/bitcoin/bitcoin/blob/45f757c72672fc351bf669e731744f2f5e269233/src/init.cpp#L1707 and here: https://github.com/bitcoin/bitcoin/blob/45f757c72672fc351bf669e731744f2f5e269233/src/qt/forms/intro.ui#L206

Concept ACK.

It would be nice to mention the second commit changes in the PR description, no?

Could just as easily change the GUI if GiB is preferred.

The GUI now uses “GB” (SI prefix) as a unit, based on the mindset of non-technical/non-CS users. @GBKS What are modern guidelines in this regard?

@Sjors

I’m not sure what the latest convention is, cc @hebasto. A few years ago #15163 made it so RPC, code and GUI all use KiB / MiB / GiB.

(though for pruning it seems the config file uses MiB, but the GUI converts it to GB - yet there are translated error strings using MiB, confusing…)

For the GUI, the conversion to/from GB is still used: https://github.com/bitcoin/bitcoin/blob/45f757c72672fc351bf669e731744f2f5e269233/src/qt/optionsmodel.h#L26-L34
GBKS commented at 6:45 am on July 11, 2024: none

The GUI now uses “GB” (SI prefix) as a unit, based on the mindset of non-technical/non-CS users.

GB is standard for general audiences, and GiB for technical, scientific, or engineering contexts. I’d recommend keeping GB in the GUI.

I am curious about use cases where it is essential for the user to know the GiB value in order to make decisions.
luke-jr force-pushed on Sep 2, 2024
DrahtBot removed the label Needs rebase on Sep 2, 2024
achow101 requested review from murchandamus on Oct 15, 2024
murchandamus commented at 8:28 am on October 17, 2024: member

All of these changes seem consistent to me.

Concept ACK on generally using GB to refer to 1,000,000,000 bytes and GiB to refer to 1024³ bytes everywhere.
BrandonOdiwuor commented at 10:56 am on November 18, 2024: contributor

Concept ACK using GB to represent 10003 bytes for consistency with GUI
DrahtBot added the label CI failed on Mar 13, 2025
maflcko commented at 1:06 pm on May 4, 2025: member

(close-open for fresh GitHub CI, I guess the Cirrus failure can be ignored for now)
maflcko closed this on May 4, 2025
maflcko reopened this on May 4, 2025
maflcko closed this on Sep 3, 2025
maflcko reopened this on Sep 3, 2025
maflcko removed the label CI failed on Sep 26, 2025
sedited commented at 11:46 am on October 11, 2025: contributor

Concept ACK
sedited approved
sedited commented at 5:02 pm on January 2, 2026: contributor

ACK c452d6c1efe253daa91f1269d378ff82971315f0
DrahtBot requested review from hebasto on Jan 2, 2026
DrahtBot requested review from BrandonOdiwuor on Jan 2, 2026
DrahtBot requested review from murchandamus on Jan 2, 2026

Type	Reviewers
Concept ACK	hebasto, murchandamus, BrandonOdiwuor
Stale ACK	sedited

util: add overflow-safe `CeilDiv` helper

Introduce `CeilDiv()` for integral ceiling division without the typical `(dividend + divisor - 1) / divisor` overflow.
`CeilDiv()` asserts non-negative arguments and a non-zero divisor.
Add unit tests covering return type deduction, max-value behavior, and divisor checks.

c41dd358fe

Bugfix: init: Correct conversion of AssumedBlockchainSize to use GB ff564dc656
Bugfix: init: For first-run disk space check, advise user of correct pruned size rather than full blockchain size 8b33935ea6
doc/release-process: Correct m_assumed_*_size to GB b1117e5a71
luke-jr force-pushed on Jan 29, 2026
luke-jr commented at 7:28 am on January 29, 2026: member

Used @l0rinc ’s CeilDiv (from #34436) to round up prune expectations
luke-jr referenced this in commit 8bc1b55baf on Jan 29, 2026
luke-jr referenced this in commit 58ceddd389 on Jan 29, 2026

Contributors
luke-jr DrahtBot Sjors fanquake hebasto GBKS murchandamus BrandonOdiwuor maflcko sedited

Review Requested
murchandamus BrandonOdiwuor hebasto

Bugfix: Correct first-run free space checks #29678

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)