From 0c9d8fedc6ef1fde939346c91d304226cb297ec1 Mon Sep 17 00:00:00 2001
From: Andrei Eres <eresav@me.com>
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>
---
.../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 fe06593bd7a..b498de55dce 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 00000000000..dca181ff5c4
--- /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 e55ceb62d7c..a8ce601d6fc 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 1e20572eeeb..e45c95c7d3c 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 99a972f914e..9300cbccc9a 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 977c4c4de66..402baa7bb2a 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 5aa374057a4..6e970b39931 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 0e713626eca..36a15f1f424 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 371b04ec9e4..8d0b757ff82 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 fdf81fcd9ff..239b76b5148 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;
--
GitLab