From fae15379cba0c876aa16c77e11809c83d1db8f5c Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Wed, 18 Sep 2024 13:04:38 +0200 Subject: [PATCH 1/5] Update POV_RESPONSE_SIZE --- .../node/network/protocol/src/request_response/mod.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index fe06593bd7a0..5d7eb1fe5d92 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -51,7 +51,7 @@ use std::{collections::HashMap, time::Duration, u64}; -use polkadot_primitives::{MAX_CODE_SIZE, MAX_POV_SIZE}; +use polkadot_primitives::MAX_CODE_SIZE; use sc_network::NetworkBackend; 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 = 16 * 1024 * 1024; /// 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, From 754c829fc73cec0dfe3fe9c115febfce5b681110 Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Thu, 19 Sep 2024 12:18:04 +0200 Subject: [PATCH 2/5] Extract into a constant --- polkadot/node/network/protocol/src/request_response/mod.rs | 4 ++-- substrate/client/network/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 ++- substrate/client/network/sync/src/block_request_handler.rs | 4 ++-- substrate/client/network/sync/src/state_request_handler.rs | 4 ++-- substrate/client/network/sync/src/warp_request_handler.rs | 4 +--- substrate/client/network/transactions/src/config.rs | 3 ++- 9 files changed, 20 insertions(+), 14 deletions(-) diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index 5d7eb1fe5d92..b498de55dce4 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -52,7 +52,7 @@ use std::{collections::HashMap, time::Duration, u64}; use polkadot_primitives::MAX_CODE_SIZE; -use sc_network::NetworkBackend; +use sc_network::{NetworkBackend, MAX_RESPONSE_SIZE}; use sp_runtime::traits::Block; use strum::{EnumIter, IntoEnumIterator}; @@ -160,7 +160,7 @@ pub const MAX_PARALLEL_ATTESTED_CANDIDATE_REQUESTS: u32 = 5; /// Response size limit for responses of POV like data. /// /// Same as what we use in substrate networking. -const POV_RESPONSE_SIZE: u64 = 16 * 1024 * 1024; +const POV_RESPONSE_SIZE: u64 = MAX_RESPONSE_SIZE; /// Maximum response sizes for `StatementFetchingV1`. /// 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; From bf368e488cc3c561c621f53b308e2f6cfc3acabc Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Thu, 19 Sep 2024 14:46:53 +0200 Subject: [PATCH 3/5] Add prdoc --- prdoc/pr_5753.prdoc | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 prdoc/pr_5753.prdoc diff --git a/prdoc/pr_5753.prdoc b/prdoc/pr_5753.prdoc new file mode 100644 index 000000000000..1500af0ec95a --- /dev/null +++ b/prdoc/pr_5753.prdoc @@ -0,0 +1,11 @@ +# 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: | + Adjust the PoV response size to the default values used in the substrate. + +crates: [ ] From 35149dff6c7de89f9df61149f56d783471f79abd Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Thu, 19 Sep 2024 15:26:09 +0200 Subject: [PATCH 4/5] Update pr doc --- prdoc/pr_5753.prdoc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/prdoc/pr_5753.prdoc b/prdoc/pr_5753.prdoc index 1500af0ec95a..cbaff755523c 100644 --- a/prdoc/pr_5753.prdoc +++ b/prdoc/pr_5753.prdoc @@ -8,4 +8,14 @@ doc: description: | Adjust the PoV response size to the default values used in the substrate. -crates: [ ] +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 From a48876ac5f804f7faf5b5e751f04248b57508438 Mon Sep 17 00:00:00 2001 From: Andrei Eres Date: Thu, 19 Sep 2024 17:04:35 +0200 Subject: [PATCH 5/5] Update prdoc/pr_5753.prdoc Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com> --- prdoc/pr_5753.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5753.prdoc b/prdoc/pr_5753.prdoc index cbaff755523c..dca181ff5c40 100644 --- a/prdoc/pr_5753.prdoc +++ b/prdoc/pr_5753.prdoc @@ -6,7 +6,7 @@ title: Use maximum allowed response size for request/response protocols doc: - audience: Node Dev description: | - Adjust the PoV response size to the default values used in the substrate. + Increase maximum PoV response size to 16MB which is equal to the default value used in the substrate. crates: - name: sc-network