btck_chainstate_manager_options_set_database_cache_bytes() accepts 0 and 1, but these produce an empty coins or coins-DB cache and later abort in node/chainstate.cpp. Should the setter reject these values before mutating the options?
Suggestion:
<details>
<summary>diff</summary>
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index a5c163502c..da6f33fed0 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -1032,12 +1032,19 @@ void btck_chainstate_manager_options_set_worker_threads_num(btck_ChainstateManag
btck_ChainstateManagerOptions::get(opts).m_chainman_options.worker_threads_num = worker_threads;
}
-void btck_chainstate_manager_options_set_database_cache_bytes(btck_ChainstateManagerOptions* chainman_opts, size_t database_cache_bytes)
+int btck_chainstate_manager_options_set_database_cache_bytes(btck_ChainstateManagerOptions* chainman_opts, size_t database_cache_bytes)
{
+ const kernel::CacheSizes cache_sizes{database_cache_bytes};
+ if (cache_sizes.coins_db == 0 || cache_sizes.coins == 0) {
+ LogError("Failed to set database cache: size must be at least 2 bytes.");
+ return -1;
+ }
+
auto& opts{btck_ChainstateManagerOptions::get(chainman_opts)};
LOCK(opts.m_mutex);
opts.m_db_cache_bytes = database_cache_bytes;
- opts.m_blockman_options.block_tree_db_params.cache_bytes = kernel::CacheSizes{database_cache_bytes}.block_tree_db;
+ opts.m_blockman_options.block_tree_db_params.cache_bytes = cache_sizes.block_tree_db;
+ return 0;
}
void btck_chainstate_manager_options_destroy(btck_ChainstateManagerOptions* options)
diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
index 73475e47e4..d0219ece6f 100644
--- a/src/kernel/bitcoinkernel.h
+++ b/src/kernel/bitcoinkernel.h
@@ -1203,9 +1203,10 @@ BITCOINKERNEL_API void btck_chainstate_manager_options_set_worker_threads_num(
* called, the total cache defaults to 450 MiB.
*
* [@param](/bitcoin-bitcoin/contributor/param/)[in] chainstate_manager_options Non-null, options to be set.
- * [@param](/bitcoin-bitcoin/contributor/param/)[in] database_cache_bytes The total database cache size in bytes.
+ * [@param](/bitcoin-bitcoin/contributor/param/)[in] database_cache_bytes The total database cache size in bytes. Values below 2 are rejected.
+ * [@return](/bitcoin-bitcoin/contributor/return/) 0 if the set was successful, non-zero if the set failed.
*/
-BITCOINKERNEL_API void btck_chainstate_manager_options_set_database_cache_bytes(
+BITCOINKERNEL_API int BITCOINKERNEL_WARN_UNUSED_RESULT btck_chainstate_manager_options_set_database_cache_bytes(
btck_ChainstateManagerOptions* chainstate_manager_options,
size_t database_cache_bytes) BITCOINKERNEL_ARG_NONNULL(1);
diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
index 42d19e4a22..9783c3aba1 100644
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -1200,9 +1200,9 @@ public:
btck_chainstate_manager_options_set_worker_threads_num(get(), worker_threads);
}
- void SetDatabaseCacheBytes(size_t database_cache_bytes)
+ bool SetDatabaseCacheBytes(size_t database_cache_bytes)
{
- btck_chainstate_manager_options_set_database_cache_bytes(get(), database_cache_bytes);
+ return btck_chainstate_manager_options_set_database_cache_bytes(get(), database_cache_bytes) == 0;
}
bool SetWipeDbs(bool wipe_block_tree, bool wipe_chainstate)
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 2ac48b6746..c72c8a995a 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -801,7 +801,10 @@ BOOST_AUTO_TEST_CASE(btck_chainman_tests)
ChainstateManagerOptions chainman_opts{context, PathToString(test_directory.m_directory), PathToString(test_directory.m_directory / "blocks")};
chainman_opts.SetWorkerThreads(4);
- chainman_opts.SetDatabaseCacheBytes(1_GiB);
+ BOOST_CHECK(chainman_opts.SetDatabaseCacheBytes(2));
+ BOOST_CHECK(chainman_opts.SetDatabaseCacheBytes(1_GiB));
+ BOOST_CHECK(!chainman_opts.SetDatabaseCacheBytes(0));
+ BOOST_CHECK(!chainman_opts.SetDatabaseCacheBytes(1));
BOOST_CHECK(!chainman_opts.SetWipeDbs(/*wipe_block_tree=*/true, /*wipe_chainstate=*/false));
BOOST_CHECK(chainman_opts.SetWipeDbs(/*wipe_block_tree=*/true, /*wipe_chainstate=*/true));
BOOST_CHECK(chainman_opts.SetWipeDbs(/*wipe_block_tree=*/false, /*wipe_chainstate=*/true));
</details>
