Merge bitcoin/bitcoin#35097: util: Return uint64_t from _MiB and _GiB operators

fa43da21f1 refactor: Run ShouldWarnOversizedDbCache calculation in u64 (MarcoFalke) fa5801762e util: Return uint64_t from _MiB and _GiB operators (MarcoFalke) fa9ddb01c9 test: Use MiB operator directly in cuckoocache_tests (MarcoFalke) Pull request description: Currently, the `_MiB` literal operator returns a value of type `size_t`. This is brittle and confusing: * It is inherently impossible to represent larger values, like storage device usage. * Similarly, it is not possible to even type an upper cap on the memory, see the failure in https://github.com/bitcoin/bitcoin/pull/34692#issuecomment-3973281952 * Using `size_t` isn't required here. The function is evaluated at compile time, and even 32-bit compilers can evaluate an `uint64_t` at compile time. * Using `size_t` here encourages it to be used in more places, which will likely lead to more bugs and CVEs, such as https://github.com/bitcoin/bitcoin/pull/34109, https://github.com/bitcoin/bitcoin/pull/33724, etc. Fix all issues, by: * Marking the operator `consteval`, to ensure it is really only called at compile-time. * Returning an `uint64_t` value. * Using it in the place where it was previously not possible. Review note: This should have no downside, because the C++11 narrowing checks continue to work as expected. For example, typing `uint8_t{1_MiB};` will continue to fail with the correct error message `error: constant expression evaluates to 1048576 which cannot be narrowed to type 'uint8_t' (aka 'unsigned char') [-Wc++11-narrowing]` (like it does on current master). ACKs for top commit: l0rinc: ACK fa43da21f1 achow101: ACK fa43da21f1 hebasto: ACK fa43da21f1, I have reviewed the code and it looks OK. hodlinator: ACK fa43da21f1 Tree-SHA512: e19f6bd18c519c2a56d44fbc9e65b02e630bc71170f02a8a8955bf5c9bb2924b55ec2f810de2bbc961162aa668b7383c9fe9b5261fa1de7d9e8413003c32fa87
2026-05-15 16:33:52 +02:00 · 2026-04-24 14:52:49 -07:00
parent 2d5ab09f0d fa43da21f1
commit 28a523fb94
7 changed files with 56 additions and 71 deletions
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -30,7 +30,7 @@ static constexpr size_t MAX_32BIT_DBCACHE{1_GiB};
 //! Larger default dbcache on 64-bit systems with enough RAM.
 static constexpr size_t HIGH_DEFAULT_DBCACHE{1_GiB};
 //! Minimum detected RAM required for HIGH_DEFAULT_DBCACHE.
-static constexpr uint64_t HIGH_DEFAULT_DBCACHE_MIN_TOTAL_RAM{uint64_t{4} * 1_GiB};
+static constexpr uint64_t HIGH_DEFAULT_DBCACHE_MIN_TOTAL_RAM{4_GiB};

 namespace node {
 size_t GetDefaultDBCache()
--- a/src/node/caches.h
+++ b/src/node/caches.h
@@ -29,9 +29,9 @@ struct CacheSizes {
    kernel::CacheSizes kernel;
 };
 CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0);
-constexpr bool ShouldWarnOversizedDbCache(size_t dbcache, size_t total_ram) noexcept
+constexpr bool ShouldWarnOversizedDbCache(uint64_t dbcache, uint64_t total_ram) noexcept
 {
-    const size_t cap{(total_ram < 2_GiB) ? DEFAULT_DB_CACHE : (total_ram / 100) * 75};
+    const uint64_t cap{(total_ram < 2_GiB) ? DEFAULT_DB_CACHE : (total_ram / 100) * 75};
    return dbcache > cap;
 }

--- a/src/test/caches_tests.cpp
+++ b/src/test/caches_tests.cpp
@@ -22,23 +22,21 @@ BOOST_AUTO_TEST_CASE(oversized_dbcache_warning)
    BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/1500_MiB, /*total_ram=*/2_GiB)); // Under cap
    BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/1600_MiB, /*total_ram=*/2_GiB)); // Over cap

