kernel: expose btck_block_tree_entry_get

pzafonte commented at 7:25 pm on March 20, 2026: none

Callers building a block locator from a stale tip currently have to walk back one entry at a time using btck_block_tree_entry_get_previous. For large step sizes this means hundreds of C API calls per locator entry.

This exposes GetAncestor via btck_block_tree_entry_get_ancestor, which uses the existing CBlockIndex skiplist to reach any ancestor height in O(log N) and works on any chain, not just the active chain.

Includes a C++ convenience wrapper and tests ~~covering same-height, genesis, and out-of-range cases~~ in btck_block_tree_entry_tests.

See previous discussion here #34843

DrahtBot added the label Validation on Mar 20, 2026

DrahtBot commented at 7:25 pm on March 20, 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
Concept ACK	stringintech
Approach ACK	sedited, stickies-v

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

Conflicts

No conflicts as of last run.

sedited commented at 1:18 pm on March 23, 2026: contributor

Approach ACK

in src/kernel/bitcoinkernel.h:1017 in 68273fc963

1012+ * @brief Return the ancestor of a block tree entry at the given height.
1013+ *
1014+ * @param[in] block_tree_entry Non-null.
1015+ * @param[in] height           The height of the requested ancestor. Must be between 0 and the
1016+ *                             height of block_tree_entry, inclusive.
1017+ * @return                     The ancestor at the given height, or null if height is out of range.

stickies-v commented at 3:15 pm on March 23, 2026:

It seems like passing a height < 0 or > block_tree_entry’s height is a programming error, which in this interface we typically handle with assert rather than making it part of the interface (i.e. with nullptr). I’m aware btck_chain_get_by_height does the same thing, but I’m wondering if we should update both? That would change the wrapper signature to (a throwing) BlockTreeEntry BlockTreeEntry::GetAncestor(int32_t height) @sedited as the author of btck_chain_get_by_height, is there a reason we shouldn’t use assert here?

maflcko commented at 3:39 pm on March 23, 2026:

@sedited as the author of btck_chain_get_by_height, is there a reason we shouldn’t use assert here?

I’d presume that the caller can not ensure that height really does still exist when the call is done (there could be a massive, but unlikely re-org on the main chain?) Whereas in this case here, the chain is fixed to be the one referred to be by the passed block_tree_entry, which has a constant and known height?

sedited commented at 4:22 pm on March 23, 2026:

What @maflcko said :)

stickies-v commented at 4:36 pm on March 23, 2026:

Oh yes of course, btck_Chain is the active chain, I forgot about that. So in that case I agree we should leave btck_chain_get_by_height as-is, but update btck_block_tree_entry_get_ancestor to have the correct height as a precondition.

pzafonte commented at 5:18 pm on March 24, 2026:

Addressed in b10b85f

stickies-v commented at 3:16 pm on March 23, 2026: contributor

Approach ACK

pzafonte force-pushed on Mar 24, 2026

in src/kernel/bitcoinkernel.cpp:893 in b10b85f267

884@@ -895,6 +885,13 @@ const btck_BlockTreeEntry* btck_block_tree_entry_get_previous(const btck_BlockTr
885     return btck_BlockTreeEntry::ref(btck_BlockTreeEntry::get(entry).pprev);
886 }
887 
888+const btck_BlockTreeEntry* btck_block_tree_entry_get_ancestor(const btck_BlockTreeEntry* block_tree_entry, int32_t height)
889+{
890+    const auto& entry{btck_BlockTreeEntry::get(block_tree_entry)};
891+    assert(height >= 0 && height <= entry.nHeight);
892+    return btck_BlockTreeEntry::ref(entry.GetAncestor(height));
893+}

stringintech commented at 5:00 pm on March 30, 2026:

Since we have the same check in GetAncestor, maybe we can avoid duplication and have this assertion instead?

0const btck_BlockTreeEntry* btck_block_tree_entry_get_ancestor(const btck_BlockTreeEntry* block_tree_entry, int height)
1{
2    const auto* ancestor{btck_BlockTreeEntry::get(block_tree_entry).GetAncestor(height)};
3    assert(ancestor);
4    return btck_BlockTreeEntry::ref(ancestor);
5}

pzafonte commented at 9:10 pm on March 30, 2026:

Agreed. Done.

in src/kernel/bitcoinkernel.h:1019 in b10b85f267 outdated

