Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: increase the number of block headers able to be downloaded at once to 8000 in protocol version 70235 #6239

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/release-notes-6239.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
P2P and Network Changes
-----------------------

The max number of compressed block headers which can be requested at once has been increased from 2000 to 8000. This
change activates with the protocol version `70235` and only applies to compressed block headers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: first line already mentions it's compressed-only

Suggested change
change activates with the protocol version `70235` and only applies to compressed block headers.
change activates with the protocol version `70235`.

26 changes: 18 additions & 8 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,15 @@ static bool IsLimitedPeer(const Peer& peer)
(peer.m_their_services & NODE_NETWORK_LIMITED));
}

/** Get maximum number of headers that can be included in one batch */
static uint16_t GetHeadersLimit(const CNode& pfrom, bool compressed)
{
if (pfrom.GetCommonVersion() >= INCREASE_MAX_HEADERS2_VERSION && compressed) {
return MAX_HEADERS_COMPRESSED_RESULT;
}
return MAX_HEADERS_UNCOMPRESSED_RESULT;
}

static void PushInv(Peer& peer, const CInv& inv)
{
// Dash always initializes m_tx_relay
Expand Down Expand Up @@ -2952,16 +2961,17 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer,
}

// Consider fetching more headers.
if (nCount == MAX_HEADERS_RESULTS) {
std::string msg_type = UsesCompressedHeaders(peer) ? NetMsgType::GETHEADERS2 : NetMsgType::GETHEADERS;
const bool uses_compressed = UsesCompressedHeaders(peer);
const std::string msg_type = uses_compressed ? NetMsgType::GETHEADERS2 : NetMsgType::GETHEADERS;
if (nCount == GetHeadersLimit(pfrom, uses_compressed)) {
// Headers message had its maximum size; the peer may have more headers.
if (MaybeSendGetHeaders(pfrom, msg_type, m_chainman.ActiveChain().GetLocator(pindexLast), peer)) {
LogPrint(BCLog::NET, "more %s (%d) to end to peer=%d (startheight:%d)\n",
msg_type, pindexLast->nHeight, pfrom.GetId(), peer.m_starting_height);
}
}

UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == MAX_HEADERS_RESULTS);
UpdatePeerStateForReceivedHeaders(pfrom, pindexLast, received_new_header, nCount == GetHeadersLimit(pfrom, uses_compressed));

// Consider immediately downloading blocks.
HeadersDirectFetchBlocks(pfrom, peer, pindexLast);
Expand Down Expand Up @@ -4055,8 +4065,8 @@ void PeerManagerImpl::ProcessMessage(
pindex = m_chainman.ActiveChain().Next(pindex);
}

