test(headerssync): Test returning of pow_validated_headers behavior
What the developer wrote
Authored by Hodlinator
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).
What changed, and why it matters
This is a test-only change for Bitcoin Core. It expands an existing unit test to verify how the headers synchronization logic returns proof-of-work-validated headers. The change increases the number of generated test blocks and adds assertions that check internal behavior of the sync state machine. There is no change to production code, no fix for a vulnerability, and no security-relevant behavior change in the software users run.
No action required. Review as normal test improvement. If auditing the related headerssync.cpp logic, consider whether the production boundary behavior is already well covered; this test only increases that coverage.
Security signals we found
No production code modified
Test-only change
Adds boundary-condition assertions for headers sync state machine
No memory safety, cryptographic, or network vulnerability indicators
Evidence from the diff
The commit modifies src/test/headers_sync_chainwork_tests.cpp only. It increases TARGET_BLOCKS from 15,000 to 15,012 and adds a local REDOWNLOAD_BUFFER_SIZE constant so the test can exercise the boundary where HeadersSyncState::PopHeadersReadyForAcceptance() begins returning headers before the final header is received. The test now feeds headers in three stages: (1) up to the buffer size without returning validated headers, (2) one additional header that triggers return of one pow-validated header, and (3) the remainder that completes the REDOWNLOAD phase. This is purely defensive test coverage for existing production logic.
Changed components
src/test/headers_sync_chainwork_tests.cppInspect captured patch +23 / −4
diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp
index 40a85f04..4496ed5f 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);
}
}
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.