Merge bitcoin/bitcoin#33515: Improve LastCommonAncestor performance + add tests

3635d62 chain: make use of pskip in LastCommonAncestor (optimization) (Pieter Wuille)
2e09d66 tests: add unit tests for CBlockIndex::GetAncestor and LastCommonAncestor (Pieter Wuille)

Pull request description:

  In theory, the `LastCommonAncestor` function in chain.cpp can take $\mathcal{O}(n)$ time, walking over the entire chain, if the forking point is very early, which could take ~milliseconds. I expect this to be very rare in normal occurrences, but it seems nontrivial to reason about worst cases as it's accessible from several places in net_processing.

  This PR modifies the algorithm to make use of the `CBlockIndex::pskip` skip pointers to find the forking point in sublinear time (a simulation shows that for heights up to $34 \cdot 4^k - 2$ and $k \geq 8$, no more than $k^2 + 10k + 13$ steps are ever needed), in a way that should be nearly free - at worst the same number of memory accesses should be made, with a tiny increase in computation.

  As it appears we didn't really have tests for this function, unit tests are added for that function as well as `CBlockIndex::GetAncestor()`.

  This is inspired by bitcoin/bitcoin#32180 (comment)

ACKs for top commit:
  optout21:
    ACK 3635d62
  achow101:
    ACK 3635d62
  vasild:
    ACK 3635d62
  mzumsande:
    Code Review ACK 3635d62
  furszy:
    ACK 3635d62
  stratospher:
    ACK 3635d62.

Tree-SHA512: f9b7dea1e34c1cc1ec1da3fb9e90c4acbf4aaf0f04768844f538201efa6b11eeeefc97b720509e78c21878977192e2c4031fd8974151667e2e756247002b8164

Author: achow101

src/chain.cpp

@@ -5,6 +5,7 @@
 
 #include <chain.h>
 #include <tinyformat.h>
+#include <util/check.h>
 #include <util/time.h>
 
 std::string CBlockFileInfo::ToString() const
@@ -158,18 +159,26 @@ int64_t GetBlockProofEquivalentTime(const CBlockIndex& to, const CBlockIndex& fr
 /** Find the last common ancestor two blocks have.
  *  Both pa and pb must be non-nullptr. */
 const CBlockIndex* LastCommonAncestor(const CBlockIndex* pa, const CBlockIndex* pb) {
+    // First rewind to the last common height (the forking point cannot be past one of the two).
     if (pa->nHeight > pb->nHeight) {
         pa = pa->GetAncestor(pb->nHeight);
     } else if (pb->nHeight > pa->nHeight) {
         pb = pb->GetAncestor(pa->nHeight);
     }
-
-    while (pa != pb && pa && pb) {
+    while (pa != pb) {
+        // Jump back until pa and pb have a common "skip" ancestor.
+        while (pa->pskip != pb->pskip) {
+            // This logic relies on the property that equal-height blocks have equal-height skip
+            // pointers.
+            Assume(pa->nHeight == pb->nHeight);
+            Assume(pa->pskip->nHeight == pb->pskip->nHeight);
+            pa = pa->pskip;
+            pb = pb->pskip;
+        }
+        // At this point, pa and pb are different, but have equal pskip. The forking point lies in
+        // between pa/pb on the one end, and pa->pskip/pb->pskip on the other end.
         pa = pa->pprev;
         pb = pb->pprev;
     }
-
-    // Eventually all chain branches meet at the genesis block.
-    assert(pa == pb);
     return pa;
 }

src/test/CMakeLists.txt

@@ -26,6 +26,7 @@ add_executable(test_bitcoin
   bloom_tests.cpp
   bswap_tests.cpp
   caches_tests.cpp
+  chain_tests.cpp
   chainstate_write_tests.cpp
   checkqueue_tests.cpp
   cluster_linearize_tests.cpp

src/test/chain_tests.cpp

@@ -0,0 +1,85 @@
+// Copyright (c) The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <boost/test/unit_test.hpp>
+
+#include <chain.h>
+#include <test/util/setup_common.h>
+
+#include <memory>
+
+BOOST_FIXTURE_TEST_SUITE(chain_tests, BasicTestingSetup)
+
+namespace {
+
+const CBlockIndex* NaiveGetAncestor(const CBlockIndex* a, int height)
+{
+    while (a->nHeight > height) {
+        a = a->pprev;
+    }
+    BOOST_REQUIRE_EQUAL(a->nHeight, height);
+    return a;
+}
+
+const CBlockIndex* NaiveLastCommonAncestor(const CBlockIndex* a, const CBlockIndex* b)
+{
+    while (a->nHeight > b->nHeight) {
+        a = a->pprev;
+    }
+    while (b->nHeight > a->nHeight) {
+        b = b->pprev;
+    }
+    while (a != b) {
+        BOOST_REQUIRE_EQUAL(a->nHeight, b->nHeight);
+        a = a->pprev;
+        b = b->pprev;
+    }
+    BOOST_REQUIRE_EQUAL(a, b);
+    return a;
+}
+
+} // namespace
+
+BOOST_AUTO_TEST_CASE(chain_test)
+{
+    FastRandomContext ctx;
+    std::vector<std::unique_ptr<CBlockIndex>> block_index;
+    // Run 10 iterations of the whole test.
+    for (int i = 0; i < 10; ++i) {
+        block_index.clear();
+        // Create genesis block.
+        auto genesis = std::make_unique<CBlockIndex>();
+        genesis->nHeight = 0;
+        block_index.push_back(std::move(genesis));
+        // Create 10000 more blocks.
+        for (int b = 0; b < 10000; ++b) {
+            auto new_index = std::make_unique<CBlockIndex>();
+            // 95% of blocks build on top of the last block; the others fork off randomly.
+            if (ctx.randrange(20) != 0) {
+                new_index->pprev = block_index.back().get();
+            } else {
+                new_index->pprev = block_index[ctx.randrange(block_index.size())].get();
+            }
+            new_index->nHeight = new_index->pprev->nHeight + 1;
+            new_index->BuildSkip();
+            block_index.push_back(std::move(new_index));
+        }
+        // Run 10000 random GetAncestor queries.
+        for (int q = 0; q < 10000; ++q) {
+            const CBlockIndex* block = block_index[ctx.randrange(block_index.size())].get();
+            unsigned height = ctx.randrange<unsigned>(block->nHeight + 1);
+            const CBlockIndex* result = block->GetAncestor(height);
+            BOOST_CHECK(result == NaiveGetAncestor(block, height));
+        }
+        // Run 10000 random LastCommonAncestor queries.
+        for (int q = 0; q < 10000; ++q) {
+            const CBlockIndex* block1 = block_index[ctx.randrange(block_index.size())].get();
+            const CBlockIndex* block2 = block_index[ctx.randrange(block_index.size())].get();
+            const CBlockIndex* result = LastCommonAncestor(block1, block2);
+            BOOST_CHECK(result == NaiveLastCommonAncestor(block1, block2));
+        }
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()