const auto send_headers = [this /* for m_connman */, &hashStop, &pindex, &nodestate, &pfrom, &msgMaker](auto msg_type, auto& v_headers, auto callback) {
int nLimit = MAX_HEADERS_RESULTS;
const auto send_headers = [this /* for m_connman */, &hashStop, &pindex, &nodestate, &pfrom, &msgMaker](auto msg_type_internal, auto& v_headers, auto callback) {
int nLimit = GetHeadersLimit(pfrom, msg_type_internal == NetMsgType::HEADERS2);
for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex)) {
v_headers.push_back(callback(pindex));

Expand All @@ -4076,7 +4086,7 @@ void PeerManagerImpl::ProcessMessage(
// will re-announce the new block via headers (or compact blocks again)
// in the SendMessages logic.
nodestate->pindexBestHeaderSent = pindex ? pindex : m_chainman.ActiveChain().Tip();
m_connman.PushMessage(&pfrom, msgMaker.Make(msg_type, v_headers));
m_connman.PushMessage(&pfrom, msgMaker.Make(msg_type_internal, v_headers));
};

LogPrint(BCLog::NET, "%s %d to %s from peer=%d\n", msg_type, (pindex ? pindex->nHeight : -1), hashStop.IsNull() ? "end" : hashStop.ToString(), pfrom.GetId());
Expand Down Expand Up @@ -4564,7 +4574,7 @@ void PeerManagerImpl::ProcessMessage(

// Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks.
unsigned int nCount = ReadCompactSize(vRecv);
if (nCount > MAX_HEADERS_RESULTS) {
if (nCount > GetHeadersLimit(pfrom, msg_type == NetMsgType::HEADERS2)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can commit result to variable and reuse it further down, also consider changing misbehavior text to mention headers2 when appropriate due to different limit.

Suggested change
if (nCount > GetHeadersLimit(pfrom, msg_type == NetMsgType::HEADERS2)) {
const bool is_compressed{msg_type == NetMsgType::HEADERS2};
if (nCount > GetHeadersLimit(pfrom, is_compressed)) {

Misbehaving(pfrom.GetId(), 20, strprintf("headers message size = %u", nCount));
return;
}
Expand Down Expand Up @@ -6003,4 +6013,4 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
}
} // release cs_main
return true;
}
}
12 changes: 6 additions & 6 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ static RPCHelpMan getblockheaders()
"If verbose is true, each item is an Object with information about a single blockheader.\n",
{
{"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
{"count", RPCArg::Type::NUM, /* default */ strprintf("%s", MAX_HEADERS_RESULTS), ""},
{"count", RPCArg::Type::NUM, /* default */ strprintf("%s", MAX_HEADERS_UNCOMPRESSED_RESULT), ""},
{"verbose", RPCArg::Type::BOOL, /* default */ "true", "true for a json object, false for the hex-encoded data"},
},
{
Expand Down Expand Up @@ -1054,11 +1054,11 @@ static RPCHelpMan getblockheaders()
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
}

int nCount = MAX_HEADERS_RESULTS;
int nCount = MAX_HEADERS_UNCOMPRESSED_RESULT;
if (!request.params[1].isNull())
nCount = request.params[1].get_int();

if (nCount <= 0 || nCount > (int)MAX_HEADERS_RESULTS)
if (nCount <= 0 || nCount > (int)MAX_HEADERS_UNCOMPRESSED_RESULT)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Count is out of range");

bool fVerbose = true;
Expand Down Expand Up @@ -1134,7 +1134,7 @@ static RPCHelpMan getmerkleblocks()
{
{"filter", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex-encoded bloom filter"},
{"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
{"count", RPCArg::Type::NUM, /* default */ strprintf("%s", MAX_HEADERS_RESULTS), ""},
{"count", RPCArg::Type::NUM, /* default */ strprintf("%s", MAX_HEADERS_UNCOMPRESSED_RESULT), ""},
},
RPCResult{
RPCResult::Type::ARR, "", "",
Expand Down Expand Up @@ -1163,11 +1163,11 @@ static RPCHelpMan getmerkleblocks()
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
}

int nCount = MAX_HEADERS_RESULTS;
int nCount = MAX_HEADERS_UNCOMPRESSED_RESULT;
if (!request.params[2].isNull())
nCount = request.params[2].get_int();

if (nCount <= 0 || nCount > (int)MAX_HEADERS_RESULTS) {
if (nCount <= 0 || nCount > (int)MAX_HEADERS_UNCOMPRESSED_RESULT) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Count is out of range");
}

Expand Down
3 changes: 2 additions & 1 deletion src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ static const int MAX_SCRIPTCHECK_THREADS = 15;
static const int DEFAULT_SCRIPTCHECK_THREADS = 0;
/** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends
* less than this number, we reached its tip. Changing this value is a protocol upgrade. */
Comment on lines 86 to 87
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
/** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends
* less than this number, we reached its tip. Changing this value is a protocol upgrade. */
/** Number of headers sent in one getheaders(2) result. We rely on the assumption that if a peer sends
* less than this number, we reached its tip. Changing these values is a protocol upgrade. */

static const unsigned int MAX_HEADERS_RESULTS = 2000;
static const unsigned int MAX_HEADERS_UNCOMPRESSED_RESULT = 2000;
static const unsigned int MAX_HEADERS_COMPRESSED_RESULT = 8000;

static const int64_t DEFAULT_MAX_TIP_AGE = 6 * 60 * 60; // ~144 blocks behind -> 2 x fork detection time, was 24 * 60 * 60 in bitcoin

Expand Down
5 changes: 4 additions & 1 deletion src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/


static const int PROTOCOL_VERSION = 70234;
static const int PROTOCOL_VERSION = 70235;

//! initial proto version, to be increased after version/verack negotiation
static const int INIT_PROTO_VERSION = 209;
Expand Down Expand Up @@ -58,6 +58,9 @@ static const int NO_LEGACY_ISLOCK_PROTO_VERSION = 70231;
//! Inventory type for DSQ messages added
static const int DSQ_INV_VERSION = 70234;

//! Maximum header count for HEADRES2 message was increased from 2000 to 8000 in this version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
//! Maximum header count for HEADRES2 message was increased from 2000 to 8000 in this version
//! Maximum header count for HEADERS2 message was increased from 2000 to 8000 in this version

static const int INCREASE_MAX_HEADERS2_VERSION = 70235;

// Make sure that none of the values above collide with `ADDRV2_FORMAT`.

#endif // BITCOIN_VERSION_H
10 changes: 8 additions & 2 deletions test/functional/p2p_invalid_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
CInv,
msg_ping,
ser_string,
MAX_HEADERS_RESULTS,
MAX_HEADERS_COMPRESSED_RESULT,
MAX_HEADERS_UNCOMPRESSED_RESULT,
MAX_INV_SIZE,
MAX_PROTOCOL_MESSAGE_LENGTH,
msg_getdata,
msg_headers,
msg_headers2,
msg_inv,
MSG_TX,
msg_version,
Expand Down Expand Up @@ -153,9 +155,13 @@ def test_oversized_getdata_msg(self):
self.test_oversized_msg(msg_getdata([CInv(MSG_TX, 1)] * size), size)

def test_oversized_headers_msg(self):
size = MAX_HEADERS_RESULTS + 1
size = MAX_HEADERS_UNCOMPRESSED_RESULT + 1
self.test_oversized_msg(msg_headers([CBlockHeader()] * size), size)

def test_oversized_headers2_msg(self):
size = MAX_HEADERS_COMPRESSED_RESULT + 1
self.test_oversized_msg(msg_headers2([CBlockHeader()] * size), size)

def test_resource_exhaustion(self):
self.log.info("Test node stays up despite many large junk messages")
conn = self.nodes[0].add_p2p_connection(P2PDataStore())
Expand Down
3 changes: 2 additions & 1 deletion test/functional/test_framework/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
BIP125_SEQUENCE_NUMBER = 0xfffffffd # Sequence number that is BIP 125 opt-in and BIP 68-opt-out

MAX_PROTOCOL_MESSAGE_LENGTH = 3 * 1024 * 1024 # Maximum length of incoming protocol messages
MAX_HEADERS_RESULTS = 2000 # Number of headers sent in one getheaders result
MAX_HEADERS_UNCOMPRESSED_RESULT = 2000 # Number of headers sent in one getheaders result
MAX_HEADERS_COMPRESSED_RESULT = 8000 # Number of headers2 sent in one getheaders2 result
MAX_INV_SIZE = 50000 # Maximum number of entries in an 'inv' protocol message

NODE_NETWORK = (1 << 0)
Expand Down
15 changes: 8 additions & 7 deletions test/functional/test_framework/p2p.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
from test_framework.messages import (
CBlockHeader,
CompressibleBlockHeader,
MAX_HEADERS_RESULTS,
MAX_HEADERS_COMPRESSED_RESULT,
MAX_HEADERS_UNCOMPRESSED_RESULT,
NODE_HEADERS_COMPRESSED,
msg_addr,
msg_addrv2,
Expand Down Expand Up @@ -91,8 +92,8 @@
# The minimum P2P version that this test framework supports
MIN_P2P_VERSION_SUPPORTED = 60001
# The P2P version that this test framework implements and sends in its `version` message
# Version 70231 drops supports for legacy InstantSend locks
P2P_VERSION = 70231
# Version 70235 increased max header count for HEADERS2 message from 2000 to 8000
P2P_VERSION = 70235
# The services that this test framework offers in its `version` message
P2P_SERVICES = NODE_NETWORK | NODE_HEADERS_COMPRESSED
# The P2P user agent string that this test framework sends in its `version` message
Expand Down Expand Up @@ -712,7 +713,7 @@ def on_getdata(self, message):
else:
logger.debug('getdata message type {} received.'.format(hex(inv.type)))

def _compute_requested_block_headers(self, locator, hash_stop):
def _compute_requested_block_headers(self, locator, hash_stop, max_results):
# Assume that the most recent block added is the tip
if not self.block_store:
return
Expand All @@ -733,14 +734,14 @@ def _compute_requested_block_headers(self, locator, hash_stop):
break

# Truncate the list if there are too many headers
headers_list = headers_list[:-MAX_HEADERS_RESULTS - 1:-1]
headers_list = headers_list[:-max_results - 1:-1]

return headers_list

def on_getheaders2(self, message):
"""Search back through our block store for the locator, and reply with a compressed headers message if found."""

headers_list = self._compute_requested_block_headers(message.locator, message.hashstop)
headers_list = self._compute_requested_block_headers(message.locator, message.hashstop, MAX_HEADERS_COMPRESSED_RESULT)
compressible_headers_list = [CompressibleBlockHeader(h) for h in headers_list] if headers_list else None
response = msg_headers2(compressible_headers_list)

Expand All @@ -750,7 +751,7 @@ def on_getheaders2(self, message):
def on_getheaders(self, message):
"""Search back through our block store for the locator, and reply with a headers message if found."""

headers_list = self._compute_requested_block_headers(message.locator, message.hashstop)
headers_list = self._compute_requested_block_headers(message.locator, message.hashstop, MAX_HEADERS_UNCOMPRESSED_RESULT)
response = msg_headers(headers_list)

if response is not None:
Expand Down
Loading