refactor: Small style and test fixups for bitcoinkernel #34488

pull maflcko wants to merge 3 commits into bitcoin:master from maflcko:2602-kernel-style-fixups changing 3 files +34 −21
  1. maflcko commented at 7:53 pm on February 2, 2026: member
    Just some small style and test fixups after #30595#pullrequestreview-3420542946
  2. DrahtBot renamed this:
    refactor: Small style and test fixups for bitcoinkernel
    refactor: Small style and test fixups for bitcoinkernel
    on Feb 2, 2026
  3. DrahtBot added the label Refactoring on Feb 2, 2026
  4. DrahtBot commented at 7:54 pm on February 2, 2026: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK stickies-v, frankomosh

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34401 (kernel: add serialization method for btck_BlockHeader API by yuvicc)
    • #32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by sedited)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    LLM Linter (✨ experimental)

    Possible places where comparison-specific test macros should replace generic comparisons:

    • src/test/kernel/test_kernel.cpp: BOOST_CHECK_THROW(range.at(expected_size), std::out_of_range); -> Replace BOOST_CHECK_THROW with BOOST_CHECK_EXCEPTION to also verify the exception message (e.g. BOOST_CHECK_EXCEPTION(range.at(expected_size), std::out_of_range, HasReason(“index out of range”)))

    2026-02-03 19:28:00

  5. maflcko force-pushed on Feb 2, 2026
  6. DrahtBot added the label CI failed on Feb 2, 2026
  7. maflcko closed this on Feb 3, 2026
  8. maflcko reopened this on Feb 3, 2026
  9. maflcko force-pushed on Feb 3, 2026
  10. DrahtBot removed the label CI failed on Feb 3, 2026
  11. in src/kernel/bitcoinkernel.h:85 in fa257cca2c 
    81@@ -82,9 +82,9 @@ extern "C" {
82  * @section error Error handling
83  *
84  * Functions communicate an error through their return types, usually returning
85- * a nullptr, 0, or false if an error is encountered. Additionally, verification
86- * functions, e.g. for scripts, may communicate more detailed error information
87- * through status code out parameters.
88+ * a nullptr, or a status code as documented by the returning function.
    stickies-v commented at 12:16 pm on February 3, 2026:

    nit

    0 * a nullptr or a status code as documented by the returning function.
  12. in src/test/kernel/test_kernel.cpp:796 in fa257cca2c 
    792@@ -792,7 +793,7 @@ void chainman_reindex_test(TestDirectory& test_directory)
793 {
794     auto notifications{std::make_shared<TestKernelNotifications>()};
795     auto context{create_context(notifications, ChainType::MAINNET)};
796-    auto chainman{create_chainman(test_directory, true, false, false, false, context)};
797+    auto chainman{create_chainman(test_directory, /*reindex=*/true, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
    stickies-v commented at 12:20 pm on February 3, 2026:

    wrap nit:

     0diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
 1index b88800f46e..ae171180bc 100644
 2--- a/src/test/kernel/test_kernel.cpp
 3+++ b/src/test/kernel/test_kernel.cpp
 4@@ -793,7 +793,9 @@ void chainman_reindex_test(TestDirectory& test_directory)
 5 {
 6     auto notifications{std::make_shared<TestKernelNotifications>()};
 7     auto context{create_context(notifications, ChainType::MAINNET)};
 8-    auto chainman{create_chainman(test_directory, /*reindex=*/true, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
 9+    auto chainman{create_chainman(
10+        test_directory, /*reindex=*/true, /*wipe_chainstate=*/false,
11+        /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
12 
13     std::vector<std::string> import_files;
14     BOOST_CHECK(chainman->ImportBlocks(import_files));
15@@ -836,7 +838,9 @@ void chainman_reindex_chainstate_test(TestDirectory& test_directory)
16 {
17     auto notifications{std::make_shared<TestKernelNotifications>()};
18     auto context{create_context(notifications, ChainType::MAINNET)};
19-    auto chainman{create_chainman(test_directory, /*reindex=*/false, /*wipe_chainstate=*/true, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
20+    auto chainman{create_chainman(
21+        test_directory, /*reindex=*/false, /*wipe_chainstate=*/true,
22+        /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
23 
24     std::vector<std::string> import_files;
25     import_files.push_back((test_directory.m_directory / "blocks" / "blk00000.dat").string());
26@@ -848,7 +852,9 @@ void chainman_mainnet_validation_test(TestDirectory& test_directory)
27     auto notifications{std::make_shared<TestKernelNotifications>()};
28     auto validation_interface{std::make_shared<TestValidationInterface>()};
29     auto context{create_context(notifications, ChainType::MAINNET, validation_interface)};
30-    auto chainman{create_chainman(test_directory, /*reindex=*/false, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
31+    auto chainman{create_chainman(
32+        test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
33+        /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
34 
35     // mainnet block 1
36     auto raw_block = hex_string_to_byte_vec("010000006fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000982051fd1e4ba744bbbe680e1fee14677ba1a3c3540bf7b1cdb606e857233e0e61bc6649ffff001d01e362990101000000010000000000000000000000000000000000000000000000000000000000000000ffffffff0704ffff001d0104ffffffff0100f2052a0100000043410496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858eeac00000000");
37@@ -976,7 +982,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_in_memory_tests)
38 
39     auto notifications{std::make_shared<TestKernelNotifications>()};
40     auto context{create_context(notifications, ChainType::REGTEST)};
41-    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)};
42+    auto chainman{create_chainman(
43+        in_memory_test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
44+        /*block_tree_db_in_memory=*/true, /*chainstate_db_in_memory=*/true, context)};
45 
46     for (auto& raw_block : REGTEST_BLOCK_DATA) {
47         Block block{hex_string_to_byte_vec(raw_block)};
48@@ -1000,7 +1008,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
49     auto context{create_context(notifications, ChainType::REGTEST)};
50 
51     {
52-        auto chainman{create_chainman(test_directory, /*reindex=*/false, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
53+        auto chainman{create_chainman(
54+            test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
55+            /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
56         for (const auto& data : REGTEST_BLOCK_DATA) {
57             Block block{hex_string_to_byte_vec(data)};
58             BlockHeader header = block.GetHeader();
59@@ -1022,7 +1032,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
60     const size_t mid{REGTEST_BLOCK_DATA.size() / 2};
61 
62     {
63-        auto chainman{create_chainman(test_directory, /*reindex=*/false, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
64+        auto chainman{create_chainman(
65+            test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
66+            /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
67         for (size_t i{0}; i < mid; i++) {
68             Block block{hex_string_to_byte_vec(REGTEST_BLOCK_DATA[i])};
69             bool new_block{false};
70@@ -1031,7 +1043,9 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
71         }
72     }
73 
74-    auto chainman{create_chainman(test_directory, /*reindex=*/false, /*wipe_chainstate=*/false, /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
75+    auto chainman{create_chainman(
76+        test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
77+        /*block_tree_db_in_memory=*/false, /*chainstate_db_in_memory=*/false, context)};
78 
79     for (size_t i{mid}; i < REGTEST_BLOCK_DATA.size(); i++) {
80         Block block{hex_string_to_byte_vec(REGTEST_BLOCK_DATA[i])};
  13. in src/test/kernel/test_kernel.cpp:324 in fa257cca2c 
    320@@ -321,6 +321,7 @@ void CheckRange(const RangeType& range, size_t expected_size)
321     using value_type = std::ranges::range_value_t<RangeType>;
322 
323     BOOST_CHECK_EQUAL(range.size(), expected_size);
324+    BOOST_REQUIRE(expected_size > 0); // Some checks below assume a non-empty range
    stickies-v commented at 12:35 pm on February 3, 2026:

    The next line now seems redundant, because the range cannot be empty. Also, L336 seems to require at least 2 elements:

    0    BOOST_CHECK_NE(range.at(0).get(), range.at(expected_size - 1).get());

    I’m not sure why a range can’t have the same start and end value, though, so maybe that check should just be removed and we can stick to requiring non-empty ranges?

    maflcko commented at 7:28 pm on February 3, 2026:
    thx, I pushed a diff
  14. stickies-v approved
  15. stickies-v commented at 12:36 pm on February 3, 2026: contributor
    ACK fa257cca2c4e70faa954c9761832886d4b002753
  16. refactor: Small style fixups in src/kernel/bitcoinkernel.cpp 
    * Use type alias TranslateFn:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653828562
* Use std::span::data:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653829743
* Use the ref helper:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653829991
* Reword error handling section:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653843805
    fa51594c5c
  17. test: Use clang-tidy named args for create_chainman 
    https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653846863
    fabb58d42d
  18. test: kernel test fixups 
    * Allow byte arrays; Adjust size check, which would otherwise fail,
  because two byte arrays of a type are always of the same size:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2642930435

* Require empty range:
  https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2653846374
    fad9dd1a88
  19. maflcko force-pushed on Feb 3, 2026
  20. stickies-v commented at 8:28 pm on February 3, 2026: contributor
    re-ACK fad9dd1a8891770846f3f98c60bebf2c2bf72e05
  21. frankomosh commented at 8:55 am on February 4, 2026: contributor
    Code Review ACK fad9dd1a8891770846f3f98c60bebf2c2bf72e05. 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).
  22. fanquake merged this on Feb 4, 2026
  23. fanquake closed this on Feb 4, 2026
  24. ?
    added_to_project_v2 fanquake
  25. ?
    project_v2_item_status_changed github-project-automation[bot]
  26. ?
    project_v2_item_status_changed github-project-automation[bot]
  27. maflcko deleted the branch on Feb 4, 2026


maflcko DrahtBot stickies-v frankomosh

Labels
Refactoring

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-02-11 21:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me