Make it possible to override maximum payload of RPC (#9019)

* Make it possible to override maximum payload of RPC

* Finish it.

* remove todo.

* Update client/cli/src/commands/run_cmd.rs

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>

* Apply suggestions from code review

Co-authored-by: David <dvdplm@gmail.com>

* Incorporate suggestions

* Thread rpc_max_payload from configuration to trace_block

* Try obey line gitlab/check_line_width.sh

* update state rpc tests

* Improve readbility

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>

substrate/client/cli/src/commands/run_cmd.rs

@@ -42,12 +42,11 @@ pub struct RunCmd {
 	/// The node will be started with the authority role and actively
 	/// participate in any consensus task that it can (e.g. depending on
 	/// availability of local keys).
-	#[structopt(
-		long = "validator"
-	)]
+	#[structopt(long)]
 	pub validator: bool,
 
-	/// Disable GRANDPA voter when running in validator mode, otherwise disable the GRANDPA observer.
+	/// Disable GRANDPA voter when running in validator mode, otherwise disable the GRANDPA
+	/// observer.
 	#[structopt(long)]
 	pub no_grandpa: bool,
 
@@ -57,8 +56,8 @@ pub struct RunCmd {
 
 	/// Listen to all RPC interfaces.
 	///
-	/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC proxy
-	/// server to filter out dangerous methods. More details:
+	/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC
+	/// proxy server to filter out dangerous methods. More details:
 	/// <https://github.com/paritytech/substrate/wiki/Public-RPC>.
 	/// Use `--unsafe-rpc-external` to suppress the warning if you understand the risks.
 	#[structopt(long = "rpc-external")]
@@ -74,8 +73,8 @@ pub struct RunCmd {
 	///
 	/// - `Unsafe`: Exposes every RPC method.
 	/// - `Safe`: Exposes only a safe subset of RPC methods, denying unsafe RPC methods.
-	/// - `Auto`: Acts as `Safe` if RPC is served externally, e.g. when `--{rpc,ws}-external` is passed,
-	///   otherwise acts as `Unsafe`.
+	/// - `Auto`: Acts as `Safe` if RPC is served externally, e.g. when `--{rpc,ws}-external` is
+	///   passed, otherwise acts as `Unsafe`.
 	#[structopt(
 		long,
 		value_name = "METHOD SET",
@@ -88,8 +87,9 @@ pub struct RunCmd {
 
 	/// Listen to all Websocket interfaces.
 	///
-	/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC proxy
-	/// server to filter out dangerous methods. More details: <https://github.com/paritytech/substrate/wiki/Public-RPC>.
+	/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC
+	/// proxy server to filter out dangerous methods. More details:
+	/// <https://github.com/paritytech/substrate/wiki/Public-RPC>.
 	/// Use `--unsafe-ws-external` to suppress the warning if you understand the risks.
 	#[structopt(long = "ws-external")]
 	pub ws_external: bool,
@@ -100,6 +100,11 @@ pub struct RunCmd {
 	#[structopt(long = "unsafe-ws-external")]
 	pub unsafe_ws_external: bool,
 
+	/// Set the the maximum RPC payload size for both requests and responses (both http and ws), in
+	/// megabytes. Default is 15MiB.
+	#[structopt(long = "rpc-max-payload")]
+	pub rpc_max_payload: Option<usize>,
+
 	/// Listen to all Prometheus data source interfaces.
 	///
 	/// Default is local.
@@ -194,7 +199,8 @@ pub struct RunCmd {
 	#[structopt(long, conflicts_with_all = &["alice", "charlie", "dave", "eve", "ferdie", "one", "two"])]
 	pub bob: bool,
 
-	/// Shortcut for `--name Charlie --validator` with session keys for `Charlie` added to keystore.
+	/// Shortcut for `--name Charlie --validator` with session keys for `Charlie` added to
+	/// keystore.
 	#[structopt(long, conflicts_with_all = &["alice", "bob", "dave", "eve", "ferdie", "one", "two"])]
 	pub charlie: bool,
 
@@ -435,6 +441,10 @@ impl CliConfiguration for RunCmd {
 		Ok(self.rpc_methods.into())
 	}
 
+	fn rpc_max_payload(&self) -> Result<Option<usize>> {
+		Ok(self.rpc_max_payload)
+	}
+
 	fn transaction_pool(&self) -> Result<TransactionPoolOptions> {
 		Ok(self.pool_config.transaction_pool())
 	}

substrate/client/cli/src/config.rs

@@ -372,6 +372,11 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		Ok(Some(Vec::new()))
 	}
 
+	/// Get maximum RPC payload.
+	fn rpc_max_payload(&self) -> Result<Option<usize>> {
+		Ok(None)
+	}
+
 	/// Get the prometheus configuration (`None` if disabled)
 	///
 	/// By default this is `None`.
@@ -535,6 +540,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 			rpc_ws_max_connections: self.rpc_ws_max_connections()?,
 			rpc_http_threads: self.rpc_http_threads()?,
 			rpc_cors: self.rpc_cors(is_dev)?,
+			rpc_max_payload: self.rpc_max_payload()?,
 			prometheus_config: self.prometheus_config(DCV::prometheus_listen_port())?,
 			telemetry_endpoints,
 			telemetry_external_transport: self.telemetry_external_transport()?,

substrate/client/executor/src/native_executor.rs

@@ -291,7 +291,7 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
 		default_heap_pages: Option<u64>,
 		max_runtime_instances: usize,
 	) -> Self {
-		let extended =  D::ExtendHostFunctions::host_functions();
+		let extended = D::ExtendHostFunctions::host_functions();
 		let mut host_functions = sp_io::SubstrateHostFunctions::host_functions()
 			.into_iter()
 			// filter out any host function overrides provided.

substrate/client/rpc-servers/src/lib.rs

@@ -27,8 +27,10 @@ use jsonrpc_core::{IoHandlerExtension, MetaIoHandler};
 use log::error;
 use pubsub::PubSubMetadata;
 
+const MEGABYTE: usize = 1024 * 1024;
+
 /// Maximal payload accepted by RPC servers.
-pub const MAX_PAYLOAD: usize = 15 * 1024 * 1024;
+pub const RPC_MAX_PAYLOAD_DEFAULT: usize = 15 * MEGABYTE;
 
 /// Default maximum number of connections for WS RPC servers.
 const WS_MAX_CONNECTIONS: usize = 100;
@@ -85,7 +87,10 @@ mod inner {
 		thread_pool_size: Option<usize>,
 		cors: Option<&Vec<String>>,
 		io: RpcHandler<M>,
+		maybe_max_payload_mb: Option<usize>,
 	) -> io::Result<http::Server> {
+		let max_request_body_size = maybe_max_payload_mb.map(|mb| mb.saturating_mul(MEGABYTE))
+			.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);
 		http::ServerBuilder::new(io)
 			.threads(thread_pool_size.unwrap_or(HTTP_THREADS))
 			.health_api(("/health", "system_health"))
@@ -96,7 +101,7 @@ mod inner {
 				http::RestApi::Unsecure
 			})
 			.cors(map_cors::<http::AccessControlAllowOrigin>(cors))
-			.max_request_body_size(MAX_PAYLOAD)
+			.max_request_body_size(max_request_body_size)
 			.start_http(addr)
 	}
 
@@ -120,14 +125,19 @@ mod inner {
 	/// Start WS server listening on given address.
 	///
 	/// **Note**: Only available if `not(target_os = "unknown")`.
-	pub fn start_ws<M: pubsub::PubSubMetadata + From<jsonrpc_core::futures::sync::mpsc::Sender<String>>> (
+	pub fn start_ws<
+		M: pubsub::PubSubMetadata + From<jsonrpc_core::futures::sync::mpsc::Sender<String>>,
+	>(
 		addr: &std::net::SocketAddr,
 		max_connections: Option<usize>,
 		cors: Option<&Vec<String>>,
 		io: RpcHandler<M>,
+		maybe_max_payload_mb: Option<usize>,
 	) -> io::Result<ws::Server> {
+		let rpc_max_payload = maybe_max_payload_mb.map(|mb| mb.saturating_mul(MEGABYTE))
+			.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);
 		ws::ServerBuilder::with_meta_extractor(io, |context: &ws::RequestContext| context.sender().into())
-			.max_payload(MAX_PAYLOAD)
+			.max_payload(rpc_max_payload)
 			.max_connections(max_connections.unwrap_or(WS_MAX_CONNECTIONS))
 			.allowed_origins(map_cors(cors))
 			.allowed_hosts(hosts_filtering(cors.is_some()))

substrate/client/rpc/src/state/mod.rs

@@ -182,6 +182,7 @@ pub fn new_full<BE, Block: BlockT, Client>(
 	client: Arc<Client>,
 	subscriptions: SubscriptionManager,
 	deny_unsafe: DenyUnsafe,
+	rpc_max_payload: Option<usize>,
 ) -> (State<Block, Client>, ChildState<Block, Client>)
 	where
 		Block: BlockT + 'static,
@@ -193,9 +194,11 @@ pub fn new_full<BE, Block: BlockT, Client>(
 		Client::Api: Metadata<Block>,
 {
 	let child_backend = Box::new(
-		self::state_full::FullState::new(client.clone(), subscriptions.clone())
+		self::state_full::FullState::new(
+			client.clone(), subscriptions.clone(), rpc_max_payload
+		)
 	);
-	let backend = Box::new(self::state_full::FullState::new(client, subscriptions));
+	let backend = Box::new(self::state_full::FullState::new(client, subscriptions, rpc_max_payload));
 	(State { backend, deny_unsafe }, ChildState { backend: child_backend })
 }
 

substrate/client/rpc/src/state/state_full.rs

@@ -67,7 +67,8 @@ struct QueryStorageRange<Block: BlockT> {
 pub struct FullState<BE, Block: BlockT, Client> {
 	client: Arc<Client>,
 	subscriptions: SubscriptionManager,
-	_phantom: PhantomData<(BE, Block)>
+	_phantom: PhantomData<(BE, Block)>,
+	rpc_max_payload: Option<usize>,
 }
 
 impl<BE, Block: BlockT, Client> FullState<BE, Block, Client>
@@ -78,8 +79,12 @@ impl<BE, Block: BlockT, Client> FullState<BE, Block, Client>
 		Block: BlockT + 'static,
 {
 	/// Create new state API backend for full nodes.
-	pub fn new(client: Arc<Client>, subscriptions: SubscriptionManager) -> Self {
-		Self { client, subscriptions, _phantom: PhantomData }
+	pub fn new(
+		client: Arc<Client>,
+		subscriptions: SubscriptionManager,
+		rpc_max_payload: Option<usize>,
+	) -> Self {
+		Self { client, subscriptions, _phantom: PhantomData, rpc_max_payload }
 	}
 
 	/// Returns given block hash or best block hash if None is passed.
@@ -540,9 +545,15 @@ impl<BE, Block, Client> StateBackend<Block, Client> for FullState<BE, Block, Cli
 		targets: Option<String>,
 		storage_keys: Option<String>,
 	) -> FutureResult<sp_rpc::tracing::TraceBlockResponse> {
+		let block_executor = sc_tracing::block::BlockExecutor::new(
+			self.client.clone(),
+			block,
+			targets,
+			storage_keys,
+			self.rpc_max_payload,
+		);
 		Box::new(result(
-			sc_tracing::block::BlockExecutor::new(self.client.clone(), block, targets, storage_keys)
-				.trace_block()
+			block_executor.trace_block()
 				.map_err(|e| invalid_block::<Block>(block, None, e.to_string()))
 		))
 	}

substrate/client/rpc/src/state/tests.rs

@@ -63,6 +63,7 @@ fn should_return_storage() {
 		Arc::new(client),
 		SubscriptionManager::new(Arc::new(TaskExecutor)),
 		DenyUnsafe::No,
+		None,
 	);
 	let key = StorageKey(KEY.to_vec());
 
@@ -105,6 +106,7 @@ fn should_return_child_storage() {
 		client,
 		SubscriptionManager::new(Arc::new(TaskExecutor)),
 		DenyUnsafe::No,
+		None,
 	);
 	let child_key = prefixed_storage_key();
 	let key = StorageKey(b"key".to_vec());
@@ -144,6 +146,7 @@ fn should_call_contract() {
 		client,
 		SubscriptionManager::new(Arc::new(TaskExecutor)),
 		DenyUnsafe::No,
+		None,
 	);
 
 	assert_matches!(
@@ -162,6 +165,7 @@ fn should_notify_about_storage_changes() {
 			client.clone(),
 			SubscriptionManager::new(Arc::new(TaskExecutor)),
 			DenyUnsafe::No,
+			None,
 		);
 
 		api.subscribe_storage(Default::default(), subscriber, None.into());
@@ -200,6 +204,7 @@ fn should_send_initial_storage_changes_and_notifications() {
 			client.clone(),
 			SubscriptionManager::new(Arc::new(TaskExecutor)),
 			DenyUnsafe::No,
+			None,
 		);
 
 		let alice_balance_key = blake2_256(&runtime::system::balance_of_key(AccountKeyring::Alice.into()));
@@ -242,6 +247,7 @@ fn should_query_storage() {
 			client.clone(),
 			SubscriptionManager::new(Arc::new(TaskExecutor)),
 			DenyUnsafe::No,
+			None,
 		);
 
 		let mut add_block = |nonce| {
@@ -463,6 +469,7 @@ fn should_return_runtime_version() {
 		client.clone(),
 		SubscriptionManager::new(Arc::new(TaskExecutor)),
 		DenyUnsafe::No,
+		None,
 	);
 
 	let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
@@ -490,6 +497,7 @@ fn should_notify_on_runtime_version_initially() {
 			client.clone(),
 			SubscriptionManager::new(Arc::new(TaskExecutor)),
 			DenyUnsafe::No,
+			None,
 		);
 
 		api.subscribe_runtime_version(Default::default(), subscriber);

substrate/client/service/src/builder.rs

@@ -804,6 +804,7 @@ fn gen_handler<TBl, TBackend, TExPool, TRpc, TCl>(
 			client.clone(),
 			subscriptions.clone(),
 			deny_unsafe,
+			config.rpc_max_payload,
 		);
 		(chain, state, child_state)
 	};

substrate/client/service/src/config.rs

@@ -95,6 +95,8 @@ pub struct Configuration {
 	pub rpc_cors: Option<Vec<String>>,
 	/// RPC methods to expose (by default only a safe subset or all of them).
 	pub rpc_methods: RpcMethods,
+	/// Maximum payload of rpc request/responses.
+	pub rpc_max_payload: Option<usize>,
 	/// Prometheus endpoint configuration. `None` if disabled.
 	pub prometheus_config: Option<PrometheusConfig>,
 	/// Telemetry service URL. `None` if disabled.

substrate/client/service/src/lib.rs

@@ -387,6 +387,7 @@ fn start_rpc_servers<
 					deny_unsafe(&address, &config.rpc_methods),
 					sc_rpc_server::RpcMiddleware::new(rpc_metrics.clone(), "http")
 				),
+				config.rpc_max_payload
 			),
 		)?.map(|s| waiting::HttpServer(Some(s))),
 		maybe_start_server(
@@ -399,6 +400,7 @@ fn start_rpc_servers<
 					deny_unsafe(&address, &config.rpc_methods),
 					sc_rpc_server::RpcMiddleware::new(rpc_metrics.clone(), "ws")
 				),
+				config.rpc_max_payload
 			),
 		)?.map(|s| waiting::WsServer(Some(s))),
 	)))

substrate/client/service/test/src/lib.rs

@@ -265,6 +265,7 @@ fn node_config<G: RuntimeGenesis + 'static, E: ChainSpecExtension + Clone + 'sta
 		rpc_http_threads: None,
 		rpc_cors: None,
 		rpc_methods: Default::default(),
+		rpc_max_payload: None,
 		prometheus_config: None,
 		telemetry_endpoints: None,
 		telemetry_external_transport: None,

substrate/client/tracing/src/block/mod.rs

@@ -22,7 +22,7 @@ use parking_lot::Mutex;
 use tracing::{Dispatch, dispatcher, Subscriber, Level, span::{Attributes, Record, Id}};
 
 use sc_client_api::BlockBackend;
-use sc_rpc_server::MAX_PAYLOAD;
+use sc_rpc_server::RPC_MAX_PAYLOAD_DEFAULT;
 use sp_api::{Core, Metadata, ProvideRuntimeApi, Encode};
 use sp_blockchain::HeaderBackend;
 use sp_runtime::{
@@ -54,6 +54,7 @@ const DEFAULT_TARGETS: &str = "pallet,frame,state";
 const TRACE_TARGET: &str = "block_trace";
 // The name of a field required for all events.
 const REQUIRED_EVENT_FIELD: &str  = "method";
+const MEGABYTE: usize = 1024 * 1024;
 
 /// Tracing Block Result type alias
 pub type TraceBlockResult<T> = Result<T, Error>;
@@ -174,6 +175,7 @@ pub struct BlockExecutor<Block: BlockT, Client> {
 	block: Block::Hash,
 	targets: Option<String>,
 	storage_keys: Option<String>,
+	rpc_max_payload: usize,
 }
 
 impl<Block, Client> BlockExecutor<Block, Client>
@@ -189,8 +191,11 @@ impl<Block, Client> BlockExecutor<Block, Client>
 		block: Block::Hash,
 		targets: Option<String>,
 		storage_keys: Option<String>,
+		rpc_max_payload: Option<usize>,
 	) -> Self {
-		Self { client, block, targets, storage_keys }
+		let rpc_max_payload = rpc_max_payload.map(|mb| mb.saturating_mul(MEGABYTE))
+			.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);
+		Self { client, block, targets, storage_keys, rpc_max_payload }
 	}
 
 	/// Execute block, record all spans and events belonging to `Self::targets`
