From eb783d8d7efafe00e3e8b5da737839110608666f Mon Sep 17 00:00:00 2001 From: stickies-v Date: Thu, 6 Feb 2025 21:35:21 +0000 Subject: [PATCH] kernel: disallow invalid wiping option combination Instead of failing the chainstate_manager_create call, disallow setting the set_wipe_block_tree_db and set_wipe_chainstate_db options to invalid combinations. This improves user error feedback, removing the need to inspect the logs when creating a ChainstateManager fails. --- src/kernel/bitcoinkernel.cpp | 16 ++++++++++++++-- src/kernel/bitcoinkernel.h | 13 +++++++++++-- src/kernel/bitcoinkernel_wrapper.h | 8 ++++---- src/test/kernel/test_kernel.cpp | 7 ++++--- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index 20061d8c6f08f..49f92da4c2342 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -769,12 +769,18 @@ kernel_ChainstateManagerOptions* kernel_chainstate_manager_options_create( } } -void kernel_chainstate_manager_options_set_wipe_chainstate_db( +bool kernel_chainstate_manager_options_set_wipe_chainstate_db( kernel_ChainstateManagerOptions* chainstate_manager_opts_, bool wipe_chainstate_db) { auto chainstate_load_opts{cast_chainstate_load_options(chainstate_manager_opts_)}; + auto block_manager_opts{cast_block_manager_options(chainstate_manager_opts_)}; + if (block_manager_opts->block_tree_db_params.wipe_data && !wipe_chainstate_db) { + LogWarning("Wiping the block tree db without also wiping the chainstate db is currently unsupported."); + return false; + } chainstate_load_opts->wipe_chainstate_db = wipe_chainstate_db; + return true; } void kernel_chainstate_manager_options_set_chainstate_db_in_memory( @@ -798,12 +804,18 @@ void kernel_chainstate_manager_options_destroy(kernel_ChainstateManagerOptions* } } -void kernel_chainstate_manager_options_set_wipe_block_tree_db( +bool kernel_chainstate_manager_options_set_wipe_block_tree_db( kernel_ChainstateManagerOptions* chainstate_manager_opts_, bool wipe_block_tree_db) { auto block_manager_options{cast_block_manager_options(chainstate_manager_opts_)}; + auto chainstate_load_opts{cast_chainstate_load_options(chainstate_manager_opts_)}; + if (wipe_block_tree_db && !chainstate_load_opts->wipe_chainstate_db) { + LogWarning("Wiping the block tree db without also wiping the chainstate db is currently unsupported."); + return false; + } block_manager_options->block_tree_db_params.wipe_data = wipe_block_tree_db; + return true; } void kernel_chainstate_manager_options_set_block_tree_db_in_memory( diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h index c299d6dfa859e..81a3e215e4b23 100644 --- a/src/kernel/bitcoinkernel.h +++ b/src/kernel/bitcoinkernel.h @@ -767,11 +767,16 @@ BITCOINKERNEL_API kernel_ChainstateManagerOptions* BITCOINKERNEL_WARN_UNUSED_RES /** * @brief Sets wipe block tree db in the chainstate manager options. + * If wipe_chainstate_db is set to False, this option may not be + * set to True. * * @param[in] chainstate_manager_options Non-null, options to be set. * @param[in] wipe_block_tree_db Set wipe block tree db. + * + * @return True if the set was successful, False if the set failed + * due to wipe_chainstate_db incompatibility. */ -BITCOINKERNEL_API void kernel_chainstate_manager_options_set_wipe_block_tree_db( +BITCOINKERNEL_API bool BITCOINKERNEL_WARN_UNUSED_RESULT kernel_chainstate_manager_options_set_wipe_block_tree_db( kernel_ChainstateManagerOptions* chainstate_manager_options, bool wipe_block_tree_db) BITCOINKERNEL_ARG_NONNULL(1); @@ -788,11 +793,15 @@ BITCOINKERNEL_API void kernel_chainstate_manager_options_set_block_tree_db_in_me /** * @brief Sets wipe chainstate db in the chainstate manager options. + * If wipe_block_tree_db is set to True, this must be set to True. * * @param[in] chainstate_manager_options Non-null, options to be set. * @param[in] wipe_chainstate_db Set wipe chainstate db. + * + * @return True if the set was successful, False if the set failed + * due to wipe_block_tree_db incompatibility. */ -BITCOINKERNEL_API void kernel_chainstate_manager_options_set_wipe_chainstate_db( +BITCOINKERNEL_API bool BITCOINKERNEL_WARN_UNUSED_RESULT kernel_chainstate_manager_options_set_wipe_chainstate_db( kernel_ChainstateManagerOptions* chainstate_manager_options, bool wipe_chainstate_db) BITCOINKERNEL_ARG_NONNULL(1); diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h index db843689fa794..eb7ded86be856 100644 --- a/src/kernel/bitcoinkernel_wrapper.h +++ b/src/kernel/bitcoinkernel_wrapper.h @@ -401,9 +401,9 @@ class ChainstateManagerOptions { } - void SetWipeBlockTreeDb(bool wipe_block_tree) const noexcept + bool SetWipeBlockTreeDb(bool wipe_block_tree) const noexcept { - kernel_chainstate_manager_options_set_wipe_block_tree_db(m_options.get(), wipe_block_tree); + return kernel_chainstate_manager_options_set_wipe_block_tree_db(m_options.get(), wipe_block_tree); } void SetBlockTreeDbInMemory(bool block_tree_db_in_memory) const noexcept @@ -411,9 +411,9 @@ class ChainstateManagerOptions kernel_chainstate_manager_options_set_block_tree_db_in_memory(m_options.get(), block_tree_db_in_memory); } - void SetWipeChainstateDb(bool wipe_chainstate) const noexcept + bool SetWipeChainstateDb(bool wipe_chainstate) const noexcept { - kernel_chainstate_manager_options_set_wipe_chainstate_db(m_options.get(), wipe_chainstate); + return kernel_chainstate_manager_options_set_wipe_chainstate_db(m_options.get(), wipe_chainstate); } void SetChainstateDbInMemory(bool chainstate_db_in_memory) const noexcept diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp index c1e9f429771c7..8569b5a1450a8 100644 --- a/src/test/kernel/test_kernel.cpp +++ b/src/test/kernel/test_kernel.cpp @@ -375,6 +375,7 @@ void chainman_test() ChainstateManagerOptions chainman_opts{context, test_directory.m_directory.string(), (test_directory.m_directory / "blocks").string()}; assert(chainman_opts); + assert(!chainman_opts.SetWipeBlockTreeDb(true)); // Can't set wipe_block_tree_db to true when wipe_chainstate_db is false chainman_opts.SetWorkerThreads(4); ChainMan chainman{context, chainman_opts}; @@ -392,11 +393,11 @@ std::unique_ptr create_chainman(TestDirectory& test_directory, assert(chainman_opts); if (reindex) { - chainman_opts.SetWipeBlockTreeDb(reindex); - chainman_opts.SetWipeChainstateDb(reindex); + assert(chainman_opts.SetWipeChainstateDb(reindex)); + assert(chainman_opts.SetWipeBlockTreeDb(reindex)); } if (wipe_chainstate) { - chainman_opts.SetWipeChainstateDb(wipe_chainstate); + assert(chainman_opts.SetWipeChainstateDb(wipe_chainstate)); } if (block_tree_db_in_memory) { chainman_opts.SetBlockTreeDbInMemory(block_tree_db_in_memory);