diff --git a/doc/release-notes-266.md b/doc/release-notes-266.md new file mode 100644 index 0000000000..bf6111be34 --- /dev/null +++ b/doc/release-notes-266.md @@ -0,0 +1,29 @@ +Headers messages stay within the message size limit +=================================================== + +A block header carries its whole AuxPoW proof, including the parent chain's +coinbase transaction, so a run of blocks with large AuxPoW payloads could make +a reply to `getheaders` larger than the 4,000,000 bytes a peer accepts in one +message. The receiving peer dropped the connection, so new or catching-up +nodes could not sync headers past those blocks. + +Nodes now end a `headers` message before it would exceed that limit, always +sending at least one header, and announce new blocks with `inv` when their +headers do not fit in one message. A node keeps requesting headers after a +`headers` message with 2,000 headers, as before, or of at least 2,000,000 +bytes, since such a message may have been cut short to fit the limit. This +applies to normal headers sync and to low-work headers sync (presync and +redownload). Replies made of ordinary headers are unchanged: 2,000 headers +per message, byte for byte as before. Consensus rules are unchanged. + +Known limitations, deferred to later releases: + +- No consensus rule limits the size of an AuxPoW payload yet. Adding one + needs an activated soft fork. +- The block index still keeps every AuxPoW payload in memory. +- Nodes running v1.0.0 still stall or abort headers sync through ranges with + large AuxPoW headers. They take a message cut short for size as the end of + the peer's headers, and when they serve headers they still send the + oversized message, which upgraded nodes also reject. +- Low-work headers sync still gives up on a peer if the AuxPoW payloads it + holds while redownloading exceed its existing 36 MB per-peer bound. diff --git a/src/test/headers_sync_chainwork_tests.cpp b/src/test/headers_sync_chainwork_tests.cpp index e0b02f7635..a64967c073 100644 --- a/src/test/headers_sync_chainwork_tests.cpp +++ b/src/test/headers_sync_chainwork_tests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -311,6 +312,78 @@ BOOST_AUTO_TEST_CASE(headers_sync_state) BOOST_CHECK(result.success); } +// A peer serving large AuxPoW headers ends a headers message early to keep it +// under the receive limit, so a message far short of MAX_HEADERS_RESULTS +// headers may not reach the peer's tip. Both sync phases must keep requesting +// headers after such a message. +BOOST_AUTO_TEST_CASE(headers_sync_continues_through_byte_limited_messages) +{ + const auto& consensus{Params().GetConsensus()}; + const CBlockIndex* chain_start = WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(Params().GenesisBlock().GetHash())); + BOOST_REQUIRE(chain_start); + + // Five large AuxPoW headers followed by three ordinary ones. The header + // hash does not commit to the payload, so the large headers can share one. + std::vector headers; + GenerateHeaders(headers, 5, chain_start->GetBlockHash(), + MakeVersion(static_cast(consensus.nAuxpowChainId), /*auxpow=*/true, /*version_bits=*/0), + chain_start->GetBlockTime(), ArithToUint256(0), chain_start->nBits); + const auto large_auxpow{MakeAuxpowPayloadWithScriptSigSize(1'900'000)}; + for (CBlockHeader& header : headers) header.auxpow = large_auxpow; + GenerateHeaders(headers, 8, headers.back().GetHash(), chain_start->nVersion, + headers.back().nTime, ArithToUint256(0), chain_start->nBits); + + const auto messages{SplitIntoHeadersMessages(headers)}; + BOOST_REQUIRE_EQUAL(messages.size(), 3U); + BOOST_CHECK_EQUAL(messages[0].size(), 2U); + BOOST_CHECK_EQUAL(messages[1].size(), 2U); + BOOST_CHECK_EQUAL(messages[2].size(), 4U); + BOOST_CHECK(MayHaveMoreHeaders(messages[0])); + BOOST_CHECK(MayHaveMoreHeaders(messages[1])); + BOOST_CHECK(!MayHaveMoreHeaders(messages[2])); + + // Judged by header count alone, the first message looks final and + // PRESYNC gives up on the peer. + { + HeadersSyncState hss{/*id=*/0, consensus, chain_start, ClaimedWork(*chain_start, headers)}; + const auto result{hss.ProcessNextHeaders(messages[0], /*full_headers_message=*/messages[0].size() == MAX_HEADERS_RESULTS)}; + BOOST_CHECK(result.success); + BOOST_CHECK(!result.request_more); + BOOST_CHECK(hss.GetState() == HeadersSyncState::State::FINAL); + } + + HeadersSyncState hss{/*id=*/0, consensus, chain_start, ClaimedWork(*chain_start, headers)}; + // PRESYNC reaches the required work with the last message, then restarts + // from the chain start for REDOWNLOAD. + for (size_t i{0}; i < messages.size(); ++i) { + const auto result{hss.ProcessNextHeaders(messages[i], MayHaveMoreHeaders(messages[i]))}; + BOOST_REQUIRE(result.success); + BOOST_REQUIRE(result.request_more); + BOOST_CHECK(result.pow_validated_headers.empty()); + BOOST_CHECK(hss.GetState() == (i + 1 < messages.size() ? HeadersSyncState::State::PRESYNC : HeadersSyncState::State::REDOWNLOAD)); + } + BOOST_CHECK_EQUAL(hss.NextHeadersRequestLocator().vHave.front(), chain_start->GetBlockHash()); + + // REDOWNLOAD asks for more after each byte-limited message and releases + // every header once it reaches the required work. + std::vector released; + for (size_t i{0}; i < messages.size(); ++i) { + const auto result{hss.ProcessNextHeaders(messages[i], MayHaveMoreHeaders(messages[i]))}; + BOOST_REQUIRE(result.success); + BOOST_CHECK_EQUAL(result.request_more, i + 1 < messages.size()); + if (result.request_more) { + BOOST_CHECK_EQUAL(hss.NextHeadersRequestLocator().vHave.front(), messages[i].back().GetHash()); + } + released.insert(released.end(), result.pow_validated_headers.begin(), result.pow_validated_headers.end()); + } + BOOST_CHECK(hss.GetState() == HeadersSyncState::State::FINAL); + BOOST_REQUIRE_EQUAL(released.size(), headers.size()); + for (size_t i{0}; i < headers.size(); ++i) { + BOOST_CHECK_EQUAL(released[i].GetHash(), headers[i].GetHash()); + BOOST_CHECK(released[i].auxpow == headers[i].auxpow); + } +} + BOOST_AUTO_TEST_CASE(headers_sync_future_chain_start_does_not_wrap_commitment_limit) { const CBlockIndex* chain_start = WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(Params().GenesisBlock().GetHash())); diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index 62d8ec1001..c4c6607389 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -3,20 +3,27 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include +#include #include +#include #include #include #include #include +#include