refactor: use `_MiB`/`_GiB` consistently for byte conversions

l0rinc commented at 9:26 pm on January 28, 2026: contributor

Problem

Byte-size conversions in the codebase currently show up in many equivalent formats (multiplication/division chains, shifts, hex/binary literals), which creates a maintenance burden and makes review error-prone - especially considering the architectural differences of size_t. Inspired by #34305 (review), it seemed appropriate to unify Mebibyte usage across the codebase and add Gibibyte support with 32/64 bit size_t validation.

Fix

This PR refactors those call sites to use ""_MiB (existing) and ""_GiB (new), and adds the encountered value/pattern replacements to unit tests to make review straightforward, and to ensure the conversions remain valid. The literals are overflow-checked when converting to size_t, and unit tests cover the 32-bit boundary cases.

Concretely, it replaces patterns such as:

1024*1024, 1<<20, 0x100000, 1048576, / 1024 / 1024, * (1.0 / 1024 / 1024) → 1_MiB or double(1_MiB)
1024*1024*1024, 1<<30, 0x40000000, 1024_MiB, >> 30 → 1_GiB

Note

In the few places where arithmetic involves signed values, the result is identical to the previous code assuming those quantities never become negative.

DrahtBot added the label Refactoring on Jan 28, 2026

DrahtBot commented at 9:26 pm on January 28, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hodlinator

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)
#33512 (coins: use number of dirty cache entries in flush warnings/logs by l0rinc)
#33259 (rpc, logging: add backgroundvalidation to getblockchaininfo by polespinasa)
#32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by sedited)
#29678 (Bugfix: Correct first-run free space checks by luke-jr)
#24539 (Add a “tx output spender” index by sstone)

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.

l0rinc force-pushed on Jan 28, 2026

DrahtBot added the label CI failed on Jan 28, 2026

DrahtBot commented at 9:34 pm on January 28, 2026: contributor

🚧 At least one of the CI tasks failed. Task ASan + LSan + UBSan + integer: https://github.com/bitcoin/bitcoin/actions/runs/21456037564/job/61797017546 LLM reason (✨ experimental): Compilation failed due to invalid user-defined literal operators in util/byte_units.h (invalid parameter type for operator""_MiB/_GiB leading to no matching literal operator).

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

l0rinc closed this on Jan 28, 2026

l0rinc force-pushed on Jan 28, 2026

l0rinc reopened this on Jan 28, 2026

l0rinc force-pushed on Jan 28, 2026

DrahtBot removed the label CI failed on Jan 28, 2026

in src/util/byte_units.h:16 in 0768e6bc84 outdated

