From 8fd1c2893e6768223069d8b2fdec033b026cb2eb Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Wed, 3 Sep 2025 23:44:59 +0200
Subject: [PATCH] test(headerssync): Test returning of pow_validated_headers
 behavior

Adding these checks necessitates increasing the length of the generated test chains so that we can properly exceed the REDOWNLOAD_BUFFER_SIZE during the test.

One can check out this commit and locally revert the TARGET_BLOCKS value change to prove the need for tests being able to control the buffer size, as is done by the next commit. Beyond the current REDOWNLOAD_BUFFER_SIZE of 15'009 we need 3 extra - 15'012 TARGET_BLOCKS:
* 1 for the genesis block.
* 1 for the test wanting to check that we start receiving headers for permanent storage *before* the final header (first_chain.back()).
* 1 to exceed REDOWNLOAD_BUFFER_SIZE in HeadersSyncState::PopHeadersReadyForAcceptance().

(The release process includes an occasional increase of the REDOWNLOAD_BUFFER_SIZE value, see release-process.md and history of headerssync.cpp).
---
 src/test/headers_sync_chainwork_tests.cpp | 27 +++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp
index 40a85f04de5..4496ed5fa27 100644
--- a/src/test/headers_sync_chainwork_tests.cpp
+++ b/src/test/headers_sync_chainwork_tests.cpp
@@ -40,9 +40,12 @@ using State = HeadersSyncState::State;
         }                                                                                                \
     } while (false)
 
-constexpr size_t TARGET_BLOCKS{15'000};
+constexpr size_t TARGET_BLOCKS{15'012};
 constexpr arith_uint256 CHAIN_WORK{TARGET_BLOCKS * 2};
 
+// Copied from headerssync.cpp, will be redefined in next commit.
+constexpr size_t REDOWNLOAD_BUFFER_SIZE{15'009};
+
 struct HeadersGeneratorSetup : public RegTestingSetup {
     const CBlock& genesis{Params().GenesisBlock()};
     const CBlockIndex* chain_start{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(genesis.GetHash()))};
@@ -180,12 +183,28 @@ BOOST_AUTO_TEST_CASE(happy_path)
             /*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
             /*exp_locator_hash=*/genesis_hash);
 
-        CHECK_RESULT(hss.ProcessNextHeaders(first_chain, full_headers_message),
-            // Nothing left for the sync logic to do:
+        // Process only so that the internal threshold isn't exceeded, meaning
+        // validated headers shouldn't be returned yet:
+        CHECK_RESULT(hss.ProcessNextHeaders({first_chain.begin(), REDOWNLOAD_BUFFER_SIZE}, true),
+            hss, /*exp_state=*/State::REDOWNLOAD,
+            /*exp_success*/true, /*exp_request_more=*/true,
+            /*exp_headers_size=*/0, /*exp_pow_validated_prev=*/std::nullopt,
+            /*exp_locator_hash=*/first_chain[REDOWNLOAD_BUFFER_SIZE - 1].GetHash());
+
+        // We start receiving headers for permanent storage before completing:
+        CHECK_RESULT(hss.ProcessNextHeaders({{first_chain[REDOWNLOAD_BUFFER_SIZE]}}, true),
+            hss, /*exp_state=*/State::REDOWNLOAD,
+            /*exp_success*/true, /*exp_request_more=*/true,
+            /*exp_headers_size=*/1, /*exp_pow_validated_prev=*/genesis_hash,
+            /*exp_locator_hash=*/first_chain[REDOWNLOAD_BUFFER_SIZE].GetHash());
+
+        // Feed in remaining headers, meeting the work threshold again and
+        // completing the REDOWNLOAD phase:
+        CHECK_RESULT(hss.ProcessNextHeaders({first_chain.begin() + REDOWNLOAD_BUFFER_SIZE + 1, first_chain.end()}, full_headers_message),
             hss, /*exp_state=*/State::FINAL,
             /*exp_success*/true, /*exp_request_more=*/false,
             // All headers except the one already returned above:
-            /*exp_headers_size=*/first_chain.size(), /*exp_pow_validated_prev=*/genesis_hash,
+            /*exp_headers_size=*/first_chain.size() - 1, /*exp_pow_validated_prev=*/first_chain.front().GetHash(),
             /*exp_locator_hash=*/std::nullopt);
     }
 }