From fee810a5ea177f95ddf13ea51270b84f1e117bae Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 20 Feb 2024 17:16:21 +0100 Subject: [PATCH] rpc server: make possible to disable/enable batch requests (#3364) The rationale behind this, is that it may be useful for some users actually disable RPC batch requests or limit them by length instead of the total size bytes of the batch. This PR adds two new CLI options: ``` --rpc-disable-batch-requests - disable batch requests on the server --rpc-max-batch-request-len - limit batches to LEN on the server. ``` --- .gitlab/pipeline/check.yml | 1 + cumulus/client/cli/src/lib.rs | 10 ++++++- cumulus/test/service/src/lib.rs | 3 ++- polkadot/node/test/service/src/lib.rs | 5 ++-- prdoc/pr_3364.prdoc | 12 +++++++++ .../bin/node/cli/benches/block_production.rs | 3 ++- .../bin/node/cli/benches/transaction_pool.rs | 3 ++- substrate/client/cli/src/commands/run_cmd.rs | 26 ++++++++++++++++++- substrate/client/cli/src/config.rs | 11 ++++++-- substrate/client/cli/src/runner.rs | 1 + substrate/client/rpc-servers/src/lib.rs | 6 ++++- substrate/client/service/src/config.rs | 3 +++ substrate/client/service/src/lib.rs | 1 + substrate/client/service/test/src/lib.rs | 3 ++- 14 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 prdoc/pr_3364.prdoc diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index 1ed12e68c2..cdb5d1b05d 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -133,6 +133,7 @@ check-runtime-migration-westend: WASM: "westend_runtime.compact.compressed.wasm" URI: "wss://westend-try-runtime-node.parity-chains.parity.io:443" SUBCOMMAND_EXTRA_ARGS: "--no-weight-warnings" + allow_failure: true check-runtime-migration-rococo: stage: check diff --git a/cumulus/client/cli/src/lib.rs b/cumulus/client/cli/src/lib.rs index 1807b8a171..a7b2eb19de 100644 --- a/cumulus/client/cli/src/lib.rs +++ b/cumulus/client/cli/src/lib.rs @@ -30,7 +30,7 @@ use codec::Encode; use sc_chain_spec::ChainSpec; use sc_client_api::HeaderBackend; use sc_service::{ - config::{PrometheusConfig, TelemetryEndpoints}, + config::{PrometheusConfig, RpcBatchRequestConfig, TelemetryEndpoints}, BasePath, TransactionPoolOptions, }; use sp_core::hexdisplay::HexDisplay; @@ -443,6 +443,14 @@ impl sc_cli::CliConfiguration for NormalizedRunCmd { Ok(self.base.rpc_max_subscriptions_per_connection) } + fn rpc_buffer_capacity_per_connection(&self) -> sc_cli::Result { + Ok(self.base.rpc_message_buffer_capacity_per_connection) + } + + fn rpc_batch_config(&self) -> sc_cli::Result { + self.base.rpc_batch_config() + } + fn transaction_pool(&self, is_dev: bool) -> sc_cli::Result { self.base.transaction_pool(is_dev) } diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 229bde4d19..1c2e1db974 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -67,7 +67,7 @@ use sc_network::{ use sc_service::{ config::{ BlocksPruning, DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, NetworkConfiguration, - OffchainWorkerConfig, PruningMode, WasmExecutionMethod, + OffchainWorkerConfig, PruningMode, RpcBatchRequestConfig, WasmExecutionMethod, }, BasePath, ChainSpec as ChainSpecService, Configuration, Error as ServiceError, PartialComponents, Role, RpcHandlers, TFullBackend, TFullClient, TaskManager, @@ -801,6 +801,7 @@ pub fn node_config( rpc_max_subs_per_conn: Default::default(), rpc_port: 9945, rpc_message_buffer_capacity: Default::default(), + rpc_batch_config: RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None, diff --git a/polkadot/node/test/service/src/lib.rs b/polkadot/node/test/service/src/lib.rs index a3af977ab3..ca904bae28 100644 --- a/polkadot/node/test/service/src/lib.rs +++ b/polkadot/node/test/service/src/lib.rs @@ -44,8 +44,8 @@ use sc_network::{ }; use sc_service::{ config::{ - DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, WasmExecutionMethod, - WasmtimeInstantiationStrategy, + DatabaseSource, KeystoreConfig, MultiaddrWithPeerId, RpcBatchRequestConfig, + WasmExecutionMethod, WasmtimeInstantiationStrategy, }, BasePath, BlocksPruning, Configuration, Role, RpcHandlers, TaskManager, }; @@ -186,6 +186,7 @@ pub fn node_config( rpc_max_subs_per_conn: Default::default(), rpc_port: 9944, rpc_message_buffer_capacity: Default::default(), + rpc_batch_config: RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None, diff --git a/prdoc/pr_3364.prdoc b/prdoc/pr_3364.prdoc new file mode 100644 index 0000000000..1e7a6a5278 --- /dev/null +++ b/prdoc/pr_3364.prdoc @@ -0,0 +1,12 @@ +# 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: rpc server expose batch request configuration + +doc: + - audience: Node Operator + description: | + Add functionality to limit RPC batch requests by two new CLI options: + --rpc-disable-batch-request - disable batch requests on the server + --rpc-max-batch-request-len - limit batches to LEN on the server +crates: [ ] diff --git a/substrate/bin/node/cli/benches/block_production.rs b/substrate/bin/node/cli/benches/block_production.rs index d28cfbddfd..23a62cc0bd 100644 --- a/substrate/bin/node/cli/benches/block_production.rs +++ b/substrate/bin/node/cli/benches/block_production.rs @@ -28,7 +28,7 @@ use sc_consensus::{ use sc_service::{ config::{ BlocksPruning, DatabaseSource, KeystoreConfig, NetworkConfiguration, OffchainWorkerConfig, - PruningMode, WasmExecutionMethod, WasmtimeInstantiationStrategy, + PruningMode, RpcBatchRequestConfig, WasmExecutionMethod, WasmtimeInstantiationStrategy, }, BasePath, Configuration, Role, }; @@ -84,6 +84,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { rpc_max_subs_per_conn: Default::default(), rpc_port: 9944, rpc_message_buffer_capacity: Default::default(), + rpc_batch_config: RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None, diff --git a/substrate/bin/node/cli/benches/transaction_pool.rs b/substrate/bin/node/cli/benches/transaction_pool.rs index 1e25b7ce6f..de4eef1944 100644 --- a/substrate/bin/node/cli/benches/transaction_pool.rs +++ b/substrate/bin/node/cli/benches/transaction_pool.rs @@ -26,7 +26,7 @@ use node_primitives::AccountId; use sc_service::{ config::{ BlocksPruning, DatabaseSource, KeystoreConfig, NetworkConfiguration, OffchainWorkerConfig, - PruningMode, TransactionPoolOptions, + PruningMode, RpcBatchRequestConfig, TransactionPoolOptions, }, BasePath, Configuration, Role, }; @@ -80,6 +80,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase { rpc_max_subs_per_conn: Default::default(), rpc_port: 9944, rpc_message_buffer_capacity: Default::default(), + rpc_batch_config: RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None, diff --git a/substrate/client/cli/src/commands/run_cmd.rs b/substrate/client/cli/src/commands/run_cmd.rs index c50198cf3c..221c32affd 100644 --- a/substrate/client/cli/src/commands/run_cmd.rs +++ b/substrate/client/cli/src/commands/run_cmd.rs @@ -30,7 +30,7 @@ use crate::{ use clap::Parser; use regex::Regex; use sc_service::{ - config::{BasePath, PrometheusConfig, TransactionPoolOptions}, + config::{BasePath, PrometheusConfig, RpcBatchRequestConfig, TransactionPoolOptions}, ChainSpec, Role, }; use sc_telemetry::TelemetryEndpoints; @@ -125,6 +125,14 @@ pub struct RunCmd { #[arg(long, default_value_t = RPC_DEFAULT_MESSAGE_CAPACITY_PER_CONN)] pub rpc_message_buffer_capacity_per_connection: u32, + /// Disable RPC batch requests + #[arg(long, alias = "rpc_no_batch_requests", conflicts_with_all = &["rpc_max_batch_request_len"])] + pub rpc_disable_batch_requests: bool, + + /// Limit the max length per RPC batch request + #[arg(long, conflicts_with_all = &["rpc_disable_batch_requests"], value_name = "LEN")] + pub rpc_max_batch_request_len: Option, + /// Specify browser *origins* allowed to access the HTTP & WS RPC servers. /// /// A comma-separated list of origins (protocol://domain or special `null` @@ -411,6 +419,22 @@ impl CliConfiguration for RunCmd { Ok(self.rpc_max_subscriptions_per_connection) } + fn rpc_buffer_capacity_per_connection(&self) -> Result { + Ok(self.rpc_message_buffer_capacity_per_connection) + } + + fn rpc_batch_config(&self) -> Result { + let cfg = if self.rpc_disable_batch_requests { + RpcBatchRequestConfig::Disabled + } else if let Some(l) = self.rpc_max_batch_request_len { + RpcBatchRequestConfig::Limit(l) + } else { + RpcBatchRequestConfig::Unlimited + }; + + Ok(cfg) + } + fn rpc_rate_limit(&self) -> Result> { Ok(self.rpc_rate_limit) } diff --git a/substrate/client/cli/src/config.rs b/substrate/client/cli/src/config.rs index 78015cf837..5def9ce9b7 100644 --- a/substrate/client/cli/src/config.rs +++ b/substrate/client/cli/src/config.rs @@ -28,7 +28,8 @@ use sc_service::{ config::{ BasePath, Configuration, DatabaseSource, KeystoreConfig, NetworkConfiguration, NodeKeyConfig, OffchainWorkerConfig, OutputFormat, PrometheusConfig, PruningMode, Role, - RpcMethods, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, + RpcBatchRequestConfig, RpcMethods, TelemetryEndpoints, TransactionPoolOptions, + WasmExecutionMethod, }, BlocksPruning, ChainSpec, TracingReceiver, }; @@ -338,7 +339,12 @@ pub trait CliConfiguration: Sized { Ok(RPC_DEFAULT_MESSAGE_CAPACITY_PER_CONN) } - /// Rate limit calls per minute. + /// RPC server batch request configuration. + fn rpc_batch_config(&self) -> Result { + Ok(RpcBatchRequestConfig::Unlimited) + } + + /// RPC rate limit configuration. fn rpc_rate_limit(&self) -> Result> { Ok(None) } @@ -515,6 +521,7 @@ pub trait CliConfiguration: Sized { rpc_max_subs_per_conn: self.rpc_max_subscriptions_per_connection()?, rpc_port: DCV::rpc_listen_port(), rpc_message_buffer_capacity: self.rpc_buffer_capacity_per_connection()?, + rpc_batch_config: self.rpc_batch_config()?, rpc_rate_limit: self.rpc_rate_limit()?, prometheus_config: self .prometheus_config(DCV::prometheus_listen_port(), &chain_spec)?, diff --git a/substrate/client/cli/src/runner.rs b/substrate/client/cli/src/runner.rs index b4937db71e..4201a0f406 100644 --- a/substrate/client/cli/src/runner.rs +++ b/substrate/client/cli/src/runner.rs @@ -271,6 +271,7 @@ mod tests { rpc_max_subs_per_conn: Default::default(), rpc_message_buffer_capacity: Default::default(), rpc_port: 9944, + rpc_batch_config: sc_service::config::RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None, diff --git a/substrate/client/rpc-servers/src/lib.rs b/substrate/client/rpc-servers/src/lib.rs index a22b7309ac..e65d954bed 100644 --- a/substrate/client/rpc-servers/src/lib.rs +++ b/substrate/client/rpc-servers/src/lib.rs @@ -47,7 +47,7 @@ pub use jsonrpsee::{ id_providers::{RandomIntegerIdProvider, RandomStringIdProvider}, traits::IdProvider, }, - server::middleware::rpc::RpcServiceBuilder, + server::{middleware::rpc::RpcServiceBuilder, BatchRequestConfig}, }; pub use middleware::{MetricsLayer, RateLimitLayer, RpcMetrics}; @@ -81,6 +81,8 @@ pub struct Config<'a, M: Send + Sync + 'static> { pub id_provider: Option>, /// Tokio runtime handle. pub tokio_handle: tokio::runtime::Handle, + /// Batch request config. + pub batch_config: BatchRequestConfig, /// Rate limit calls per minute. pub rate_limit: Option, } @@ -103,6 +105,7 @@ where { let Config { addrs, + batch_config, cors, max_payload_in_mb, max_payload_out_mb, @@ -139,6 +142,7 @@ where ) .set_http_middleware(http_middleware) .set_message_buffer_capacity(message_buffer_capacity) + .set_batch_request_config(batch_config) .custom_tokio_runtime(tokio_handle.clone()); if let Some(provider) = id_provider { diff --git a/substrate/client/service/src/config.rs b/substrate/client/service/src/config.rs index 74c5dd775b..35262ff493 100644 --- a/substrate/client/service/src/config.rs +++ b/substrate/client/service/src/config.rs @@ -18,6 +18,7 @@ //! Service configuration. +pub use jsonrpsee::server::BatchRequestConfig as RpcBatchRequestConfig; use prometheus_endpoint::Registry; use sc_chain_spec::ChainSpec; pub use sc_client_db::{BlocksPruning, Database, DatabaseSource, PruningMode}; @@ -103,6 +104,8 @@ pub struct Configuration { pub rpc_port: u16, /// The number of messages the JSON-RPC server is allowed to keep in memory. pub rpc_message_buffer_capacity: u32, + /// JSON-RPC server batch config. + pub rpc_batch_config: RpcBatchRequestConfig, /// RPC rate limit per minute. pub rpc_rate_limit: Option, /// Prometheus endpoint configuration. `None` if disabled. diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index d0adf4f7d5..9480d4a0b0 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -393,6 +393,7 @@ where let server_config = sc_rpc_server::Config { addrs: [addr, backup_addr], + batch_config: config.rpc_batch_config, max_connections: config.rpc_max_connections, max_payload_in_mb: config.rpc_max_request_size, max_payload_out_mb: config.rpc_max_response_size, diff --git a/substrate/client/service/test/src/lib.rs b/substrate/client/service/test/src/lib.rs index 6148bb05fc..349538965e 100644 --- a/substrate/client/service/test/src/lib.rs +++ b/substrate/client/service/test/src/lib.rs @@ -29,7 +29,7 @@ use sc_network::{ use sc_network_sync::SyncingService; use sc_service::{ client::Client, - config::{BasePath, DatabaseSource, KeystoreConfig}, + config::{BasePath, DatabaseSource, KeystoreConfig, RpcBatchRequestConfig}, BlocksPruning, ChainSpecExtension, Configuration, Error, GenericChainSpec, Role, RuntimeGenesis, SpawnTaskHandle, TaskManager, }; @@ -254,6 +254,7 @@ fn node_config< rpc_max_subs_per_conn: Default::default(), rpc_port: 9944, rpc_message_buffer_capacity: Default::default(), + rpc_batch_config: RpcBatchRequestConfig::Unlimited, rpc_rate_limit: None, prometheus_config: None, telemetry_endpoints: None,