13 
14-//! Overflow-safe conversion of MiB to bytes.
15-constexpr size_t operator""_MiB(unsigned long long mebibytes)
16+namespace util::detail {
17+template<unsigned SHIFT>
18+constexpr size_t ByteUnitsToBytes(unsigned long long units, const std::string& exception_msg)

hodlinator commented at 3:52 pm on January 29, 2026:

Should use string_view or const char* to avoid heap allocations for the happy path.

0constexpr size_t ByteUnitsToBytes(unsigned long long units, std::string_view exception_msg)

(Edit: I realize this is only run in a constexpr context so it’s probably negligible in practice, but would still be nice to fix).

l0rinc commented at 6:02 pm on January 29, 2026:

I had that originally but had some problems, don’t remember what. Will try again on next push. Edit: it’s used in std::overflow_error which expects a String:

0  _LIBCPP_HIDE_FROM_ABI explicit overflow_error(const string& __s) : runtime_error(__s) {}
1  _LIBCPP_HIDE_FROM_ABI explicit overflow_error(const char* __s) : runtime_error(__s) {}

hodlinator commented at 7:32 pm on January 29, 2026:

Yes, so why not use const char* to avoid the “heap” allocation (usually in constexpr/consteval) in cases where the function succeeds and the exception isn’t even thrown?

l0rinc commented at 7:53 pm on January 29, 2026:

That would be true without constexpr, as shown in https://godbolt.org/z/TGb57r7zz. But with constexpr it has zero runtime impact since the exception string is completely optimized away e.g. 8_MiB and friends are always compile-time constants, see https://godbolt.org/z/7rhfqc39f. But it would eliminate a string import, so I don’t mind, pushed.

in src/qt/optionsmodel.h:30 in 0768e6bc84 outdated

26@@ -26,12 +27,12 @@ static constexpr uint16_t DEFAULT_GUI_PROXY_PORT = 9050;
27 /**
28  * Convert configured prune target MiB to displayed GB. Round up to avoid underestimating max disk usage.
29  */
30-static inline int PruneMiBtoGB(int64_t mib) { return (mib * 1024 * 1024 + GB_BYTES - 1) / GB_BYTES; }
31+static inline int PruneMiBtoGB(int64_t mib) { return (mib * 1_MiB + GB_BYTES - 1) / GB_BYTES; }

hodlinator commented at 4:06 pm on January 29, 2026:

I wonder if multiplying with size_t instead of unspecified integer expression risks flipping negative function inputs into positive output values, but it’s probably only used with positive inputs regardless.

l0rinc commented at 6:47 pm on January 29, 2026:

https://github.com/bitcoin/bitcoin/blob/25884bd89684262b94e09c9904c1560d5ba87d86/src/qt/intro.cpp#L35 indicates this cannot be negative

hodlinator commented at 9:41 pm on January 29, 2026:

Experimented a bit and it seems behavior hasn’t changed. Passing test:

 0constexpr uint64_t GB_BYTES{1000000000};
 1static inline int PruneMiBtoGB_old(int64_t mib) { return (mib * 1024 * 1024 + GB_BYTES - 1) / GB_BYTES; }
 2static inline int PruneMiBtoGB_new(int64_t mib) { return (mib * 1_MiB + GB_BYTES - 1) / GB_BYTES; }
 3static inline int64_t PruneGBtoMiB_old(int gb) { return gb * GB_BYTES / 1024 / 1024; }
 4static inline int64_t PruneGBtoMiB_new(int gb) { return gb * GB_BYTES / 1_MiB; }
 5
 6BOOST_AUTO_TEST_CASE(prune_tests)
 7{
 8    BOOST_CHECK_EQUAL(PruneMiBtoGB_old(1500), 2);
 9    BOOST_CHECK_EQUAL(PruneMiBtoGB_new(1500), 2);
10    BOOST_CHECK_EQUAL(PruneMiBtoGB_old(-1500), 1266874889);
11    BOOST_CHECK_EQUAL(PruneMiBtoGB_new(-1500), 1266874889);
12
13    BOOST_CHECK_EQUAL(PruneGBtoMiB_old(1), 953);
14    BOOST_CHECK_EQUAL(PruneGBtoMiB_new(1), 953);
15    BOOST_CHECK_EQUAL(PruneGBtoMiB_old(-1), 17592186043462);
16    BOOST_CHECK_EQUAL(PruneGBtoMiB_new(-1), 17592186043462);
17}

in src/util/byte_units.h:15 in 0768e6bc84

12 #include <stdexcept>
13 
14-//! Overflow-safe conversion of MiB to bytes.
15-constexpr size_t operator""_MiB(unsigned long long mebibytes)
16+namespace util::detail {
17+template<unsigned SHIFT>

hodlinator commented at 4:20 pm on January 29, 2026:

nit: I suspect clang-format prefers:

0template <unsigned SHIFT>

0$ git grep "template <" src/ | wc -l
1807
2$ git grep "template<" src/ | wc -l
3424

l0rinc commented at 6:04 pm on January 29, 2026:

Thanks, will do it on next push

Edit: pushed this nit + rebased

hodlinator changes_requested

l0rinc force-pushed on Jan 29, 2026

DrahtBot added the label CI failed on Jan 29, 2026

in src/init.cpp:1923 in 112b991463 outdated

1919@@ -1919,7 +1920,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
1920 
1921     // On first startup, warn on low block storage space
1922     if (!do_reindex && !do_reindex_chainstate && chain_active_height <= 1) {
1923-        uint64_t assumed_chain_bytes{chainparams.AssumedBlockchainSize() * 1024 * 1024 * 1024};
1924+        uint64_t assumed_chain_bytes{chainparams.AssumedBlockchainSize() * 1_MiB * 1024};

hodlinator commented at 8:49 pm on January 29, 2026:

In 112b9914639aab450f0aadc187f6985f56cb7e70: How about putting the GiB commit first and only touch this line once instead of twice?

l0rinc commented at 9:50 pm on January 29, 2026:

Good idea, haven’t thought of that! Pushed - also simplified the gib_string_literal_test to remove some duplication.

DrahtBot removed the label CI failed on Jan 29, 2026

hodlinator commented at 9:42 pm on January 29, 2026: contributor

Concept ACK e5b573f18ac2e13508330ada6e92c8155f86a0e5

Makes the code more readible.

Examples:

 0-constexpr unsigned int FLTR_FILE_CHUNK_SIZE = 0x100000; // 1 MiB
 1+constexpr unsigned int FLTR_FILE_CHUNK_SIZE = 1_MiB;
 2
 3-if (m_total_allocated > 0x1000000) return;
 4+if (m_total_allocated > 16_MiB) return;
 5
 6-cuckoo_cache.setup_bytes(megabytes << 20);
 7+cuckoo_cache.setup_bytes(megabytes * 1_MiB);
 8
 9-LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.ApproximateSize() * (1.0 / 1048576.0));
10+LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.ApproximateSize() / double(1_MiB));
11
12-constexpr uint64_t min_disk_space = 52428800; // 50 MiB
13+constexpr uint64_t min_disk_space = 50_MiB;

l0rinc force-pushed on Jan 29, 2026

l0rinc requested review from hodlinator on Jan 29, 2026

in src/test/util_tests.cpp:1882 in 792d679a54 outdated

1879+    BOOST_CHECK_EQUAL(3_GiB, 3U << 30);
1880+
1881+    // Overflow handling and specific codebase values
1882+    constexpr auto max_gib{std::numeric_limits<size_t>::max() >> 30};
1883+    if constexpr (SIZE_MAX == UINT32_MAX) {
1884+        BOOST_CHECK_EQUAL(max_gib, 3U);

hodlinator commented at 10:27 pm on January 29, 2026:

remark:

Initial thought: “(2^32) >> 30 should be 2^2 == 4, not 3”.
Then I realized uint32_t-max is (2^32)-1 and rounds down.

hodlinator approved

hodlinator commented at 10:36 pm on January 29, 2026: contributor

ACK 792d679a54dcaf359628554fa2681c03d05f1676

Concpt. review: #34435#pullrequestreview-3724982952

Checked trickier hex literals actually match _MiB/_GiB-versions.

Diffed against previously reviewed push and aside from commit re-ordering, only saw minimal deduplication in gib_string_literal_test.

DrahtBot added the label Needs rebase on Feb 7, 2026

util: add _GiB for Gibibyte conversions

Introduce `operator""_GiB`, sharing the overflow-checked conversion logic with the existing `operator""_MiB`.

Use `1_GiB` in a few existing places where it is a drop-in replacement (e.g. `1024_MiB`, `1<<30`) and extend unit tests to cover boundary behavior.

e78d03ee51

refactor: use _MiB consistently for Mebibyte conversions

Replace hard-coded MiB byte conversions (e.g. `1024*1024`, `1<<20`, `1048576`) with the existing `_MiB` literal to improve readability and avoid repeating constants.

In the few spots where arithmetic involves signed values, the result is identical to the previous code assuming those quantities never turn negative.

Extend unit tests to cover the 32-bit `size_t` overflow boundary and to assert equivalence for integer and floating-point conversions.

6091d9105d

l0rinc force-pushed on Feb 8, 2026

DrahtBot removed the label Needs rebase on Feb 8, 2026

hodlinator approved

hodlinator commented at 9:42 am on February 9, 2026: contributor

re-ACK 6091d9105d88ca4a43385dcb1936cf9a0b6c6b03

Change since previous review: Adjusts the second commit to merge cleanly after surrounding code changed, mainly #includes.

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

Problem

Fix

Note

Reviews

Conflicts

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