From ecdf34392527e5b3fd50879bb57f8d860eafabe3 Mon Sep 17 00:00:00 2001
From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
Date: Wed, 29 Nov 2023 20:15:33 +0200
Subject: [PATCH] chainHead: Backport error codes from spec (#2539)

This PR backports the error codes from the spec.

This relies on two specs for defining the error codes:
- Our rpc-spec-v2 https://github.com/paritytech/json-rpc-interface-spec.
- JSON-RPC spec https://www.jsonrpc.org/specification#error_object.

To better describe the error codes, they are divided into two separate
modules `rpc_spec_v2` and `json_rpc_spec` respectively.

The `InvalidSubscriptionID` and `FetchBlockHeader` are merged into the
JSON-RPC spec `INTERNAL_ERROR`.
While the other error codes are adjusted from spec.

Errors that are currently in use:
- -32801 block hash not reported by chainHead_follow or block hash has
been unpinned
- -32802 chainHead_follow started with withRuntime == false
- -32803 chainHead_follow did not generate an
operationWaitingForContinue event

The following are errors defined in the [JSON-RPC
spec](https://www.jsonrpc.org/specification#error_object):
- -32602 The provided parameter isn't one of the expected values, has
different format or is missing
- -32603 Internal server error

Note: Error `-32801` must be introduced and generated by the outstanding
https://github.com/paritytech/polkadot-sdk/issues/1505

Closes: https://github.com/paritytech/polkadot-sdk/issues/2530

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
---
 .../rpc-spec-v2/src/chain_head/chain_head.rs  |  8 ++-
 .../rpc-spec-v2/src/chain_head/error.rs       | 64 +++++++++++--------
 .../rpc-spec-v2/src/chain_head/tests.rs       | 24 +++----
 3 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs
index 866701a7dbf..8e04ac7b177 100644
--- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs
+++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head.rs
@@ -198,7 +198,9 @@ where
 		let sub_id = match self.accept_subscription(&mut sink) {
 			Ok(sub_id) => sub_id,
 			Err(err) => {
-				sink.close(ChainHeadRpcError::InvalidSubscriptionID);
+				sink.close(ChainHeadRpcError::InternalError(
+					"Cannot generate subscription ID".into(),
+				));
 				return Err(err)
 			},
 		};
@@ -306,7 +308,7 @@ where
 		self.client
 			.header(hash)
 			.map(|opt_header| opt_header.map(|h| hex_string(&h.encode())))
-			.map_err(ChainHeadRpcError::FetchBlockHeader)
+			.map_err(|err| ChainHeadRpcError::InternalError(err.to_string()))
 			.map_err(Into::into)
 	}
 
@@ -393,7 +395,7 @@ where
 
 		// Reject subscription if with_runtime is false.
 		if !block_guard.has_runtime() {
-			return Err(ChainHeadRpcError::InvalidParam(
+			return Err(ChainHeadRpcError::InvalidRuntimeCall(
 				"The runtime updates flag must be set".to_string(),
 			)
 			.into())
diff --git a/substrate/client/rpc-spec-v2/src/chain_head/error.rs b/substrate/client/rpc-spec-v2/src/chain_head/error.rs
index 811666428c5..a9b7d7f96e4 100644
--- a/substrate/client/rpc-spec-v2/src/chain_head/error.rs
+++ b/substrate/client/rpc-spec-v2/src/chain_head/error.rs
@@ -22,7 +22,6 @@ use jsonrpsee::{
 	core::Error as RpcError,
 	types::error::{CallError, ErrorObject},
 };
-use sp_blockchain::Error as BlockchainError;
 
 /// ChainHead RPC errors.
 #[derive(Debug, thiserror::Error)]
@@ -30,44 +29,55 @@ pub enum Error {
 	/// The provided block hash is invalid.
 	#[error("Invalid block hash")]
 	InvalidBlock,
-	/// Fetch block header error.
-	#[error("Could not fetch block header: {0}")]
-	FetchBlockHeader(BlockchainError),
+	/// The follow subscription was started with `withRuntime` set to `false`.
+	#[error("The `chainHead_follow` subscription was started with `withRuntime` set to `false`")]
+	InvalidRuntimeCall(String),
+	/// Wait-for-continue event not generated.
+	#[error("Wait for continue event was not generated for the subscription")]
+	InvalidContinue,
 	/// Invalid parameter provided to the RPC method.
 	#[error("Invalid parameter: {0}")]
 	InvalidParam(String),
-	/// Invalid subscription ID provided by the RPC server.
-	#[error("Invalid subscription ID")]
-	InvalidSubscriptionID,
+	/// Internal error.
+	#[error("Internal error: {0}")]
+	InternalError(String),
+}
+
+/// Errors for `chainHead` RPC module, as defined in
+/// <https://github.com/paritytech/json-rpc-interface-spec>.
+pub mod rpc_spec_v2 {
+	/// The provided block hash is invalid.
+	pub const INVALID_BLOCK_ERROR: i32 = -32801;
+	/// The follow subscription was started with `withRuntime` set to `false`.
+	pub const INVALID_RUNTIME_CALL: i32 = -32802;
 	/// Wait-for-continue event not generated.
-	#[error("Wait for continue event was not generated for the subscription")]
-	InvalidContinue,
+	pub const INVALID_CONTINUE: i32 = -32803;
 }
 
-// Base code for all `chainHead` errors.
-const BASE_ERROR: i32 = 2000;
-/// The provided block hash is invalid.
-const INVALID_BLOCK_ERROR: i32 = BASE_ERROR + 1;
-/// Fetch block header error.
-const FETCH_BLOCK_HEADER_ERROR: i32 = BASE_ERROR + 2;
-/// Invalid parameter error.
-const INVALID_PARAM_ERROR: i32 = BASE_ERROR + 3;
-/// Invalid subscription ID.
-const INVALID_SUB_ID: i32 = BASE_ERROR + 4;
-/// Wait-for-continue event not generated.
-const INVALID_CONTINUE: i32 = BASE_ERROR + 5;
+/// General purpose errors, as defined in
+/// <https://www.jsonrpc.org/specification#error_object>.
+pub mod json_rpc_spec {
+	/// Invalid parameter error.
+	pub const INVALID_PARAM_ERROR: i32 = -32602;
+	/// Internal error.
+	pub const INTERNAL_ERROR: i32 = -32603;
+}
 
 impl From<Error> for ErrorObject<'static> {
 	fn from(e: Error) -> Self {
 		let msg = e.to_string();
 
 		match e {
-			Error::InvalidBlock => ErrorObject::owned(INVALID_BLOCK_ERROR, msg, None::<()>),
-			Error::FetchBlockHeader(_) =>
-				ErrorObject::owned(FETCH_BLOCK_HEADER_ERROR, msg, None::<()>),
-			Error::InvalidParam(_) => ErrorObject::owned(INVALID_PARAM_ERROR, msg, None::<()>),
-			Error::InvalidSubscriptionID => ErrorObject::owned(INVALID_SUB_ID, msg, None::<()>),
-			Error::InvalidContinue => ErrorObject::owned(INVALID_CONTINUE, msg, None::<()>),
+			Error::InvalidBlock =>
+				ErrorObject::owned(rpc_spec_v2::INVALID_BLOCK_ERROR, msg, None::<()>),
+			Error::InvalidRuntimeCall(_) =>
+				ErrorObject::owned(rpc_spec_v2::INVALID_RUNTIME_CALL, msg, None::<()>),
+			Error::InvalidContinue =>
+				ErrorObject::owned(rpc_spec_v2::INVALID_CONTINUE, msg, None::<()>),
+			Error::InvalidParam(_) =>
+				ErrorObject::owned(json_rpc_spec::INVALID_PARAM_ERROR, msg, None::<()>),
+			Error::InternalError(_) =>
+				ErrorObject::owned(json_rpc_spec::INTERNAL_ERROR, msg, None::<()>),
 		}
 	}
 }
diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs
index 518b7da432b..c8f2362b9eb 100644
--- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs
+++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs
@@ -360,7 +360,7 @@ async fn get_header() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Obtain the valid header.
@@ -389,7 +389,7 @@ async fn get_body() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call.
@@ -474,7 +474,7 @@ async fn call_runtime() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Pass an invalid parameters that cannot be decode.
@@ -487,7 +487,7 @@ async fn call_runtime() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2003 && err.message().contains("Invalid parameter")
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::json_rpc_spec::INVALID_PARAM_ERROR && err.message().contains("Invalid parameter")
 	);
 
 	// Valid call.
@@ -590,7 +590,7 @@ async fn call_runtime_without_flag() {
 		.unwrap_err();
 
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2003 && err.message().contains("The runtime updates flag must be set")
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_RUNTIME_CALL && err.message().contains("subscription was started with `withRuntime` set to `false`")
 	);
 }
 
@@ -628,7 +628,7 @@ async fn get_storage_hash() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call without storage at the key.
@@ -896,7 +896,7 @@ async fn get_storage_value() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Valid call without storage at the key.
@@ -1571,7 +1571,7 @@ async fn follow_with_unpin() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// To not exceed the number of pinned blocks, we need to unpin before the next import.
@@ -1720,7 +1720,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	let _res: () = api
@@ -1737,7 +1737,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	// Unpin multiple blocks.
@@ -1755,7 +1755,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 
 	let err = api
@@ -1766,7 +1766,7 @@ async fn follow_with_multiple_unpin_hashes() {
 		.await
 		.unwrap_err();
 	assert_matches!(err,
-		Error::Call(CallError::Custom(ref err)) if err.code() == 2001 && err.message() == "Invalid block hash"
+		Error::Call(CallError::Custom(ref err)) if err.code() == super::error::rpc_spec_v2::INVALID_BLOCK_ERROR && err.message() == "Invalid block hash"
 	);
 }
 
-- 
GitLab