Bugfix: Correct first-run free space checks by luke-jr · Pull Request #29678 · bitcoin/bitcoin

luke-jr · 2024-03-19T16:36:10Z

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 · 2024-03-19T16:36:13Z

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  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34628 (p2p: Replace per-peer transaction rate-limiting with global rate limits by ajtowns)
#34436 (refactor: add overflow-safe CeilDiv helper and use it in unsigned callsites by l0rinc)
#34435 (refactor: use _MiB/_GiB consistently for byte conversions 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.

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 · 2024-03-19T17:37:36Z

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...)

fanquake · 2024-06-25T15:44:09Z

@hebasto can you follow up given the gui / translation Qs here. This also needs a rebase.

hebasto · 2024-07-10T17:56:00Z

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:

bitcoin/src/init.cpp

Line 1707 in 45f757c

"Approximately %u GB of data will be stored in this directory."

and here:

bitcoin/src/qt/forms/intro.ui

Line 206 in 45f757c

    
           <string>When you click OK, %1 will begin to download and process the full %4 block chain (%2 GB) starting with the earliest transactions in %3 when %4 initially launched.</string>

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:

bitcoin/src/qt/optionsmodel.h

Lines 26 to 34 in 45f757c

    
           /** 
        
            * Convert configured prune target MiB to displayed GB. Round up to avoid underestimating max disk usage. 
        
            */ 
        
           static inline int PruneMiBtoGB(int64_t mib) { return (mib * 1024 * 1024 + GB_BYTES - 1) / GB_BYTES; } 
        
           /** 
        
            * Convert displayed prune target GB to configured MiB. Round down so roundtrip GB -> MiB -> GB conversion is stable. 
        
            */ 
        
           static inline int64_t PruneGBtoMiB(int gb) { return gb * GB_BYTES / 1024 / 1024; }

GBKS · 2024-07-11T06:45:36Z

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.

murchandamus

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

Concept ACK using GB to represent 1000³ bytes for consistency with GUI

maflcko · 2025-05-04T13:06:15Z

(close-open for fresh GitHub CI, I guess the Cirrus failure can be ignored for now)

sedited · 2025-10-11T11:46:52Z

Concept ACK

sedited

ACK c452d6c

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.

…pruned size rather than full blockchain size

luke-jr · 2026-01-29T07:28:14Z

Used @l0rinc 's CeilDiv (from #34436) to round up prune expectations

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. Github-Pull: bitcoin#29678 Rebased-From: c41dd35

…quirement Github-Pull: bitcoin#29678 Rebased-From: 8b33935

DrahtBot · 2026-03-11T11:33:54Z

🐙 This pull request conflicts with the target branch and needs rebase.

This was referenced Apr 1, 2024

Testnet4 including PoW difficulty adjustment fix #29775

Merged

kernel: De-globalize fReindex #29817

Merged

DrahtBot added the Needs rebase label May 20, 2024

luke-jr mentioned this pull request May 28, 2024

doc: Change GiB to GB in release-process.md #30171

Closed

luke-jr force-pushed the fix_init_lowdisk_warning_reqd branch from 279c4ee to 847ad93 Compare May 28, 2024 15:02

DrahtBot removed the Needs rebase label May 28, 2024

DrahtBot mentioned this pull request May 30, 2024

Stratum v2 Template Provider (take 3) #29432

Closed

24 tasks

DrahtBot mentioned this pull request Jun 7, 2024

indexes: Don't wipe indexes again when continuing a prior reindex #30132

Merged

DrahtBot added the Needs rebase label Jun 10, 2024

hebasto mentioned this pull request Jul 15, 2024

Welcome screen uses GiB values as GB, risking running out of space bitcoin-core/gui#821

Open

1 task

luke-jr force-pushed the fix_init_lowdisk_warning_reqd branch from 847ad93 to c452d6c Compare September 2, 2024 19:53

DrahtBot removed the Needs rebase label Sep 2, 2024

achow101 requested a review from murchandamus October 15, 2024 15:40

murchandamus reviewed Oct 17, 2024

View reviewed changes

BrandonOdiwuor reviewed Nov 18, 2024

View reviewed changes

DrahtBot mentioned this pull request Mar 3, 2025

Drop testnet3 #31974

Draft

DrahtBot added the CI failed label Mar 13, 2025

maflcko closed this May 4, 2025

maflcko reopened this May 4, 2025

This was referenced Jun 12, 2025

docs: adds correct updated documentation links #32699

Closed

net: Fix Discover() not running when using -bind=0.0.0.0:port #32757

Open

maflcko closed this Sep 3, 2025

maflcko reopened this Sep 3, 2025

maflcko removed the CI failed label Sep 26, 2025

sedited approved these changes Jan 2, 2026

View reviewed changes

DrahtBot requested review from BrandonOdiwuor, hebasto and murchandamus January 2, 2026 17:02

DrahtBot mentioned this pull request Jan 15, 2026

log: correct size output in disk space check #34305

Closed

luke-jr added 4 commits January 29, 2026 07:13

Bugfix: init: Correct conversion of AssumedBlockchainSize to use GB

ff564dc

Bugfix: init: For first-run disk space check, advise user of correct …

8b33935

…pruned size rather than full blockchain size

doc/release-process: Correct m_assumed_*_size to GB

b1117e5

luke-jr force-pushed the fix_init_lowdisk_warning_reqd branch from c452d6c to b1117e5 Compare January 29, 2026 07:26

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Jan 29, 2026

Bugfix: init: For first-run disk space check, round up pruned size re…

58ceddd

…quirement Github-Pull: bitcoin#29678 Rebased-From: 8b33935

This was referenced Jan 29, 2026

refactor: add overflow-safe CeilDiv helper and use it in unsigned callsites #34436

Merged

refactor: use _MiB/_GiB consistently for byte conversions #34435

Open

DrahtBot mentioned this pull request Feb 20, 2026

p2p: Replace per-peer transaction rate-limiting with global rate limits #34628

Open

DrahtBot added the Needs rebase label Mar 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: Correct first-run free space checks#29678

Bugfix: Correct first-run free space checks#29678
luke-jr wants to merge 4 commits intobitcoin:masterfrom
luke-jr:fix_init_lowdisk_warning_reqd

luke-jr commented Mar 19, 2024

Uh oh!

DrahtBot commented Mar 19, 2024 •

edited

Loading

Uh oh!

Sjors commented Mar 19, 2024 •

edited

Loading

Uh oh!

fanquake commented Jun 25, 2024

Uh oh!

hebasto commented Jul 10, 2024

Uh oh!

GBKS commented Jul 11, 2024

Uh oh!

murchandamus left a comment

Uh oh!

BrandonOdiwuor left a comment •

edited

Loading

Uh oh!

maflcko commented May 4, 2025

Uh oh!

sedited commented Oct 11, 2025

Uh oh!

sedited left a comment

Uh oh!

luke-jr commented Jan 29, 2026

Uh oh!

DrahtBot commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Conversation

luke-jr commented Mar 19, 2024

Uh oh!

DrahtBot commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

Sjors commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Jun 25, 2024

Uh oh!

hebasto commented Jul 10, 2024

Uh oh!

GBKS commented Jul 11, 2024

Uh oh!

murchandamus left a comment

Choose a reason for hiding this comment

Uh oh!

BrandonOdiwuor left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented May 4, 2025

Uh oh!

sedited commented Oct 11, 2025

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Jan 29, 2026

Uh oh!

DrahtBot commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

DrahtBot commented Mar 19, 2024 •

edited

Loading

Sjors commented Mar 19, 2024 •

edited

Loading

BrandonOdiwuor left a comment •

edited

Loading