Repository: bitcoin/bitcoin
File: src/kernel/caches.h (C/C++)

Code before:
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_KERNEL_CACHES_H
#define BITCOIN_KERNEL_CACHES_H

#include <util/check.h>

#include <algorithm>
#include <bit>
#include <cstdint>
#include <limits>

//! Suggested default amount of cache reserved for the kernel (MiB)
static constexpr int64_t DEFAULT_KERNEL_CACHE{450};
//! Max memory allocated to block tree DB specific cache (MiB)
static constexpr int64_t MAX_BLOCK_DB_CACHE{2};
//! Max memory allocated to coin DB specific cache (MiB)
static constexpr int64_t MAX_COINS_DB_CACHE{8};

//! Guard against truncation of values before converting.
constexpr size_t MiBToBytes(int64_t mib)
{
    Assert(std::countl_zero(static_cast<uint64_t>(mib)) >= 21); // Ensure sign bit is unset + enough zeros to shift.
    const int64_t bytes{mib << 20};
    Assert(static_cast<uint64_t>(bytes) <= std::numeric_limits<size_t>::max());
    return static_cast<size_t>(bytes);
}

namespace kernel {
struct CacheSizes {
    size_t block_tree_db;
    size_t coins_db;
    size_t coins;

    CacheSizes(size_t total_cache)
    {
        block_tree_db = std::min(total_cache / 8, MiBToBytes(MAX_BLOCK_DB_CACHE));
        total_cache -= block_tree_db;
        coins_db = std::min(total_cache / 2, MiBToBytes(MAX_COINS_DB_CACHE));
        total_cache -= coins_db;
        coins = total_cache; // the rest goes to the coins cache
    }
};
} // namespace kernel

#endif // BITCOIN_KERNEL_CACHES_H

Review comment (performance):
I think a `std::optional<size_t>` return type is more appropriate for this function, the assertions make this function hard to test. Below diff updates the return type, adds a positiveness check (as per my other comment), and adds a test case:

<details>
<summary>git diff on 578654c686</summary>