1014+ * @param[in] block_tree_entry Non-null.
1015+ * @param[in] height           The height of the requested ancestor.
1016+ * @return                     The ancestor at the given height.
1017+ * @pre height >= 0 && height <= btck_block_tree_entry_get_height(block_tree_entry)
1018+ */
1019+BITCOINKERNEL_API const btck_BlockTreeEntry* BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_tree_entry_get_ancestor(

stringintech commented at 5:06 pm on March 30, 2026:

This is the first time we are using the @pre tag in the documentation while there are other functions that might hit an assertion on out-of-bounds input. Not sure but maybe it is better to drop it to stay consistent with the rest of the documentation.

pzafonte commented at 9:11 pm on March 30, 2026:

Done.

in src/kernel/bitcoinkernel_wrapper.h:891 in b10b85f267

886+    std::optional<BlockTreeEntry> GetAncestor(int32_t height) const
887+    {
888+        auto entry{btck_block_tree_entry_get_ancestor(get(), height)};
889+        if (!entry) return std::nullopt;
890+        return entry;
891+    }

stringintech commented at 5:13 pm on March 30, 2026:

Since for an out-of-bounds height we would hit an assertion in btck_block_tree_entry_get_ancestor, we wouldn’t have a nullptr entry to return nulloptfor; so we could simply return a BlockTreeEntry?

0    BlockTreeEntry GetAncestor(int height) const
1    {
2        return BlockTreeEntry{btck_block_tree_entry_get_ancestor(get(), height)};
3    }

pzafonte commented at 9:18 pm on March 30, 2026:

Done.

in src/test/kernel/test_kernel.cpp:1181 in b10b85f267

1176@@ -1180,3 +1177,34 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
1177     fs::remove_all(test_directory.m_directory / "blocks" / "rev00000.dat");
1178     BOOST_CHECK_THROW(chainman->ReadBlockSpentOutputs(tip), std::runtime_error);
1179 }
1180+
1181+BOOST_AUTO_TEST_CASE(btck_block_tree_entry_ancestor_tests)

stringintech commented at 5:36 pm on March 30, 2026:

I think the new tests belong to the existing test case btck_block_tree_entry_tests:

 0diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
 1index 49c7f4a6e4..5399fef761 100644
 2--- a/src/test/kernel/test_kernel.cpp
 3+++ b/src/test/kernel/test_kernel.cpp
 4@@ -973,6 +973,11 @@ BOOST_AUTO_TEST_CASE(btck_block_tree_entry_tests)
 5     auto prev{entry_1.GetPrevious()};
 6     BOOST_CHECK(prev.has_value());
 7     BOOST_CHECK(prev.value() == entry_0);
 8+
 9+    // Test GetAncestor
10+    BOOST_CHECK(entry_2.GetAncestor(2) == entry_2);
11+    BOOST_CHECK(entry_2.GetAncestor(1) == entry_1);
12+    BOOST_CHECK(entry_2.GetAncestor(0) == entry_0);
13 }
14 
15 BOOST_AUTO_TEST_CASE(btck_chainman_in_memory_tests)
16@@ -1178,33 +1183,3 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
17     BOOST_CHECK_THROW(chainman->ReadBlockSpentOutputs(tip), std::runtime_error);
18 }
19 
20-BOOST_AUTO_TEST_CASE(btck_block_tree_entry_ancestor_tests)
21-{
22-    auto test_directory{TestDirectory{"ancestor_tests"}};
23-    auto notifications{std::make_shared<TestKernelNotifications>()};
24-    auto context{create_context(notifications, ChainType::REGTEST)};
25-    auto chainman{create_chainman(
26-        test_directory, /*reindex=*/false, /*wipe_chainstate=*/false,
27-        /*block_tree_db_in_memory=*/true, /*chainstate_db_in_memory=*/true,
28-        context)};
29-    for (const auto& raw_block : REGTEST_BLOCK_DATA) {
30-        Block block{hex_string_to_byte_vec(raw_block)};
31-        bool new_block{false};
32-        BOOST_CHECK(chainman->ProcessBlock(block, &new_block));
33-        BOOST_CHECK(new_block);
34-    }
35-
36-    auto tip{chainman->GetBestEntry()};
37-    int32_t tip_height{tip.GetHeight()};
38-
39-    // GetAncestor at the tip's own height returns the tip.
40-    auto same{tip.GetAncestor(tip_height)};
41-    BOOST_REQUIRE(same.has_value());
42-    BOOST_CHECK(same.value() == tip);
43-
44-    // GetAncestor at height 0 returns the genesis block.
45-    auto genesis{tip.GetAncestor(0)};
46-    BOOST_REQUIRE(genesis.has_value());
47-    BOOST_CHECK_EQUAL(genesis->GetHeight(), 0);
48-
49-}

pzafonte commented at 9:19 pm on March 30, 2026:

Done, moved into btck_block_tree_entry_tests. Thanks!

stringintech commented at 5:41 pm on March 30, 2026: contributor

Concept ACK.

kernel: expose btck_block_tree_entry_get_ancestor

Allows callers to jump to any ancestor of a block tree entry by height
using the skiplist-backed GetAncestor, which runs in O(log N) rather
than walking back one entry at a time with btck_block_tree_entry_get_previous.

Includes a C++ convenience wrapper and tests in btck_block_tree_entry_tests.

23074e0f65

pzafonte force-pushed on Mar 30, 2026

kernel: expose btck_block_tree_entry_get_ancestor #34885

Reviews

Conflicts