refactor: state RPC remove max_rpc_payload_size (paritytech#13649)

This limit is not needed anymore as the JSON-RPC servers doesn't terminate
the connection if the RPC max limit is exceeded anymore

client/rpc/src/state/mod.rs

@@ -169,7 +169,6 @@ pub fn new_full<BE, Block: BlockT, Client>(
 	client: Arc<Client>,
 	executor: SubscriptionTaskExecutor,
 	deny_unsafe: DenyUnsafe,
-	rpc_max_payload: Option<usize>,
 ) -> (State<Block, Client>, ChildState<Block, Client>)
 where
 	Block: BlockT + 'static,
@@ -189,12 +188,9 @@ where
 		+ 'static,
 	Client::Api: Metadata<Block>,
 {
-	let child_backend = Box::new(self::state_full::FullState::new(
-		client.clone(),
-		executor.clone(),
-		rpc_max_payload,
-	));
-	let backend = Box::new(self::state_full::FullState::new(client, executor, rpc_max_payload));
+	let child_backend =
+		Box::new(self::state_full::FullState::new(client.clone(), executor.clone()));
+	let backend = Box::new(self::state_full::FullState::new(client, executor));
 	(State { backend, deny_unsafe }, ChildState { backend: child_backend })
 }
 

client/rpc/src/state/state_full.rs

@@ -66,7 +66,6 @@ pub struct FullState<BE, Block: BlockT, Client> {
 	client: Arc<Client>,
 	executor: SubscriptionTaskExecutor,
 	_phantom: PhantomData<(BE, Block)>,
-	rpc_max_payload: Option<usize>,
 }
 
 impl<BE, Block: BlockT, Client> FullState<BE, Block, Client>
@@ -79,12 +78,8 @@ where
 	Block: BlockT + 'static,
 {
 	/// Create new state API backend for full nodes.
-	pub fn new(
-		client: Arc<Client>,
-		executor: SubscriptionTaskExecutor,
-		rpc_max_payload: Option<usize>,
-	) -> Self {
-		Self { client, executor, _phantom: PhantomData, rpc_max_payload }
+	pub fn new(client: Arc<Client>, executor: SubscriptionTaskExecutor) -> Self {
+		Self { client, executor, _phantom: PhantomData }
 	}
 
 	/// Returns given block hash or best block hash if None is passed.
@@ -481,7 +476,6 @@ where
 			targets,
 			storage_keys,
 			methods,
-			self.rpc_max_payload,
 		)
 		.trace_block()
 		.map_err(|e| invalid_block::<Block>(block, None, e.to_string()))

client/rpc/src/state/tests.rs

@@ -55,7 +55,7 @@ async fn should_return_storage() {
 		.add_extra_storage(b":map:acc2".to_vec(), vec![1, 2, 3])
 		.build();
 	let genesis_hash = client.genesis_hash();
-	let (client, child) = new_full(Arc::new(client), test_executor(), DenyUnsafe::No, None);
+	let (client, child) = new_full(Arc::new(client), test_executor(), DenyUnsafe::No);
 	let key = StorageKey(KEY.to_vec());
 
 	assert_eq!(
@@ -103,7 +103,7 @@ async fn should_return_storage_entries() {
 		.add_extra_child_storage(&child_info, KEY2.to_vec(), CHILD_VALUE2.to_vec())
 		.build();
 	let genesis_hash = client.genesis_hash();
-	let (_client, child) = new_full(Arc::new(client), test_executor(), DenyUnsafe::No, None);
+	let (_client, child) = new_full(Arc::new(client), test_executor(), DenyUnsafe::No);
 
 	let keys = &[StorageKey(KEY1.to_vec()), StorageKey(KEY2.to_vec())];
 	assert_eq!(
@@ -134,7 +134,7 @@ async fn should_return_child_storage() {
 			.build(),
 	);
 	let genesis_hash = client.genesis_hash();
-	let (_client, child) = new_full(client, test_executor(), DenyUnsafe::No, None);
+	let (_client, child) = new_full(client, test_executor(), DenyUnsafe::No);
 	let child_key = prefixed_storage_key();
 	let key = StorageKey(b"key".to_vec());
 
@@ -165,7 +165,7 @@ async fn should_return_child_storage_entries() {
 			.build(),
 	);
 	let genesis_hash = client.genesis_hash();
-	let (_client, child) = new_full(client, test_executor(), DenyUnsafe::No, None);
+	let (_client, child) = new_full(client, test_executor(), DenyUnsafe::No);
 	let child_key = prefixed_storage_key();
 	let keys = vec![StorageKey(b"key1".to_vec()), StorageKey(b"key2".to_vec())];
 
@@ -196,7 +196,7 @@ async fn should_return_child_storage_entries() {
 async fn should_call_contract() {
 	let client = Arc::new(substrate_test_runtime_client::new());
 	let genesis_hash = client.genesis_hash();
-	let (client, _child) = new_full(client, test_executor(), DenyUnsafe::No, None);
+	let (client, _child) = new_full(client, test_executor(), DenyUnsafe::No);
 
 	use jsonrpsee::{core::Error, types::error::CallError};
 
@@ -210,7 +210,7 @@ async fn should_call_contract() {
 async fn should_notify_about_storage_changes() {
 	let mut sub = {
 		let mut client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No, None);
+		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No);
 
 		let api_rpc = api.into_rpc();
 		let sub = api_rpc.subscribe("state_subscribeStorage", EmptyParams::new()).await.unwrap();
@@ -242,7 +242,7 @@ async fn should_notify_about_storage_changes() {
 async fn should_send_initial_storage_changes_and_notifications() {
 	let mut sub = {
 		let mut client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No, None);
+		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No);
 
 		let alice_balance_key =
 			blake2_256(&runtime::system::balance_of_key(AccountKeyring::Alice.into()));
@@ -278,7 +278,7 @@ async fn should_send_initial_storage_changes_and_notifications() {
 #[tokio::test]
 async fn should_query_storage() {
 	async fn run_tests(mut client: Arc<TestClient>) {
-		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No, None);
+		let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No);
 
 		let mut add_block = |nonce| {
 			let mut builder = client.new_block(Default::default()).unwrap();
@@ -480,7 +480,7 @@ async fn should_query_storage() {
 #[tokio::test]
 async fn should_return_runtime_version() {
 	let client = Arc::new(substrate_test_runtime_client::new());
-	let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No, None);
+	let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No);
 
 	let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
 		\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\
@@ -501,7 +501,7 @@ async fn should_return_runtime_version() {
 async fn should_notify_on_runtime_version_initially() {
 	let mut sub = {
 		let client = Arc::new(substrate_test_runtime_client::new());
-		let (api, _child) = new_full(client, test_executor(), DenyUnsafe::No, None);
+		let (api, _child) = new_full(client, test_executor(), DenyUnsafe::No);
 
 		let api_rpc = api.into_rpc();
 		let sub = api_rpc
@@ -530,7 +530,7 @@ fn should_deserialize_storage_key() {
 #[tokio::test]
 async fn wildcard_storage_subscriptions_are_rpc_unsafe() {
 	let client = Arc::new(substrate_test_runtime_client::new());
-	let (api, _child) = new_full(client, test_executor(), DenyUnsafe::Yes, None);
+	let (api, _child) = new_full(client, test_executor(), DenyUnsafe::Yes);
 
 	let api_rpc = api.into_rpc();
 	let err = api_rpc.subscribe("state_subscribeStorage", EmptyParams::new()).await;
@@ -540,7 +540,7 @@ async fn wildcard_storage_subscriptions_are_rpc_unsafe() {
 #[tokio::test]
 async fn concrete_storage_subscriptions_are_rpc_safe() {
 	let client = Arc::new(substrate_test_runtime_client::new());
-	let (api, _child) = new_full(client, test_executor(), DenyUnsafe::Yes, None);
+	let (api, _child) = new_full(client, test_executor(), DenyUnsafe::Yes);
 	let api_rpc = api.into_rpc();
 
 	let key = StorageKey(STORAGE_KEY.to_vec());

client/service/src/builder.rs

@@ -629,12 +629,8 @@ where
 
 	let (chain, state, child_state) = {
 		let chain = sc_rpc::chain::new_full(client.clone(), task_executor.clone()).into_rpc();
-		let (state, child_state) = sc_rpc::state::new_full(
-			client.clone(),
-			task_executor.clone(),
-			deny_unsafe,
-			config.rpc_max_payload,
-		);
+		let (state, child_state) =
+			sc_rpc::state::new_full(client.clone(), task_executor.clone(), deny_unsafe);
 		let state = state.into_rpc();
 		let child_state = child_state.into_rpc();
 

client/tracing/src/block/mod.rs

@@ -34,38 +34,21 @@ use tracing::{
 
 use crate::{SpanDatum, TraceEvent, Values};
 use sc_client_api::BlockBackend;
-use sc_rpc_server::RPC_MAX_PAYLOAD_DEFAULT;
 use sp_api::{Core, Encode, Metadata, ProvideRuntimeApi};
 use sp_blockchain::HeaderBackend;
 use sp_core::hexdisplay::HexDisplay;
-use sp_rpc::tracing::{BlockTrace, Span, TraceBlockResponse, TraceError};
+use sp_rpc::tracing::{BlockTrace, Span, TraceBlockResponse};
 use sp_runtime::{
 	generic::BlockId,
 	traits::{Block as BlockT, Header},
 };
 use sp_tracing::{WASM_NAME_KEY, WASM_TARGET_KEY, WASM_TRACE_IDENTIFIER};
 
-// Heuristic for average event size in bytes.
-const AVG_EVENT: usize = 600 * 8;
-// Heuristic for average span size in bytes.
-const AVG_SPAN: usize = 100 * 8;
-// Estimate of the max base RPC payload size when the Id is bound as a u64. If strings
-// are used for the RPC Id this may need to be adjusted. Note: The base payload
-// does not include the RPC result.
-//
-// The estimate is based on the JSON-RPC response message which has the following format:
-// `{"jsonrpc":"2.0","result":[],"id":18446744073709551615}`.
-//
-// We care about the total size of the payload because jsonrpc-server will simply ignore
-// messages larger than `sc_rpc_server::MAX_PAYLOAD` and the caller will not get any
-// response.
-const BASE_PAYLOAD: usize = 100;
 // Default to only pallet, frame support and state related traces
 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>;
@@ -182,7 +165,6 @@ pub struct BlockExecutor<Block: BlockT, Client> {
 	targets: Option<String>,
 	storage_keys: Option<String>,
 	methods: Option<String>,
-	rpc_max_payload: usize,
 }
 
 impl<Block, Client> BlockExecutor<Block, Client>
@@ -203,12 +185,8 @@ where
 		targets: Option<String>,
 		storage_keys: Option<String>,
 		methods: Option<String>,
-		rpc_max_payload: Option<usize>,
 	) -> Self {
-		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, methods, rpc_max_payload }
+		Self { client, block, targets, storage_keys, methods }
 	}
 
 	/// Execute block, record all spans and events belonging to `Self::targets`
@@ -289,24 +267,15 @@ where
 			.collect();
 		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 > self.rpc_max_payload {
-			TraceBlockResponse::TraceError(TraceError {
-				error: "Payload likely exceeds max payload size of RPC server.".to_string(),
-			})
-		} else {
-			TraceBlockResponse::BlockTrace(BlockTrace {
-				block_hash: block_id_as_string(BlockId::<Block>::Hash(self.block)),
-				parent_hash: block_id_as_string(BlockId::<Block>::Hash(parent_hash)),
-				tracing_targets: targets.to_string(),
-				storage_keys: self.storage_keys.clone().unwrap_or_default(),
-				methods: self.methods.clone().unwrap_or_default(),
-				spans,
-				events,
-			})
-		};
-
-		Ok(response)
+		Ok(TraceBlockResponse::BlockTrace(BlockTrace {
+			block_hash: block_id_as_string(BlockId::<Block>::Hash(self.block)),
+			parent_hash: block_id_as_string(BlockId::<Block>::Hash(parent_hash)),
+			tracing_targets: targets.to_string(),
+			storage_keys: self.storage_keys.clone().unwrap_or_default(),
+			methods: self.methods.clone().unwrap_or_default(),
+			spans,
+			events,
+		}))
 	}
 }