-    if constexpr (SIZE_MAX == UINT64_MAX) {
-        // 4 GiB RAM - cap is 75%
-        BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/2500_MiB, /*total_ram=*/4_GiB)); // Under cap
-        BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/3500_MiB, /*total_ram=*/4_GiB)); // Over cap
+    // 4 GiB RAM - cap is 75%
+    BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/2500_MiB, /*total_ram=*/4_GiB)); // Under cap
+    BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/3500_MiB, /*total_ram=*/4_GiB)); // Over cap

-        // 8 GiB RAM - cap is 75%
-        BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/6000_MiB, /*total_ram=*/8_GiB)); // Under cap
-        BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/7000_MiB, /*total_ram=*/8_GiB)); // Over cap
+    // 8 GiB RAM - cap is 75%
+    BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/6000_MiB, /*total_ram=*/8_GiB)); // Under cap
+    BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/7000_MiB, /*total_ram=*/8_GiB)); // Over cap

-        // 16 GiB RAM - cap is 75%
-        BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/10'000_MiB, /*total_ram=*/16_GiB)); // Under cap
-        BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/15'000_MiB, /*total_ram=*/16_GiB)); // Over cap
+    // 16 GiB RAM - cap is 75%
+    BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/10_GiB, /*total_ram=*/16_GiB)); // Under cap
+    BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/15_GiB, /*total_ram=*/16_GiB)); // Over cap

-        // 32 GiB RAM - cap is 75%
-        BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/20'000_MiB, /*total_ram=*/32_GiB)); // Under cap
-        BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/30'000_MiB, /*total_ram=*/32_GiB)); // Over cap
-    }
+    // 32 GiB RAM - cap is 75%
+    BOOST_CHECK(!ShouldWarnOversizedDbCache(/*dbcache=*/20_GiB, /*total_ram=*/32_GiB)); // Under cap
+    BOOST_CHECK( ShouldWarnOversizedDbCache(/*dbcache=*/30_GiB, /*total_ram=*/32_GiB)); // Over cap
 }

 BOOST_AUTO_TEST_SUITE_END()
