Skip to content

Commit

Permalink
Use maximum allowed response size for request/response protocols (#5753)
Browse files Browse the repository at this point in the history
# Description

Adjust the PoV response size to the default values used in the
substrate.
Fixes #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>
  • Loading branch information
AndreiEres and sandreim authored Sep 19, 2024
1 parent d31bb8a commit 0c9d8fe
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 20 deletions.
13 changes: 5 additions & 8 deletions polkadot/node/network/protocol/src/request_response/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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`.
///
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions prdoc/pr_5753.prdoc
Original file line number Diff line number Diff line change
@@ -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
6 changes: 4 additions & 2 deletions substrate/client/network/light/src/light_client_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
)
Expand Down
3 changes: 2 additions & 1 deletion substrate/client/network/src/bitswap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use crate::{
request_responses::{IncomingRequest, OutgoingResponse, ProtocolConfig},
types::ProtocolName,
MAX_RESPONSE_SIZE,
};

use cid::{self, Version};
Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions substrate/client/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
3 changes: 2 additions & 1 deletion substrate/client/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::{
protocol_controller::{self, SetId},
service::{metrics::NotificationMetrics, traits::Direction},
types::ProtocolName,
MAX_RESPONSE_SIZE,
};

use codec::Encode;
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions substrate/client/network/sync/src/block_request_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
)
Expand Down
4 changes: 2 additions & 2 deletions substrate/client/network/sync/src/state_request_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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),
)
Expand Down
4 changes: 1 addition & 3 deletions substrate/client/network/sync/src/warp_request_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion substrate/client/network/transactions/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down

0 comments on commit 0c9d8fe

Please sign in to comment.