From e8a416b7264e39b70672208338728e6f59578f2c Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Thu, 19 Sep 2024 17:45:35 +0200 Subject: [PATCH] Use maximum allowed response size for request/response protocols (#5753) # Description Adjust the PoV response size to the default values used in the substrate. Fixes https://github.com/paritytech/polkadot-sdk/issues/5503 ## Integration The changes shouldn't impact downstream projects since we are only increasing the limit. ## Review Notes You can't see it from the changes, but it affects all protocols that use the `POV_RESPONSE_SIZE` constant. - Protocol::ChunkFetchingV1 - Protocol::ChunkFetchingV2 - Protocol::CollationFetchingV1 - Protocol::CollationFetchingV2 - Protocol::PoVFetchingV1 - Protocol::AvailableDataFetchingV1 ## Increasing timeouts https://github.com/paritytech/polkadot-sdk/blob/fae15379cba0c876aa16c77e11809c83d1db8f5c/polkadot/node/network/protocol/src/request_response/mod.rs#L126-L129 I assume the current PoV request timeout is set to 1.2s to handle 5 consecutive requests during a 6s block. This setting does not relate to the PoV response size. I see no reason to change the current timeouts after adjusting the response size. However, we should consider networking speed limitations if we want to increase the maximum PoV size to 10 MB. With the number of parallel requests set to 10, validators will need the following networking speeds: - 5 MB PoV: at least 42 MB/s, ideally 50 MB/s. - 10 MB PoV: at least 84 MB/s, ideally 100 MB/s. The current required speed of 50 MB/s aligns with the 62.5 MB/s specified [in the reference hardware requirements](https://wiki.polkadot.network/docs/maintain-guides-how-to-validate-polkadot#reference-hardware). Increasing the PoV size to 10 MB may require a higher networking speed. --------- Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> (cherry picked from commit 0c9d8fedc6ef1fde939346c91d304226cb297ec1) --- .../protocol/src/request_response/mod.rs | 13 +++++------- prdoc/pr_5753.prdoc | 21 +++++++++++++++++++ .../light/src/light_client_requests.rs | 6 ++++-- substrate/client/network/src/bitswap/mod.rs | 3 ++- substrate/client/network/src/lib.rs | 3 +++ substrate/client/network/src/protocol.rs | 3 ++- .../network/sync/src/block_request_handler.rs | 4 ++-- .../network/sync/src/state_request_handler.rs | 4 ++-- .../network/sync/src/warp_request_handler.rs | 4 +--- .../client/network/transactions/src/config.rs | 3 ++- 10 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 prdoc/pr_5753.prdoc diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index fe06593bd7a0..b498de55dce4 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -51,8 +51,8 @@ use std::{collections::HashMap, time::Duration, u64}; -use polkadot_primitives::{MAX_CODE_SIZE, MAX_POV_SIZE}; -use sc_network::NetworkBackend; +use polkadot_primitives::MAX_CODE_SIZE; +use sc_network::{NetworkBackend, MAX_RESPONSE_SIZE}; use sp_runtime::traits::Block; use strum::{EnumIter, IntoEnumIterator}; @@ -159,11 +159,8 @@ pub const MAX_PARALLEL_ATTESTED_CANDIDATE_REQUESTS: u32 = 5; /// Response size limit for responses of POV like data. /// -/// This is larger than `MAX_POV_SIZE` to account for protocol overhead and for additional data in -/// `CollationFetchingV1` or `AvailableDataFetchingV1` for example. We try to err on larger limits -/// here as a too large limit only allows an attacker to waste our bandwidth some more, a too low -/// limit might have more severe effects. -const POV_RESPONSE_SIZE: u64 = MAX_POV_SIZE as u64 + 10_000; +/// Same as what we use in substrate networking. +const POV_RESPONSE_SIZE: u64 = MAX_RESPONSE_SIZE; /// Maximum response sizes for `StatementFetchingV1`. /// @@ -217,7 +214,7 @@ impl Protocol { name, legacy_names, 1_000, - POV_RESPONSE_SIZE as u64 * 3, + POV_RESPONSE_SIZE, // We are connected to all validators: CHUNK_REQUEST_TIMEOUT, tx, diff --git a/prdoc/pr_5753.prdoc b/prdoc/pr_5753.prdoc new file mode 100644 index 000000000000..dca181ff5c40 --- /dev/null +++ b/prdoc/pr_5753.prdoc @@ -0,0 +1,21 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Use maximum allowed response size for request/response protocols + +doc: + - audience: Node Dev + description: | + Increase maximum PoV response size to 16MB which is equal to the default value used in the substrate. + +crates: + - name: sc-network + bump: patch + - name: sc-network-light + bump: patch + - name: sc-network-sync + bump: patch + - name: polkadot-node-network-protocol + bump: patch + - name: sc-network-transactions + bump: patch diff --git a/substrate/client/network/light/src/light_client_requests.rs b/substrate/client/network/light/src/light_client_requests.rs index e55ceb62d7cd..a8ce601d6fc2 100644 --- a/substrate/client/network/light/src/light_client_requests.rs +++ b/substrate/client/network/light/src/light_client_requests.rs @@ -18,7 +18,9 @@ //! Helpers for outgoing and incoming light client requests. -use sc_network::{config::ProtocolId, request_responses::IncomingRequest, NetworkBackend}; +use sc_network::{ + config::ProtocolId, request_responses::IncomingRequest, NetworkBackend, MAX_RESPONSE_SIZE, +}; use sp_runtime::traits::Block; use std::time::Duration; @@ -57,7 +59,7 @@ pub fn generate_protocol_config< generate_protocol_name(genesis_hash, fork_id).into(), std::iter::once(generate_legacy_protocol_name(protocol_id).into()).collect(), 1 * 1024 * 1024, - 16 * 1024 * 1024, + MAX_RESPONSE_SIZE, Duration::from_secs(15), Some(inbound_queue), ) diff --git a/substrate/client/network/src/bitswap/mod.rs b/substrate/client/network/src/bitswap/mod.rs index 1e20572eeeb1..e45c95c7d3c8 100644 --- a/substrate/client/network/src/bitswap/mod.rs +++ b/substrate/client/network/src/bitswap/mod.rs @@ -23,6 +23,7 @@ use crate::{ request_responses::{IncomingRequest, OutgoingResponse, ProtocolConfig}, types::ProtocolName, + MAX_RESPONSE_SIZE, }; use cid::{self, Version}; @@ -47,7 +48,7 @@ const LOG_TARGET: &str = "bitswap"; // https://github.com/ipfs/js-ipfs-bitswap/blob/ // d8f80408aadab94c962f6b88f343eb9f39fa0fcc/src/decision-engine/index.js#L16 // We set it to the same value as max substrate protocol message -const MAX_PACKET_SIZE: u64 = 16 * 1024 * 1024; +const MAX_PACKET_SIZE: u64 = MAX_RESPONSE_SIZE; /// Max number of queued responses before denying requests. const MAX_REQUEST_QUEUE: usize = 20; diff --git a/substrate/client/network/src/lib.rs b/substrate/client/network/src/lib.rs index 99a972f914e2..9300cbccc9ad 100644 --- a/substrate/client/network/src/lib.rs +++ b/substrate/client/network/src/lib.rs @@ -302,3 +302,6 @@ const MAX_CONNECTIONS_PER_PEER: usize = 2; /// The maximum number of concurrent established connections that were incoming. const MAX_CONNECTIONS_ESTABLISHED_INCOMING: u32 = 10_000; + +/// Maximum response size limit. +pub const MAX_RESPONSE_SIZE: u64 = 16 * 1024 * 1024; diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs index 977c4c4de663..402baa7bb2a4 100644 --- a/substrate/client/network/src/protocol.rs +++ b/substrate/client/network/src/protocol.rs @@ -22,6 +22,7 @@ use crate::{ protocol_controller::{self, SetId}, service::{metrics::NotificationMetrics, traits::Direction}, types::ProtocolName, + MAX_RESPONSE_SIZE, }; use codec::Encode; @@ -56,7 +57,7 @@ pub mod message; /// Maximum size used for notifications in the block announce and transaction protocols. // Must be equal to `max(MAX_BLOCK_ANNOUNCE_SIZE, MAX_TRANSACTIONS_SIZE)`. -pub(crate) const BLOCK_ANNOUNCES_TRANSACTIONS_SUBSTREAM_SIZE: u64 = 16 * 1024 * 1024; +pub(crate) const BLOCK_ANNOUNCES_TRANSACTIONS_SUBSTREAM_SIZE: u64 = MAX_RESPONSE_SIZE; /// Identifier of the peerset for the block announces protocol. const HARDCODED_PEERSETS_SYNC: SetId = SetId::from(0); diff --git a/substrate/client/network/sync/src/block_request_handler.rs b/substrate/client/network/sync/src/block_request_handler.rs index 5aa374057a4a..6e970b399310 100644 --- a/substrate/client/network/sync/src/block_request_handler.rs +++ b/substrate/client/network/sync/src/block_request_handler.rs @@ -39,7 +39,7 @@ use sc_network::{ request_responses::{IfDisconnected, IncomingRequest, OutgoingResponse, RequestFailure}, service::traits::RequestResponseConfig, types::ProtocolName, - NetworkBackend, + NetworkBackend, MAX_RESPONSE_SIZE, }; use sc_network_common::sync::message::{BlockAttributes, BlockData, BlockRequest, FromBlock}; use sc_network_types::PeerId; @@ -89,7 +89,7 @@ pub fn generate_protocol_config< generate_protocol_name(genesis_hash, fork_id).into(), std::iter::once(generate_legacy_protocol_name(protocol_id).into()).collect(), 1024 * 1024, - 16 * 1024 * 1024, + MAX_RESPONSE_SIZE, Duration::from_secs(20), Some(inbound_queue), ) diff --git a/substrate/client/network/sync/src/state_request_handler.rs b/substrate/client/network/sync/src/state_request_handler.rs index 0e713626ecaa..36a15f1f4240 100644 --- a/substrate/client/network/sync/src/state_request_handler.rs +++ b/substrate/client/network/sync/src/state_request_handler.rs @@ -33,7 +33,7 @@ use sc_client_api::{BlockBackend, ProofProvider}; use sc_network::{ config::ProtocolId, request_responses::{IncomingRequest, OutgoingResponse}, - NetworkBackend, + NetworkBackend, MAX_RESPONSE_SIZE, }; use sp_runtime::traits::Block as BlockT; @@ -69,7 +69,7 @@ pub fn generate_protocol_config< generate_protocol_name(genesis_hash, fork_id).into(), std::iter::once(generate_legacy_protocol_name(protocol_id).into()).collect(), 1024 * 1024, - 16 * 1024 * 1024, + MAX_RESPONSE_SIZE, Duration::from_secs(40), Some(inbound_queue), ) diff --git a/substrate/client/network/sync/src/warp_request_handler.rs b/substrate/client/network/sync/src/warp_request_handler.rs index 371b04ec9e4d..8d0b757ff821 100644 --- a/substrate/client/network/sync/src/warp_request_handler.rs +++ b/substrate/client/network/sync/src/warp_request_handler.rs @@ -27,14 +27,12 @@ use crate::{ use sc_network::{ config::ProtocolId, request_responses::{IncomingRequest, OutgoingResponse}, - NetworkBackend, + NetworkBackend, MAX_RESPONSE_SIZE, }; use sp_runtime::traits::Block as BlockT; use std::{sync::Arc, time::Duration}; -const MAX_RESPONSE_SIZE: u64 = 16 * 1024 * 1024; - /// Incoming warp requests bounded queue size. const MAX_WARP_REQUEST_QUEUE: usize = 20; diff --git a/substrate/client/network/transactions/src/config.rs b/substrate/client/network/transactions/src/config.rs index fdf81fcd9ff4..239b76b51485 100644 --- a/substrate/client/network/transactions/src/config.rs +++ b/substrate/client/network/transactions/src/config.rs @@ -19,6 +19,7 @@ //! Configuration of the transaction protocol use futures::prelude::*; +use sc_network::MAX_RESPONSE_SIZE; use sc_network_common::ExHashT; use sp_runtime::traits::Block as BlockT; use std::{collections::HashMap, future::Future, pin::Pin, time}; @@ -32,7 +33,7 @@ pub(crate) const PROPAGATE_TIMEOUT: time::Duration = time::Duration::from_millis pub(crate) const MAX_KNOWN_TRANSACTIONS: usize = 10240; // ~300kb per peer + overhead. /// Maximum allowed size for a transactions notification. -pub(crate) const MAX_TRANSACTIONS_SIZE: u64 = 16 * 1024 * 1024; +pub(crate) const MAX_TRANSACTIONS_SIZE: u64 = MAX_RESPONSE_SIZE; /// Maximum number of transaction validation request we keep at any moment. pub(crate) const MAX_PENDING_TRANSACTIONS: usize = 8192;