Merge bitcoin/bitcoin#34488: refactor: Small style and test fixups for bitcoinkernel

fad9dd1a88 test: kernel test fixups (MarcoFalke)
fabb58d42d test: Use clang-tidy named args for create_chainman (MarcoFalke)
fa51594c5c refactor: Small style fixups in src/kernel/bitcoinkernel.cpp (MarcoFalke)

Pull request description:

  Just some small style and test fixups after https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3420542946

ACKs for top commit:
  stickies-v:
    re-ACK fad9dd1a88
  frankomosh:
    Code Review ACK fad9dd1a88. All changes are sound refactoring with no functional issues. Nice improvements to readability (named args in create_chainman, span.data(), range checks now properly require non-empty).

Tree-SHA512: 0a92e871b4db75a590acad39672594625e402895bc0d36635d36ec2fe8dce7cc2c5cb6ebf2a92bc14617d94648b84bffb95ff783cea71bd91ac4a9871ef5dbef
This commit is contained in:
merge-script
2026-02-04 13:48:21 +00:00
3 changed files with 34 additions and 21 deletions

View File

@@ -41,7 +41,6 @@
#include <cstring>
#include <exception>
#include <functional>
#include <iterator>
#include <list>
#include <memory>
#include <span>
@@ -56,7 +55,7 @@ using util::ImmediateTaskRunner;
// Define G_TRANSLATION_FUN symbol in libbitcoinkernel library so users of the
// library aren't required to export this symbol
extern const std::function<std::string(const char*)> G_TRANSLATION_FUN{nullptr};
extern const TranslateFn G_TRANSLATION_FUN{nullptr};
static const kernel::Context btck_context_static{};
@@ -84,7 +83,7 @@ public:
//
void write(std::span<const std::byte> src)
{
if (m_writer(std::data(src), src.size(), m_user_data) != 0) {
if (m_writer(src.data(), src.size(), m_user_data) != 0) {
throw std::runtime_error("Failed to write serialization data");
}
}
@@ -113,13 +112,13 @@ struct Handle {
static C* create(Args&&... args)
{
auto cpp_obj{std::make_unique<CPP>(std::forward<Args>(args)...)};
return reinterpret_cast<C*>(cpp_obj.release());
return ref(cpp_obj.release());
}
static C* copy(const C* ptr)
{
auto cpp_obj{std::make_unique<CPP>(get(ptr))};
return reinterpret_cast<C*>(cpp_obj.release());
return ref(cpp_obj.release());
}
static const CPP& get(const C* ptr)

View File

@@ -82,9 +82,9 @@ extern "C" {
* @section error Error handling
*
* Functions communicate an error through their return types, usually returning
* a nullptr, 0, or false if an error is encountered. Additionally, verification
* functions, e.g. for scripts, may communicate more detailed error information
* through status code out parameters.
* a nullptr or a status code as documented by the returning function.
* Additionally, verification functions, e.g. for scripts, may communicate more
* detailed error information through status code out parameters.
*
* Fine-grained validation information is communicated through the validation
* interface.

View File

@@ -265,9 +265,7 @@ void run_verify_test(
}
template <typename T>
concept HasToBytes = requires(T t) {
{ t.ToBytes() } -> std::convertible_to<std::vector<std::byte>>;
};
concept HasToBytes = requires(T t) { t.ToBytes(); };
template <typename T>
void CheckHandle(T object, T distinct_object)
@@ -277,7 +275,9 @@ void CheckHandle(T object, T distinct_object)
BOOST_CHECK(object.get() != distinct_object.get());
if constexpr (HasToBytes<T>) {
BOOST_CHECK_NE(object.ToBytes().size(), distinct_object.ToBytes().size());
const auto object_bytes = object.ToBytes();
const auto distinct_bytes = distinct_object.ToBytes();
BOOST_CHECK(!std::ranges::equal(object_bytes, distinct_bytes));
}
// Copy constructor
@@ -321,7 +321,8 @@ void CheckRange(const RangeType& range, size_t expected_size)
using value_type = std::ranges::range_value_t<RangeType>;
BOOST_CHECK_EQUAL(range.size(), expected_size);
BOOST_CHECK_EQUAL(range.empty(), (expected_size == 0));
BOOST_REQUIRE(range.size() > 0); // Some checks below assume a non-empty range
BOOST_REQUIRE(!range.empty());
BOOST_CHECK(range.begin() != range.end());
BOOST_CHECK_EQUAL(std::distance(range.begin(), range.end()), static_cast<std::ptrdiff_t>(expected_size));
@@ -332,7 +333,6 @@ void CheckRange(const RangeType& range, size_t expected_size)
BOOST_CHECK_EQUAL(range[i].get(), (*(range.begin() + i)).get());
}
BOOST_CHECK_NE(range.at(0).get(), range.at(expected_size - 1).get());
BOOST_CHECK_THROW(range.at(expected_size), std::out_of_range);
BOOST_CHECK_EQUAL(range.front().get(), range[0].get());
@@ -792,7 +792,9 @@ void chainman_reindex_test(TestDirectory& test_directory)
{
auto notifications{std::make_shared<TestKernelNotifications>()};
auto context{create_context(notifications, ChainType::MAINNET)};
auto chainman{create_chainman(test_directory, true, false, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/true, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
std::vector<std::string> import_files;
BOOST_CHECK(chainman->ImportBlocks(import_files));
@@ -835,7 +837,9 @@ void chainman_reindex_chainstate_test(TestDirectory& test_directory)
{
auto notifications{std::make_shared<TestKernelNotifications>()};
auto context{create_context(notifications, ChainType::MAINNET)};
auto chainman{create_chainman(test_directory, false, true, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/false, /*wipe_chainstate=*/true,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
std::vector<std::string> import_files;
import_files.push_back((test_directory.m_directory / "blocks" / "blk00000.dat").string());
@@ -847,7 +851,9 @@ void chainman_mainnet_validation_test(TestDirectory& test_directory)
auto notifications{std::make_shared<TestKernelNotifications>()};
auto validation_interface{std::make_shared<TestValidationInterface>()};
auto context{create_context(notifications, ChainType::MAINNET, validation_interface)};
auto chainman{create_chainman(test_directory, false, false, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
// mainnet block 1
auto raw_block = hex_string_to_byte_vec("010000006fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000982051fd1e4ba744bbbe680e1fee14677ba1a3c3540bf7b1cdb606e857233e0e61bc6649ffff001d01e362990101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0704ffff001d0104ffffffff0100f2052a0100000043410496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858eeac00000000");
@@ -975,7 +981,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_in_memory_tests)
auto notifications{std::make_shared<TestKernelNotifications>()};
auto context{create_context(notifications, ChainType::REGTEST)};
auto chainman{create_chainman(in_memory_test_directory, false, false, true, true, context)};
auto chainman{create_chainman(
in_memory_test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/true, /*chainstate_db_in_memory=*/true, context)};
for (auto& raw_block : REGTEST_BLOCK_DATA) {
Block block{hex_string_to_byte_vec(raw_block)};
@@ -999,7 +1007,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
auto context{create_context(notifications, ChainType::REGTEST)};
{
auto chainman{create_chainman(test_directory, false, false, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
for (const auto& data : REGTEST_BLOCK_DATA) {
Block block{hex_string_to_byte_vec(data)};
BlockHeader header = block.GetHeader();
@@ -1021,7 +1031,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
const size_t mid{REGTEST_BLOCK_DATA.size() / 2};
{
auto chainman{create_chainman(test_directory, false, false, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
for (size_t i{0}; i < mid; i++) {
Block block{hex_string_to_byte_vec(REGTEST_BLOCK_DATA[i])};
bool new_block{false};
@@ -1030,7 +1042,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
}
}
auto chainman{create_chainman(test_directory, false, false, false, false, context)};
auto chainman{create_chainman(
test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
/*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
for (size_t i{mid}; i < REGTEST_BLOCK_DATA.size(); i++) {
Block block{hex_string_to_byte_vec(REGTEST_BLOCK_DATA[i])};