```diff
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 46ca35ea90..623aec5aa0 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -123,7 +123,7 @@ int main(int argc, char* argv[])
     util::SignalInterrupt interrupt;
     ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
 
-    kernel::CacheSizes cache_sizes{MiBToBytes(DEFAULT_KERNEL_CACHE)};
+    kernel::CacheSizes cache_sizes{*Assert(MiBToBytes(DEFAULT_KERNEL_CACHE))};
     node::ChainstateLoadOptions options;
     auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
diff --git a/src/kernel/caches.h b/src/kernel/caches.h
index 2b5ba9eefc..298ebf8d43 100644
--- a/src/kernel/caches.h
+++ b/src/kernel/caches.h
@@ -11,6 +11,7 @@
 #include <bit>
 #include <cstdint>
 #include <limits>
+#include <optional>
 
 //! Suggested default amount of cache reserved for the kernel (MiB)
 static constexpr int64_t DEFAULT_KERNEL_CACHE{450};
@@ -20,11 +21,15 @@ static constexpr int64_t MAX_BLOCK_DB_CACHE{2};
 static constexpr int64_t MAX_COINS_DB_CACHE{8};
 
 //! Guard against truncation of values before converting.
-constexpr size_t MiBToBytes(int64_t mib)
+constexpr std::optional<size_t> MiBToBytes(int64_t mib)
 {
-    Assert(std::countl_zero(static_cast<uint64_t>(mib)) >= 21); // Ensure sign bit is unset + enough zeros to shift.
+    if (mib < 0 || std::countl_zero(static_cast<uint64_t>(mib)) < 21) {
+        return std::nullopt;
+    }
     const int64_t bytes{mib << 20};
-    Assert(static_cast<uint64_t>(bytes) <= std::numeric_limits<size_t>::max());
+    if (static_cast<uint64_t>(bytes) > std::numeric_limits<size_t>::max()) {
+        return std::nullopt;
+    }
     return static_cast<size_t>(bytes);
 }
 
@@ -36,9 +41,9 @@ struct CacheSizes {
 
     CacheSizes(size_t total_cache)
     {
-        block_tree_db = std::min(total_cache / 8, MiBToBytes(MAX_BLOCK_DB_CACHE));
+        block_tree_db = std::min(total_cache / 8, *Assert(MiBToBytes(MAX_BLOCK_DB_CACHE)));
         total_cache -= block_tree_db;
-        coins_db = std::min(total_cache / 2, MiBToBytes(MAX_COINS_DB_CACHE));
+        coins_db = std::min(total_cache / 2, *Assert(MiBToBytes(MAX_COINS_DB_CACHE)));
         total_cache -= coins_db;
         coins = total_cache; // the rest goes to the coins cache
     }
diff --git a/src/node/caches.cpp b/src/node/caches.cpp
index 9dbbccd304..f59748a222 100644
--- a/src/node/caches.cpp
+++ b/src/node/caches.cpp
@@ -8,6 +8,7 @@
 #include <index/txindex.h>
 #include <kernel/caches.h>
 #include <logging.h>
+#include <util/check.h>
 
 #include <algorithm>
 #include <optional>
@@ -31,14 +32,14 @@ std::optional<CacheSizes> CalculateCacheSizes(const ArgsManager& args, size_t n_
 
     // negative values are permitted, but interpreted as zero.
     db_cache = std::max(int64_t{0}, db_cache);
-    size_t total_cache = std::max(MiBToBytes(db_cache), MiBToBytes(MIN_DB_CACHE));
+    size_t total_cache = std::max(*Assert(MiBToBytes(db_cache)), *Assert(MiBToBytes(MIN_DB_CACHE)));
 
     IndexCacheSizes index_sizes;
-    index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MiBToBytes(MAX_TX_INDEX_CACHE) : 0);
+    index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? *Assert(MiBToBytes(MAX_TX_INDEX_CACHE)) : 0);
     total_cache -= index_sizes.tx_index;
     index_sizes.filter_index = 0;
     if (n_indexes > 0) {
-        int64_t max_cache = std::min(total_cache / 8, MiBToBytes(MAX_FILTER_INDEX_CACHE));
+        int64_t max_cache = std::min(total_cache / 8, *Assert(MiBToBytes(MAX_FILTER_INDEX_CACHE)));
         index_sizes.filter_index = max_cache / n_indexes;
         total_cache -= index_sizes.filter_index * n_indexes;
     }
diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt
index ed73e44901..7cf42c2aa9 100644
--- a/src/test/CMakeLists.txt
+++ b/src/test/CMakeLists.txt
@@ -71,6 +71,7 @@ add_executable(test_bitcoin
   httpserver_tests.cpp
   i2p_tests.cpp
   interfaces_tests.cpp
+  kernel_caches_tests.cpp
   key_io_tests.cpp
   key_tests.cpp
   logging_tests.cpp

```
</details>


Code after:
// Copyright (c) 2024-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#ifndef BITCOIN_KERNEL_CACHES_H
#define BITCOIN_KERNEL_CACHES_H

#include <util/byte_units.h>

#include <algorithm>

//! Suggested default amount of cache reserved for the kernel (bytes)
static constexpr size_t DEFAULT_KERNEL_CACHE{450_MiB};
//! Max memory allocated to block tree DB specific cache (bytes)
static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
//! Max memory allocated to coin DB specific cache (bytes)
static constexpr size_t MAX_COINS_DB_CACHE{8_MiB};

namespace kernel {
struct CacheSizes {
    size_t block_tree_db;
    size_t coins_db;
    size_t coins;

    CacheSizes(size_t total_cache)
    {
        block_tree_db = std::min(total_cache / 8, MAX_BLOCK_DB_CACHE);
        total_cache -= block_tree_db;
        coins_db = std::min(total_cache / 2, MAX_COINS_DB_CACHE);
        total_cache -= coins_db;
        coins = total_cache; // the rest goes to the coins cache
    }
};
} // namespace kernel

#endif // BITCOIN_KERNEL_CACHES_H