@@ -260,7 +265,7 @@ impl<Block, Client> BlockExecutor<Block, Client>
 		tracing::debug!(target: "state_tracing", "Captured {} spans and {} events", spans.len(), events.len());
 
 		let approx_payload_size = BASE_PAYLOAD + events.len() * AVG_EVENT + spans.len() * AVG_SPAN;
-		let response = if approx_payload_size > MAX_PAYLOAD {
+		let response = if approx_payload_size > self.rpc_max_payload {
 				TraceBlockResponse::TraceError(TraceError {
 					error:
 						"Payload likely exceeds max payload size of RPC server.".to_string()

substrate/test-utils/test-runner/src/utils.rs

@@ -127,6 +127,7 @@ pub fn default_config(task_executor: TaskExecutor, mut chain_spec: Box<dyn Chain
 		rpc_http_threads: None,
 		rpc_cors: None,
 		rpc_methods: Default::default(),
+		rpc_max_payload: None,
 		prometheus_config: None,
 		telemetry_endpoints: None,
 		telemetry_external_transport: None,

substrate/utils/browser/src/lib.rs

@@ -103,6 +103,7 @@ where
 		rpc_ws_max_connections: Default::default(),
 		rpc_http_threads: Default::default(),
 		rpc_methods: Default::default(),
+		rpc_max_payload: Default::default(),
 		state_cache_child_ratio: Default::default(),
 		state_cache_size: Default::default(),
 		tracing_receiver: Default::default(),