feat: Better errors for JsonRPC calls (#1002)

## What ❔

Provides additional context for JSON-RPC client calls used by the EN
with the help of an extension trait.

## Why ❔

Some of client errors are currently quite obscure and thus difficult to
debug.

## Checklist

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
- [x] Linkcheck has been run via `zk linkcheck`.

---------

Signed-off-by: tomg10 <lemures64@gmail.com>
Co-authored-by: Alex Ostrovski <aov@matterlabs.dev>
Co-authored-by: Alex Ostrovski <slowli@users.noreply.github.com>

core/bin/external_node/src/config/mod.rs

@@ -14,6 +14,7 @@ use zksync_core::{
 };
 use zksync_types::api::BridgeAddresses;
 use zksync_web3_decl::{
+    error::ClientRpcContext,
     jsonrpsee::http_client::{HttpClient, HttpClientBuilder},
     namespaces::{EthNamespaceClient, ZksNamespaceClient},
 };
@@ -40,31 +41,21 @@ impl RemoteENConfig {
     pub async fn fetch(client: &HttpClient) -> anyhow::Result<Self> {
         let bridges = client
             .get_bridge_contracts()
-            .await
-            .context("Failed to fetch bridge contracts")?;
+            .rpc_context("get_bridge_contracts")
+            .await?;
         let l2_testnet_paymaster_addr = client
             .get_testnet_paymaster()
-            .await
-            .context("Failed to fetch paymaster")?;
+            .rpc_context("get_testnet_paymaster")
+            .await?;
         let diamond_proxy_addr = client
             .get_main_contract()
-            .await
-            .context("Failed to fetch L1 contract address")?;
-        let l2_chain_id = L2ChainId::try_from(
-            client
-                .chain_id()
-                .await
-                .context("Failed to fetch L2 chain ID")?
-                .as_u64(),
-        )
-        .unwrap();
-        let l1_chain_id = L1ChainId(
-            client
-                .l1_chain_id()
-                .await
-                .context("Failed to fetch L1 chain ID")?
-                .as_u64(),
-        );
+            .rpc_context("get_main_contract")
+            .await?;
+        let l2_chain_id = client.chain_id().rpc_context("chain_id").await?;
+        let l2_chain_id = L2ChainId::try_from(l2_chain_id.as_u64())
+            .map_err(|err| anyhow::anyhow!("invalid chain ID supplied by main node: {err}"))?;
+        let l1_chain_id = client.l1_chain_id().rpc_context("l1_chain_id").await?;
+        let l1_chain_id = L1ChainId(l1_chain_id.as_u64());
 
         Ok(Self {
             diamond_proxy_addr,

core/lib/web3_decl/Cargo.toml

@@ -20,6 +20,7 @@ jsonrpsee = { version = "0.21.0", default-features = false, features = [
     "macros",
 ] }
 chrono = "0.4"
+pin-project-lite = "0.2.13"
 zksync_types = { path = "../../lib/types" }
 
 [features]

core/lib/web3_decl/src/error.rs

@@ -1,5 +1,18 @@
 //! Definition of errors that can occur in the zkSync Web3 API.
 
+use std::{
+    collections::HashMap,
+    error,
+    error::Error,
+    fmt,
+    future::Future,
+    mem,
+    pin::Pin,
+    task::{Context, Poll},
+};
+
+use jsonrpsee::core::ClientError;
+use pin_project_lite::pin_project;
 use thiserror::Error;
 use zksync_types::{api::SerializationTransactionError, L1BatchNumber, MiniblockNumber};
 
@@ -42,3 +55,152 @@ pub enum Web3Error {
     #[error("Tree API is not available")]
     TreeApiUnavailable,
 }
+
+/// Client RPC error with additional details: the method name and arguments of the called method.
+///
+/// The wrapped error can be accessed using [`AsRef`].
+#[derive(Debug)]
+pub struct EnrichedClientError {
+    inner_error: ClientError,
+    method: &'static str,
+    args: HashMap<&'static str, String>,
+}
+
+/// Alias for a result with enriched client RPC error.
+pub type EnrichedClientResult<T> = Result<T, EnrichedClientError>;
+
+impl EnrichedClientError {
+    /// Wraps the specified `inner_error`.
+    pub fn new(inner_error: ClientError, method: &'static str) -> Self {
+        Self {
+            inner_error,
+            method,
+            args: HashMap::new(),
+        }
+    }
+
+    /// Creates an error wrapping [`RpcError::Custom`].
+    pub fn custom(message: impl Into<String>, method: &'static str) -> Self {
+        Self::new(ClientError::Custom(message.into()), method)
+    }
+
+    /// Adds a tracked argument for this error.
+    #[must_use]
+    pub fn with_arg(mut self, name: &'static str, value: &dyn fmt::Debug) -> Self {
+        self.args.insert(name, format!("{value:?}"));
+        self
+    }
+}
+
+impl AsRef<ClientError> for EnrichedClientError {
+    fn as_ref(&self) -> &ClientError {
+        &self.inner_error
+    }
+}
+
+impl error::Error for EnrichedClientError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        Some(&self.inner_error)
+    }
+}
+
+impl fmt::Display for EnrichedClientError {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        struct DebugArgs<'a>(&'a HashMap<&'static str, String>);
+
+        impl fmt::Debug for DebugArgs<'_> {
+            fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+                formatter.write_str("(")?;
+                for (i, (name, value)) in self.0.iter().enumerate() {
+                    write!(formatter, "{name}={value}")?;
+                    if i + 1 < self.0.len() {
+                        formatter.write_str(", ")?;
+                    }
+                }
+                formatter.write_str(")")
+            }
+        }
+
+        write!(
+            formatter,
+            "JSON-RPC request {}{:?} failed: {}",
+            self.method,
+            DebugArgs(&self.args),
+            self.inner_error
+        )
+    }
+}
+
+pin_project! {
+    /// Contextual information about an RPC. Returned by [`ClientRpcContext::rpc_context()`]. The context is eventually converted
+    /// to a result with [`EnrichedClientError`] error type.
+    #[derive(Debug)]
+    pub struct ClientCallWrapper<'a, F> {
+        #[pin]
+        inner: F,
+        method: &'static str,
+        args: HashMap<&'static str, &'a (dyn fmt::Debug + Send + Sync)>,
+    }
+}
+
+impl<'a, T, F> ClientCallWrapper<'a, F>
+where
+    F: Future<Output = Result<T, ClientError>>,
+{
+    /// Adds a tracked argument for this context.
+    #[must_use]
+    pub fn with_arg(
+        mut self,
+        name: &'static str,
+        value: &'a (dyn fmt::Debug + Send + Sync),
+    ) -> Self {
+        self.args.insert(name, value);
+        self
+    }
+}
+
+impl<T, F> Future for ClientCallWrapper<'_, F>
+where
+    F: Future<Output = Result<T, ClientError>>,
+{
+    type Output = Result<T, EnrichedClientError>;
+
+    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+        let projection = self.project();
+        match projection.inner.poll(cx) {
+            Poll::Pending => Poll::Pending,
+            Poll::Ready(Ok(value)) => Poll::Ready(Ok(value)),
+            Poll::Ready(Err(err)) => {
+                let err = EnrichedClientError {
+                    inner_error: err,
+                    method: projection.method,
+                    // `mem::take()` is safe to use: by contract, a `Future` shouldn't be polled after completion
+                    args: mem::take(projection.args)
+                        .into_iter()
+                        .map(|(name, value)| (name, format!("{value:?}")))
+                        .collect(),
+                };
+                Poll::Ready(Err(err))
+            }
+        }
+    }
+}
+
+/// Extension trait allowing to add context to client RPC calls. Can be used on any future resolving to `Result<_, ClientError>`.
+pub trait ClientRpcContext: Sized {
+    /// Adds basic context information: the name of the invoked RPC method.
+    fn rpc_context(self, method: &'static str) -> ClientCallWrapper<Self>;
+}
+
+impl<T, F> ClientRpcContext for F
+where
+    F: Future<Output = Result<T, ClientError>>,
+{
+    fn rpc_context(self, method: &'static str) -> ClientCallWrapper<Self> {
+        ClientCallWrapper {
+            inner: self,
+            method,
+            args: HashMap::new(),
+        }
+    }
+}

core/lib/web3_decl/src/lib.rs

@@ -15,6 +15,3 @@ pub mod namespaces;
 pub mod types;
 
 pub use jsonrpsee;
-use jsonrpsee::core::ClientError;
-
-pub type RpcResult<T> = Result<T, ClientError>;

core/lib/zksync_core/src/api_server/tx_sender/proxy.rs

@@ -7,9 +7,9 @@ use zksync_types::{
     H256,
 };
 use zksync_web3_decl::{
+    error::{ClientRpcContext, EnrichedClientResult},
     jsonrpsee::http_client::{HttpClient, HttpClientBuilder},
     namespaces::{EthNamespaceClient, ZksNamespaceClient},
-    RpcResult,
 };
 
 /// Used by external node to proxy transaction to the main node
@@ -41,30 +41,54 @@ impl TxProxy {
         self.tx_cache.write().await.insert(tx_hash, tx);
     }
 
-    pub async fn submit_tx(&self, tx: &L2Tx) -> RpcResult<H256> {
+    pub async fn submit_tx(&self, tx: &L2Tx) -> EnrichedClientResult<H256> {
         let input_data = tx.common_data.input_data().expect("raw tx is absent");
         let raw_tx = zksync_types::Bytes(input_data.to_vec());
-        tracing::info!("Proxying tx {}", tx.hash());
-        self.client.send_raw_transaction(raw_tx).await
+        let tx_hash = tx.hash();
+        tracing::info!("Proxying tx {tx_hash:?}");
+        self.client
+            .send_raw_transaction(raw_tx)
+            .rpc_context("send_raw_transaction")
+            .with_arg("tx_hash", &tx_hash)
+            .await
     }
 
-    pub async fn request_tx(&self, id: TransactionId) -> RpcResult<Option<Transaction>> {
+    pub async fn request_tx(&self, id: TransactionId) -> EnrichedClientResult<Option<Transaction>> {
         match id {
             TransactionId::Block(BlockId::Hash(block), index) => {
                 self.client
                     .get_transaction_by_block_hash_and_index(block, index)
+                    .rpc_context("get_transaction_by_block_hash_and_index")
+                    .with_arg("block", &block)
+                    .with_arg("index", &index)
                     .await
             }
             TransactionId::Block(BlockId::Number(block), index) => {
                 self.client
                     .get_transaction_by_block_number_and_index(block, index)
+                    .rpc_context("get_transaction_by_block_number_and_index")
+                    .with_arg("block", &block)
+                    .with_arg("index", &index)
+                    .await
+            }
+            TransactionId::Hash(hash) => {
+                self.client
+                    .get_transaction_by_hash(hash)
+                    .rpc_context("get_transaction_by_hash")
+                    .with_arg("hash", &hash)
                     .await
             }
-            TransactionId::Hash(hash) => self.client.get_transaction_by_hash(hash).await,
         }
     }
 
-    pub async fn request_tx_details(&self, hash: H256) -> RpcResult<Option<TransactionDetails>> {
-        self.client.get_transaction_details(hash).await
+    pub async fn request_tx_details(
+        &self,
+        hash: H256,
+    ) -> EnrichedClientResult<Option<TransactionDetails>> {
+        self.client
+            .get_transaction_details(hash)
+            .rpc_context("get_transaction_details")
+            .with_arg("hash", &hash)
+            .await
     }
 }

core/lib/zksync_core/src/api_server/tx_sender/result.rs

@@ -1,6 +1,7 @@
 use multivm::interface::{ExecutionResult, VmExecutionResultAndLogs};
 use thiserror::Error;
 use zksync_types::{l2::error::TxCheckError, U256};
+use zksync_web3_decl::error::EnrichedClientError;
 
 use crate::api_server::execution_sandbox::{SandboxExecutionError, ValidationError};
 
@@ -66,7 +67,7 @@ pub enum SubmitTxError {
     IntrinsicGas,
     /// Error returned from main node
     #[error("{0}")]
-    ProxyError(#[from] zksync_web3_decl::jsonrpsee::core::ClientError),
+    ProxyError(#[from] EnrichedClientError),
     #[error("not enough gas to publish compressed bytecodes")]
     FailedToPublishCompressedBytecodes,
     /// Catch-all internal error (e.g., database error) that should not be exposed to the caller.

core/lib/zksync_core/src/api_server/web3/backend_jsonrpsee/mod.rs

@@ -52,6 +52,11 @@ impl SubmitTxError {
     pub(crate) fn into_web3_error(self, method_name: &'static str) -> Web3Error {
         match self {
             Self::Internal(err) => internal_error(method_name, err),
+            Self::ProxyError(ref err) => {
+                // Strip internal error details that should not be exposed to the caller.
+                tracing::warn!("Error proxying call to main node in method {method_name}: {err}");
+                Web3Error::SubmitTransactionError(err.as_ref().to_string(), self.data())
+            }
             _ => Web3Error::SubmitTransactionError(self.to_string(), self.data()),
         }
     }