Make ProcessNextHeaders use the headers argument as in/out

This allows reusing the same vector storage for input and output headers to
HeadersSync::ProcessNextHeaders. It's also natural in the sense that this argument
just represents the headers-to-be-processed, both in the caller and the callee, and
both before and after the calls.
This commit is contained in:
Pieter Wuille
2022-08-23 17:29:37 -04:00
parent fa5c224d44
commit 4e7ac7b94d
4 changed files with 38 additions and 40 deletions

View File

@@ -65,13 +65,13 @@ void HeadersSyncState::Finalize()
/** Process the next batch of headers received from our peer.
* Validate and store commitments, and compare total chainwork to our target to
* see if we can switch to REDOWNLOAD mode. */
HeadersSyncState::ProcessingResult HeadersSyncState::ProcessNextHeaders(const
std::vector<CBlockHeader>& received_headers, const bool full_headers_message)
HeadersSyncState::ProcessingResult HeadersSyncState::ProcessNextHeaders(
std::vector<CBlockHeader>& headers, const bool full_headers_message)
{
ProcessingResult ret;
Assume(!received_headers.empty());
if (received_headers.empty()) return ret;
Assume(!headers.empty());
if (headers.empty()) return ret;
Assume(m_download_state != State::FINAL);
if (m_download_state == State::FINAL) return ret;
@@ -80,8 +80,9 @@ HeadersSyncState::ProcessingResult HeadersSyncState::ProcessNextHeaders(const
// During PRESYNC, we minimally validate block headers and
// occasionally add commitments to them, until we reach our work
// threshold (at which point m_download_state is updated to REDOWNLOAD).
ret.success = ValidateAndStoreHeadersCommitments(received_headers);
ret.success = ValidateAndStoreHeadersCommitments(headers);
if (ret.success) {
headers = {};
if (full_headers_message || m_download_state == State::REDOWNLOAD) {
// A full headers message means the peer may have more to give us;
// also if we just switched to REDOWNLOAD then we need to re-request
@@ -101,7 +102,7 @@ HeadersSyncState::ProcessingResult HeadersSyncState::ProcessNextHeaders(const
// gets big enough (meaning that we've checked enough commitments),
// we'll return a batch of headers to the caller for processing.
ret.success = true;
for (const auto& hdr : received_headers) {
for (const auto& hdr : headers) {
if (!ValidateAndStoreRedownloadedHeader(hdr)) {
// Something went wrong -- the peer gave us an unexpected chain.
// We could consider looking at the reason for failure and
@@ -112,8 +113,8 @@ HeadersSyncState::ProcessingResult HeadersSyncState::ProcessNextHeaders(const
}
if (ret.success) {
// Return any headers that are ready for acceptance.
ret.pow_validated_headers = PopHeadersReadyForAcceptance();
// Return any headers that are ready for acceptance, reusing the headers vector.
PopHeadersReadyForAcceptance(headers);
// If we hit our target blockhash, then all remaining headers will be
// returned and we can clear any leftover internal state.
@@ -277,20 +278,20 @@ bool HeadersSyncState::ValidateAndStoreRedownloadedHeader(const CBlockHeader& he
return true;
}
std::vector<CBlockHeader> HeadersSyncState::PopHeadersReadyForAcceptance()
void HeadersSyncState::PopHeadersReadyForAcceptance(std::vector<CBlockHeader>& headers)
{
std::vector<CBlockHeader> ret;
headers.clear();
Assume(m_download_state == State::REDOWNLOAD);
if (m_download_state != State::REDOWNLOAD) return ret;
if (m_download_state != State::REDOWNLOAD) return;
while (m_redownloaded_headers.size() > REDOWNLOAD_BUFFER_SIZE ||
(m_redownloaded_headers.size() > 0 && m_process_all_remaining_headers)) {
ret.emplace_back(m_redownloaded_headers.front().GetFullHeader(m_redownload_buffer_first_prev_hash));
headers.emplace_back(m_redownloaded_headers.front().GetFullHeader(m_redownload_buffer_first_prev_hash));
m_redownloaded_headers.pop_front();
m_redownload_buffer_first_prev_hash = ret.back().GetHash();
m_redownload_buffer_first_prev_hash = headers.back().GetHash();
}
return ret;
return;
}
CBlockLocator HeadersSyncState::NextHeadersRequestLocator() const

View File

@@ -140,20 +140,17 @@ public:
/** Result data structure for ProcessNextHeaders. */
struct ProcessingResult {
std::vector<CBlockHeader> pow_validated_headers;
bool success{false};
bool request_more{false};
};
/** Process a batch of headers, once a sync via this mechanism has started
*
* received_headers: headers that were received over the network for processing.
* Assumes the caller has already verified the headers
* are continuous, and has checked that each header
* satisfies the proof-of-work target included in the
* header (but not necessarily verified that the
* proof-of-work target is correct and passes consensus
* rules).
* headers: headers that were received over the network for processing. Assumes the caller has
* already verified the headers are continuous, and has checked that each header
* satisfies the proof-of-work target included in the header (but not necessarily
* verified that the proof-of-work target is correct and passes consensus rules).
* On output, this will be replaced with headers the caller has to process.
* full_headers_message: true if the message was at max capacity,
* indicating more headers may be available
* ProcessingResult.pow_validated_headers: will be filled in with any
@@ -165,8 +162,7 @@ public:
* ProcessingResult.request_more: if true, the caller is suggested to call
* NextHeadersRequestLocator and send a getheaders message using it.
*/
ProcessingResult ProcessNextHeaders(const std::vector<CBlockHeader>&
received_headers, bool full_headers_message);
ProcessingResult ProcessNextHeaders(std::vector<CBlockHeader>& headers, bool full_headers_message);
/** Issue the next GETHEADERS message to our peer.
*
@@ -198,7 +194,7 @@ private:
bool ValidateAndStoreRedownloadedHeader(const CBlockHeader& header);
/** Return a set of headers that satisfy our proof-of-work threshold */
std::vector<CBlockHeader> PopHeadersReadyForAcceptance();
void PopHeadersReadyForAcceptance(std::vector<CBlockHeader>& headers);
private:
/** NodeId of the peer (used for log messages) **/

View File

@@ -2514,12 +2514,6 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro
}
}
if (result.success) {
// We only overwrite the headers passed in if processing was
// successful.
headers.swap(result.pow_validated_headers);
}
return result.success;
}
// Either we didn't have a sync in progress, or something went wrong

View File

@@ -93,9 +93,11 @@ BOOST_AUTO_TEST_CASE(headers_sync_state)
headers_batch.insert(headers_batch.end(), std::next(first_chain.begin()), first_chain.end());
hss.reset(new HeadersSyncState(0, Params().GetConsensus(), chain_start, chain_work));
(void)hss->ProcessNextHeaders({first_chain.front()}, true);
std::vector<CBlockHeader> to_proc{first_chain.front()};
(void)hss->ProcessNextHeaders(to_proc, true);
// Pretend the first header is still "full", so we don't abort.
auto result = hss->ProcessNextHeaders(headers_batch, true);
to_proc = headers_batch;
auto result = hss->ProcessNextHeaders(to_proc, true);
// This chain should look valid, and we should have met the proof-of-work
// requirement.
@@ -104,20 +106,23 @@ BOOST_AUTO_TEST_CASE(headers_sync_state)
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::REDOWNLOAD);
// Try to sneakily feed back the second chain.
result = hss->ProcessNextHeaders(second_chain, true);
to_proc = second_chain;
result = hss->ProcessNextHeaders(to_proc, true);
BOOST_CHECK(!result.success); // foiled!
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::FINAL);
// Now try again, this time feeding the first chain twice.
hss.reset(new HeadersSyncState(0, Params().GetConsensus(), chain_start, chain_work));
(void)hss->ProcessNextHeaders(first_chain, true);
to_proc = first_chain;
(void)hss->ProcessNextHeaders(to_proc, true);
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::REDOWNLOAD);
result = hss->ProcessNextHeaders(first_chain, true);
to_proc = first_chain;
result = hss->ProcessNextHeaders(to_proc, true);
BOOST_CHECK(result.success);
BOOST_CHECK(!result.request_more);
// All headers should be ready for acceptance:
BOOST_CHECK(result.pow_validated_headers.size() == first_chain.size());
BOOST_CHECK(to_proc.size() == first_chain.size());
// Nothing left for the sync logic to do:
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::FINAL);
@@ -126,7 +131,8 @@ BOOST_AUTO_TEST_CASE(headers_sync_state)
hss.reset(new HeadersSyncState(0, Params().GetConsensus(), chain_start, chain_work));
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::PRESYNC);
// Pretend just the first message is "full", so we don't abort.
(void)hss->ProcessNextHeaders({second_chain.front()}, true);
to_proc = {second_chain.front()};
(void)hss->ProcessNextHeaders(to_proc, true);
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::PRESYNC);
headers_batch.clear();
@@ -134,9 +140,10 @@ BOOST_AUTO_TEST_CASE(headers_sync_state)
// Tell the sync logic that the headers message was not full, implying no
// more headers can be requested. For a low-work-chain, this should causes
// the sync to end with no headers for acceptance.
result = hss->ProcessNextHeaders(headers_batch, false);
to_proc = headers_batch;
result = hss->ProcessNextHeaders(to_proc, false);
BOOST_CHECK(hss->GetState() == HeadersSyncState::State::FINAL);
BOOST_CHECK(result.pow_validated_headers.empty());
BOOST_CHECK(to_proc.empty());
BOOST_CHECK(!result.request_more);
// Nevertheless, no validation errors should have been detected with the
// chain: