refactor(jsonrpc): Refactor broadcast_tx jsonrpc handler to add struc…

…tured errors (#4180)

* Refactor broadcast_tx jsonrpc handler to add structurred errors

* Remove wip comments

* Refactor tx_status_common, move impl From<NetworkClientResponses> to jsonrpc to avoid near_network dependency in jsonrpc-primitives

* Return error in case of NetworkClientResponses::RequestRouted, return same result on check_tx and send_tx_sync

* Make backward compatible error messages to refactored methods. Make corresponding tests pass

* Move backward compatibility to jsonrpc-primitives from jsonrpc, move ServerError from jsonrpc to jsonrpc-primitives, fix dependencies

* Address review suggestions

* Address review suggestions. Update jsonrpc changelog, bump version

* Address review suggestions

chain/client-primitives/src/types.rs

@@ -418,7 +418,7 @@ pub enum TxStatusError {
     ChainError(near_chain_primitives::Error),
     MissingTransaction(CryptoHash),
     InvalidTx(InvalidTxError),
-    InternalError,
+    InternalError(String),
     TimeoutError,
 }
 
@@ -429,7 +429,9 @@ impl From<TxStatusError> for String {
             TxStatusError::MissingTransaction(tx_hash) => {
                 format!("Transaction {} doesn't exist", tx_hash)
             }
-            TxStatusError::InternalError => format!("Internal error"),
+            TxStatusError::InternalError(debug_message) => {
+                format!("Internal error: {}", debug_message)
+            }
             TxStatusError::TimeoutError => format!("Timeout error"),
             TxStatusError::InvalidTx(e) => format!("Invalid transaction: {}", e),
         }

chain/jsonrpc-primitives/Cargo.toml

@@ -21,3 +21,5 @@ near-crypto = { path = "../../core/crypto" }
 near-metrics = { path = "../../core/metrics" }
 near-primitives = { path = "../../core/primitives" }
 near-primitives-core = { path = "../../core/primitives-core" }
+near-rpc-error-macro = { path = "../../tools/rpctypegen/macro" }
+near-runtime-utils = { path = "../../runtime/near-runtime-utils" }

chain/jsonrpc-primitives/src/errors.rs

@@ -1,6 +1,10 @@
+use std::fmt::Display;
+
 use serde::{Deserialize, Serialize};
 use serde_json::{to_value, Value};
 
+use near_primitives::errors::{InvalidTxError, TxExecutionError};
+
 #[derive(Serialize)]
 pub struct RpcParseError(pub String);
 
@@ -16,6 +20,15 @@ pub struct RpcError {
     pub data: Option<Value>,
 }
 
+/// A general Server Error
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, near_rpc_error_macro::RpcError)]
+pub enum ServerError {
+    TxExecutionError(TxExecutionError),
+    Timeout,
+    Closed,
+    InternalError,
+}
+
 impl RpcError {
     /// A generic constructor.
     ///
@@ -75,3 +88,35 @@ impl From<crate::errors::RpcParseError> for RpcError {
         Self::invalid_params(parse_error.0)
     }
 }
+
+impl Display for ServerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        match self {
+            ServerError::TxExecutionError(e) => write!(f, "ServerError: {}", e),
+            ServerError::Timeout => write!(f, "ServerError: Timeout"),
+            ServerError::Closed => write!(f, "ServerError: Closed"),
+            ServerError::InternalError => write!(f, "ServerError: Internal Error"),
+        }
+    }
+}
+
+impl From<InvalidTxError> for ServerError {
+    fn from(e: InvalidTxError) -> ServerError {
+        ServerError::TxExecutionError(TxExecutionError::InvalidTxError(e))
+    }
+}
+
+impl From<actix::MailboxError> for ServerError {
+    fn from(e: actix::MailboxError) -> Self {
+        match e {
+            actix::MailboxError::Closed => ServerError::Closed,
+            actix::MailboxError::Timeout => ServerError::Timeout,
+        }
+    }
+}
+
+impl From<ServerError> for RpcError {
+    fn from(e: ServerError) -> RpcError {
+        RpcError::server_error(Some(e))
+    }
+}

chain/jsonrpc-primitives/src/rpc.rs

@@ -6,8 +6,7 @@ use serde::{Deserialize, Serialize};
 
 use near_primitives::hash::CryptoHash;
 use near_primitives::merkle::MerklePath;