--- a/src/test/cuckoocache_tests.cpp
+++ b/src/test/cuckoocache_tests.cpp
@@ -50,16 +50,15 @@ BOOST_AUTO_TEST_CASE(test_cuckoocache_no_fakes)
 };

 struct HitRateTest : BasicTestingSetup {
-/** This helper returns the hit rate when megabytes*load worth of entries are
- * inserted into a megabytes sized cache
+/** This helper returns the hit rate when bytes*load worth of entries are
+ * inserted into a bytes sized cache
 */
 template <typename Cache>
-double test_cache(size_t megabytes, double load)
+double test_cache(size_t bytes, double load)
 {
    SeedRandomForTest(SeedRand::ZEROS);
    std::vector<uint256> hashes;
    Cache set{};
-    size_t bytes{megabytes * 1_MiB};
    set.setup_bytes(bytes);
    uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
    hashes.resize(n_insert);
@@ -114,9 +113,8 @@ BOOST_FIXTURE_TEST_CASE(cuckoocache_hit_rate_ok, HitRateTest)
     * as a lower bound on performance.
     */
    double HitRateThresh = 0.98;
-    size_t megabytes = 4;
    for (double load = 0.1; load < 2; load *= 2) {
-        double hits = test_cache<CuckooCache::cache<uint256, SignatureCacheHasher>>(megabytes, load);
+        double hits = test_cache<CuckooCache::cache<uint256, SignatureCacheHasher>>(4_MiB, load);
        BOOST_CHECK(normalize_hit_rate(hits, load) > HitRateThresh);
    }
 }
@@ -126,13 +124,12 @@ struct EraseTest : BasicTestingSetup {
 /** This helper checks that erased elements are preferentially inserted onto and
 * that the hit rate of "fresher" keys is reasonable*/
 template <typename Cache>
-void test_cache_erase(size_t megabytes)
+void test_cache_erase(size_t bytes)
 {
    double load = 1;
    SeedRandomForTest(SeedRand::ZEROS);
    std::vector<uint256> hashes;
    Cache set{};
-    size_t bytes{megabytes * 1_MiB};
    set.setup_bytes(bytes);
    uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
    hashes.resize(n_insert);
@@ -185,19 +182,17 @@ void test_cache_erase(size_t megabytes)

 BOOST_FIXTURE_TEST_CASE(cuckoocache_erase_ok, EraseTest)
 {
-    size_t megabytes = 4;
-    test_cache_erase<CuckooCache::cache<uint256, SignatureCacheHasher>>(megabytes);
+    test_cache_erase<CuckooCache::cache<uint256, SignatureCacheHasher>>(4_MiB);
 }

 struct EraseParallelTest : BasicTestingSetup {
 template <typename Cache>
-void test_cache_erase_parallel(size_t megabytes)
+void test_cache_erase_parallel(size_t bytes)
 {
    double load = 1;
    SeedRandomForTest(SeedRand::ZEROS);
    std::vector<uint256> hashes;
    Cache set{};
-    size_t bytes{megabytes * 1_MiB};
    set.setup_bytes(bytes);
    uint32_t n_insert = static_cast<uint32_t>(load * (bytes / sizeof(uint256)));
    hashes.resize(n_insert);
@@ -277,8 +272,7 @@ void test_cache_erase_parallel(size_t megabytes)
 }; // struct EraseParallelTest
 BOOST_FIXTURE_TEST_CASE(cuckoocache_erase_parallel_ok, EraseParallelTest)
 {
-    size_t megabytes = 4;
-    test_cache_erase_parallel<CuckooCache::cache<uint256, SignatureCacheHasher>>(megabytes);
+    test_cache_erase_parallel<CuckooCache::cache<uint256, SignatureCacheHasher>>(4_MiB);
 }


--- a/src/test/system_ram_tests.cpp
+++ b/src/test/system_ram_tests.cpp
@@ -21,12 +21,7 @@ BOOST_AUTO_TEST_CASE(total_ram)
    }

    BOOST_CHECK_GE(*total, 1000_MiB);
-
-    if constexpr (SIZE_MAX == UINT64_MAX) {
-        // Upper bound check only on 64-bit: 32-bit systems can reasonably have max memory,
-        // but extremely large values on 64-bit likely indicate detection errors
-        BOOST_CHECK_LT(*total, 10'000'000_MiB); // >10 TiB memory is unlikely
-    }
+    BOOST_CHECK_LT(*total, 10'000_GiB); // ~10 TiB memory is unlikely
 }

 BOOST_AUTO_TEST_SUITE_END()
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -14,6 +14,7 @@
 #include <test/util/setup_common.h>
 #include <test/util/time.h>
 #include <uint256.h>
+#include <univalue.h>
 #include <util/bitdeque.h>
 #include <util/byte_units.h>
 #include <util/fs.h>
@@ -36,7 +37,7 @@
 #include <optional>
 #include <string>
 #include <thread>
-#include <univalue.h>
+#include <type_traits>
 #include <utility>
 #include <vector>

@@ -1827,6 +1828,9 @@ BOOST_AUTO_TEST_CASE(saturating_left_shift_test)
    TestSaturatingLeftShift<int64_t>();
 }

+template <class Int, auto bytes>
+concept BraceInitializesTo = requires { Int{bytes}; };
+
 BOOST_AUTO_TEST_CASE(mib_string_literal_test)
 {
    // Basic equivalences and simple arithmetic operations
@@ -1850,16 +1854,12 @@ BOOST_AUTO_TEST_CASE(mib_string_literal_test)
    BOOST_CHECK_EQUAL(128_MiB, 0x8000000U);
    BOOST_CHECK_EQUAL(550_MiB, 550ULL * 1024 * 1024);

-    // Overflow handling
-    constexpr auto max_mib{std::numeric_limits<size_t>::max() >> 20};
-    if constexpr (SIZE_MAX == UINT32_MAX) {
-        BOOST_CHECK_EQUAL(max_mib, 4095U);
-        BOOST_CHECK_EQUAL(4095_MiB, size_t{4095} << 20);
-        BOOST_CHECK_EXCEPTION(4096_MiB, std::overflow_error, HasReason("MiB value too large for size_t byte conversion"));
-    } else {
-        BOOST_CHECK_EQUAL(4096_MiB, size_t{4096} << 20);
-    }
-    BOOST_CHECK_EXCEPTION(operator""_MiB(max_mib + 1), std::overflow_error, HasReason("MiB value too large for size_t byte conversion"));
+    // 4095 MiB fits in uint32_t bytes. 4096 MiB requires the uint64_t return type.
+    static_assert(BraceInitializesTo<uint32_t, 4095_MiB>);
+    static_assert(!BraceInitializesTo<uint32_t, 4096_MiB>);
+    static_assert(BraceInitializesTo<uint64_t, 4096_MiB>);
+    BOOST_CHECK_EQUAL(4095_MiB, uint32_t{4095} << 20);
+    BOOST_CHECK_EQUAL(4096_MiB, uint64_t{4096} << 20);
 }

 BOOST_AUTO_TEST_CASE(ceil_div_test)
@@ -1916,20 +1916,18 @@ BOOST_AUTO_TEST_CASE(gib_string_literal_test)
    BOOST_CHECK_EQUAL(3_GiB / 1_GiB, 3U);
    BOOST_CHECK_EQUAL(3_GiB, 3U << 30);

-    // Overflow handling and specific codebase values
-    constexpr auto max_gib{std::numeric_limits<size_t>::max() >> 30};
-    if constexpr (SIZE_MAX == UINT32_MAX) {
-        BOOST_CHECK_EQUAL(max_gib, 3U);
-        BOOST_CHECK_EXCEPTION(4_GiB, std::overflow_error, HasReason("GiB value too large for size_t byte conversion"));
-    } else {
-        BOOST_CHECK_GT(max_gib, 3U);
-        BOOST_CHECK_EQUAL(4_GiB, size_t{4} << 30);
-        BOOST_CHECK_EQUAL(4_GiB, 4096_MiB);
-        BOOST_CHECK_EQUAL(8_GiB, 8192_MiB);
-        BOOST_CHECK_EQUAL(16_GiB, 16384_MiB);
-        BOOST_CHECK_EQUAL(32_GiB, 32768_MiB);
-    }
-    BOOST_CHECK_EXCEPTION(operator""_GiB(max_gib + 1), std::overflow_error, HasReason("GiB value too large for size_t byte conversion"));
+    // 3 GiB fits in uint32_t bytes. 4 GiB requires the uint64_t return type.
+    static_assert(BraceInitializesTo<uint32_t, 3_GiB>);
+    static_assert(!BraceInitializesTo<uint32_t, 4_GiB>);
+    static_assert(BraceInitializesTo<uint64_t, 4_GiB>);
+    BOOST_CHECK_EQUAL(3_GiB, uint32_t{3} << 30);
+    BOOST_CHECK_EQUAL(4_GiB, uint64_t{4} << 30);
+
+    // Specific codebase values
+    BOOST_CHECK_EQUAL(4_GiB, 4096_MiB);
+    BOOST_CHECK_EQUAL(8_GiB, 8192_MiB);
+    BOOST_CHECK_EQUAL(16_GiB, 16384_MiB);
+    BOOST_CHECK_EQUAL(32_GiB, 32768_MiB);
 }

 BOOST_AUTO_TEST_SUITE_END()
--- a/src/util/byte_units.h
+++ b/src/util/byte_units.h
@@ -12,26 +12,26 @@

 namespace util::detail {
 template <unsigned SHIFT>
-constexpr size_t ByteUnitsToBytes(unsigned long long units, const char* exception_msg)
+consteval uint64_t ByteUnitsToBytes(unsigned long long units)
 {
    const auto bytes{CheckedLeftShift(units, SHIFT)};
-    if (!bytes || *bytes > std::numeric_limits<size_t>::max()) {
-        throw std::overflow_error(exception_msg);
+    if (!bytes || *bytes > std::numeric_limits<uint64_t>::max()) {
+        throw std::overflow_error("Too large");
    }
    return *bytes;
 }
 } // namespace util::detail

-//! Overflow-safe conversion of MiB to bytes.
-constexpr size_t operator""_MiB(unsigned long long mebibytes)
+/// Conversion of MiB to bytes.
+consteval uint64_t operator""_MiB(unsigned long long mebibytes)
 {
-    return util::detail::ByteUnitsToBytes<20>(mebibytes, "MiB value too large for size_t byte conversion");
+    return util::detail::ByteUnitsToBytes<20>(mebibytes);
 }

-//! Overflow-safe conversion of GiB to bytes.
-constexpr size_t operator""_GiB(unsigned long long gibibytes)
+/// Conversion of GiB to bytes.
+consteval uint64_t operator""_GiB(unsigned long long gibibytes)
 {
-    return util::detail::ByteUnitsToBytes<30>(gibibytes, "GiB value too large for size_t byte conversion");
+    return util::detail::ByteUnitsToBytes<30>(gibibytes);
 }

 #endif // BITCOIN_UTIL_BYTE_UNITS_H