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

Use maximum allowed response size for request/response protocols #5753

Merged
merged 5 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to not base this value on MAX_POV_SIZE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's okay, I suppose, because we make these changes to bump it.


/// 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: |
Adjust the PoV response size to the default values used in the substrate.
AndreiEres marked this conversation as resolved.
Show resolved Hide resolved

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
Loading