-use near_primitives::transaction::SignedTransaction;
-use near_primitives::types::{AccountId, BlockReference, MaybeBlockId, TransactionOrReceiptId};
+use near_primitives::types::{BlockReference, MaybeBlockId, TransactionOrReceiptId};
 use near_primitives::views::{
     ExecutionOutcomeWithIdView, LightClientBlockLiteView, QueryRequest, StateChangeWithCauseView,
     StateChangesKindsView, StateChangesRequestView,
@@ -47,12 +46,6 @@ pub struct RpcStateChangesInBlockResponse {
     pub changes: StateChangesKindsView,
 }
 
-#[derive(Serialize, Deserialize)]
-pub struct RpcBroadcastTxSyncResponse {
-    pub transaction_hash: String,
-    pub is_routed: bool,
-}
-
 #[derive(Serialize, Deserialize)]
 pub struct RpcLightClientExecutionProofRequest {
     #[serde(flatten)]
@@ -68,12 +61,6 @@ pub struct RpcLightClientExecutionProofResponse {
     pub block_proof: MerklePath,
 }
 
-#[derive(Clone, Debug)]
-pub enum TransactionInfo {
-    Transaction(SignedTransaction),
-    TransactionId { hash: CryptoHash, account_id: AccountId },
-}
-
 #[derive(Serialize, Deserialize)]
 pub struct RpcValidatorsOrderedRequest {
     pub block_id: MaybeBlockId,

chain/jsonrpc-primitives/src/types/mod.rs

@@ -4,4 +4,5 @@ pub mod config;
 pub mod gas_price;
 pub mod query;
 pub mod receipts;
+pub mod transactions;
 pub mod validator;

chain/jsonrpc-primitives/src/types/transactions.rs

@@ -0,0 +1,125 @@
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+#[derive(Debug, Clone)]
+pub struct RpcBroadcastTransactionRequest {
+    pub signed_transaction: near_primitives::transaction::SignedTransaction,
+}
+
+#[derive(Debug)]
+pub struct RpcTransactionStatusCommonRequest {
+    pub transaction_info: TransactionInfo,
+}
+
+#[derive(Clone, Debug)]
+pub enum TransactionInfo {
+    Transaction(near_primitives::transaction::SignedTransaction),
+    TransactionId {
+        hash: near_primitives::hash::CryptoHash,
+        account_id: near_primitives::types::AccountId,
+    },
+}
+
+#[derive(thiserror::Error, Debug)]
+pub enum RpcTransactionError {
+    #[error("An error happened during transaction execution: {context:?}")]
+    InvalidTransaction { context: near_primitives::errors::InvalidTxError },
+    #[error("Node doesn't track this shard. Cannot determine whether the transaction is valid")]
+    DoesNotTrackShard,
+    #[error("Transaction with hash {transaction_hash} was routed")]
+    RequestRouted { transaction_hash: String },
+    #[error("Transaction {requested_transaction_hash} doesn't exist")]
+    UnknownTransaction { requested_transaction_hash: near_primitives::hash::CryptoHash },
+    #[error("The node reached its limits. Try again later. More details: {debug_info}")]
+    InternalError { debug_info: String },
+    #[error("Timeout")]
+    TimeoutError,
+}
+
+#[derive(Serialize, Deserialize, Debug)]
+pub struct RpcTransactionResponse {
+    #[serde(flatten)]
+    pub final_execution_outcome: near_primitives::views::FinalExecutionOutcomeViewEnum,
+}
+
+#[derive(Serialize, Deserialize, Debug)]
+pub struct RpcBroadcastTxSyncResponse {
+    pub transaction_hash: String,
+}
+
+impl RpcBroadcastTransactionRequest {
+    pub fn parse(value: Option<Value>) -> Result<Self, crate::errors::RpcParseError> {
+        let signed_transaction = crate::utils::parse_signed_transaction(value)?;
+        Ok(Self { signed_transaction })
+    }
+}
+
+impl RpcTransactionStatusCommonRequest {
+    pub fn parse(value: Option<Value>) -> Result<Self, crate::errors::RpcParseError> {
+        if let Ok((hash, account_id)) =
+            crate::utils::parse_params::<(near_primitives::hash::CryptoHash, String)>(value.clone())
+        {
+            if !near_runtime_utils::is_valid_account_id(&account_id) {
+                return Err(crate::errors::RpcParseError(format!(
+                    "Invalid account id: {}",
+                    account_id
+                )));
+            }
+            let transaction_info = TransactionInfo::TransactionId { hash, account_id };
+            Ok(Self { transaction_info })
+        } else {
+            let signed_transaction = crate::utils::parse_signed_transaction(value)?;
+            let transaction_info = TransactionInfo::Transaction(signed_transaction);
+            Ok(Self { transaction_info })
+        }
+    }
+}
+
+impl From<near_client_primitives::types::TxStatusError> for RpcTransactionError {
+    fn from(error: near_client_primitives::types::TxStatusError) -> Self {
+        match error {
+            near_client_primitives::types::TxStatusError::ChainError(err) => {
+                Self::InternalError { debug_info: format!("{:?}", err) }
+            }
+            near_client_primitives::types::TxStatusError::MissingTransaction(
+                requested_transaction_hash,
+            ) => Self::UnknownTransaction { requested_transaction_hash },
+            near_client_primitives::types::TxStatusError::InvalidTx(context) => {
+                Self::InvalidTransaction { context }
+            }
+            near_client_primitives::types::TxStatusError::InternalError(debug_info) => {
+                Self::InternalError { debug_info }
+            }
+            near_client_primitives::types::TxStatusError::TimeoutError => Self::TimeoutError,
+        }
+    }
+}
+
+impl From<near_primitives::views::FinalExecutionOutcomeViewEnum> for RpcTransactionResponse {
+    fn from(
+        final_execution_outcome: near_primitives::views::FinalExecutionOutcomeViewEnum,
+    ) -> Self {
+        Self { final_execution_outcome }
+    }
+}
+
+impl From<RpcTransactionError> for crate::errors::RpcError {
+    fn from(error: RpcTransactionError) -> Self {
+        let error_data = match error {
+            RpcTransactionError::InvalidTransaction { ref context } => Some(
+                serde_json::to_value(crate::errors::ServerError::TxExecutionError(
+                    near_primitives::errors::TxExecutionError::InvalidTxError(context.clone()),
+                ))
+                .unwrap_or(Value::String(error.to_string())),
+            ),
+            _ => Some(Value::String(error.to_string())),
+        };
+        Self::new(-32_000, "Server error".to_string(), error_data)
+    }
+}
+
+impl From<actix::MailboxError> for RpcTransactionError {
+    fn from(error: actix::MailboxError) -> Self {
+        Self::InternalError { debug_info: error.to_string() }
+    }
+}

chain/jsonrpc-primitives/src/utils.rs

@@ -1,6 +1,8 @@
 use serde::de::DeserializeOwned;
 use serde_json::Value;
 
+use near_primitives::borsh::BorshDeserialize;
+
 pub(crate) fn parse_params<T: DeserializeOwned>(
     value: Option<Value>,
 ) -> Result<T, crate::errors::RpcParseError> {
@@ -11,3 +13,18 @@ pub(crate) fn parse_params<T: DeserializeOwned>(
         Err(crate::errors::RpcParseError("Require at least one parameter".to_owned()))
     }
 }
+
+fn from_base64_or_parse_err(encoded: String) -> Result<Vec<u8>, crate::errors::RpcParseError> {
+    near_primitives_core::serialize::from_base64(&encoded)
+        .map_err(|err| crate::errors::RpcParseError(err.to_string()))
+}
+
+pub(crate) fn parse_signed_transaction(
+    value: Option<Value>,
+) -> Result<near_primitives::transaction::SignedTransaction, crate::errors::RpcParseError> {
+    let (encoded,) = crate::utils::parse_params::<(String,)>(value.clone())?;
+    let bytes = crate::utils::from_base64_or_parse_err(encoded)?;
+    Ok(near_primitives::transaction::SignedTransaction::try_from_slice(&bytes).map_err(|err| {
+        crate::errors::RpcParseError(format!("Failed to decode transaction: {}", err))
+    })?)
+}

chain/jsonrpc/CHANGELOG.md

@@ -1,6 +1,20 @@
 # Changelog
 
-## Unreleased
+## 0.2.1
+
+* Refactored methods:
+  * `broadcast_tx_async`
+  * `broadcast_tx_commit`
+  * `EXPERIMENTAL_broadcast_tx_sync`
+  * `EXPERIMENTAL_check_tx`
+  * `EXPERIMENTAL_tx_status`
+  * `tx`
+
+## Breaking changes
+
+Response from `EXPERIMENTAL_broadcast_tx_sync` and `EXPERIMENTAL_check_tx` doesn't return `is_routed` 
+field anymore. In case of a transaction getting routed an error will be returned. Also, `EXPERIMENTAL_check_tx` 
+returns response with transaction hash instead of empty body.
 
 * Added `EXPERIMENTAL_tx_status` endpoint exposing receipts in addition to all
   the rest data available in `tx` endpoint

chain/jsonrpc/Cargo.toml

@@ -1,13 +1,14 @@
 [package]
 name = "near-jsonrpc"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Near Inc <[email protected]>"]
 edition = "2018"
 
 [dependencies]
 actix = "0.11.0-beta.2"
 actix-web = "4.0.0-beta.3"
 actix-cors = { git = "https://github.com/near/actix-extras.git", branch="actix-web-4-beta.3" }
+easy-ext = "0.2"
 tokio = { version = "1.1", features = ["full"] }
 futures = "0.3"
 lazy_static = "1.4"
@@ -27,7 +28,6 @@ near-network = { path = "../network" }
 near-jsonrpc-client = { path = "client" }
 near-jsonrpc-primitives = { path = "../jsonrpc-primitives" }
 near-rpc-error-macro = { path = "../../tools/rpctypegen/macro" }
-near-runtime-utils = { path = "../../runtime/near-runtime-utils" }
 near-performance-metrics = { path = "../../utils/near-performance-metrics" }
 
 [dev-dependencies]