diff --git a/Cargo.lock b/Cargo.lock index 94d572801cb7..2e14ea7ba3ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8793,14 +8793,13 @@ dependencies = [ "futures 0.3.28", "governor", "hex", + "http", "itertools 0.10.5", - "jsonrpsee", "lru", "multivm", "once_cell", "pin-project-lite", "rand 0.8.5", - "reqwest", "serde", "serde_json", "test-casing", diff --git a/core/lib/web3_decl/src/error.rs b/core/lib/web3_decl/src/error.rs index 1ada1f602324..c52bc3054f82 100644 --- a/core/lib/web3_decl/src/error.rs +++ b/core/lib/web3_decl/src/error.rs @@ -39,10 +39,12 @@ pub enum Web3Error { LogsLimitExceeded(usize, u32, u32), #[error("invalid filter: if blockHash is supplied fromBlock and toBlock must not be")] InvalidFilterBlockHash, - #[error("Not implemented")] - NotImplemented, - - #[error("Tree API is not available")] + /// Weaker form of a "method not found" error; the method implementation is technically present, + /// but the node configuration prevents the method from functioning. + #[error("Method not implemented")] + MethodNotImplemented, + /// Unavailability caused by node configuration is returned as [`Self::MethodNotImplemented`]. + #[error("Tree API is temporarily unavailable")] TreeApiUnavailable, #[error("Internal error")] InternalError(#[from] anyhow::Error), diff --git a/core/node/api_server/Cargo.toml b/core/node/api_server/Cargo.toml index 9009c66e1469..4fbf866b15e8 100644 --- a/core/node/api_server/Cargo.toml +++ b/core/node/api_server/Cargo.toml @@ -22,7 +22,7 @@ zksync_shared_metrics.workspace = true zksync_state.workspace = true zksync_system_constants.workspace = true zksync_metadata_calculator.workspace = true -zksync_web3_decl.workspace = true +zksync_web3_decl = { workspace = true, features = ["client", "server"] } zksync_utils.workspace = true zksync_protobuf.workspace = true zksync_mini_merkle_tree.workspace = true @@ -46,10 +46,9 @@ thread_local.workspace = true governor.workspace = true pin-project-lite.workspace = true hex.workspace = true -jsonrpsee.workspace = true -reqwest.workspace = true +http.workspace = true tower.workspace = true -tower-http.workspace = true +tower-http = { workspace = true, features = ["cors", "metrics"] } lru.workspace = true [dev-dependencies] diff --git a/core/node/api_server/src/web3/backend_jsonrpsee/middleware.rs b/core/node/api_server/src/web3/backend_jsonrpsee/middleware.rs index 17d4d3398908..5c25b0ebc3ca 100644 --- a/core/node/api_server/src/web3/backend_jsonrpsee/middleware.rs +++ b/core/node/api_server/src/web3/backend_jsonrpsee/middleware.rs @@ -92,10 +92,8 @@ where let rp = MethodResponse::error( request.id, ErrorObject::borrowed( - ErrorCode::ServerError( - reqwest::StatusCode::TOO_MANY_REQUESTS.as_u16().into(), - ) - .code(), + ErrorCode::ServerError(http::StatusCode::TOO_MANY_REQUESTS.as_u16().into()) + .code(), "Too many requests", None, ), @@ -336,10 +334,10 @@ where mod tests { use std::time::Duration; - use jsonrpsee::helpers::MethodResponseResult; use rand::{thread_rng, Rng}; use test_casing::{test_casing, Product}; use zksync_types::api; + use zksync_web3_decl::jsonrpsee::helpers::MethodResponseResult; use super::*; diff --git a/core/node/api_server/src/web3/backend_jsonrpsee/mod.rs b/core/node/api_server/src/web3/backend_jsonrpsee/mod.rs index 76beb0f7a3a8..856ddb35ca3f 100644 --- a/core/node/api_server/src/web3/backend_jsonrpsee/mod.rs +++ b/core/node/api_server/src/web3/backend_jsonrpsee/mod.rs @@ -31,7 +31,7 @@ impl MethodTracer { _ => None, }; let code = match err { - Web3Error::NotImplemented => ErrorCode::MethodNotFound.code(), + Web3Error::MethodNotImplemented => ErrorCode::MethodNotFound.code(), Web3Error::InternalError(_) => ErrorCode::InternalError.code(), Web3Error::NoBlock | Web3Error::PrunedBlock(_) diff --git a/core/node/api_server/src/web3/backend_jsonrpsee/testonly.rs b/core/node/api_server/src/web3/backend_jsonrpsee/testonly.rs index e93f6c886fdc..98d6bf2440e4 100644 --- a/core/node/api_server/src/web3/backend_jsonrpsee/testonly.rs +++ b/core/node/api_server/src/web3/backend_jsonrpsee/testonly.rs @@ -2,7 +2,7 @@ use std::{mem, sync::Mutex}; -use jsonrpsee::{helpers::MethodResponseResult, MethodResponse}; +use zksync_web3_decl::jsonrpsee::{helpers::MethodResponseResult, MethodResponse}; use super::metadata::MethodMetadata; diff --git a/core/node/api_server/src/web3/metrics.rs b/core/node/api_server/src/web3/metrics.rs index a8d6c0d5851a..af6e1bf63ad8 100644 --- a/core/node/api_server/src/web3/metrics.rs +++ b/core/node/api_server/src/web3/metrics.rs @@ -185,7 +185,7 @@ impl Web3ErrorKind { Web3Error::LogsLimitExceeded(..) => Self::LogsLimitExceeded, Web3Error::InvalidFilterBlockHash => Self::InvalidFilterBlockHash, Web3Error::TreeApiUnavailable => Self::TreeApiUnavailable, - Web3Error::InternalError(_) | Web3Error::NotImplemented => Self::Internal, + Web3Error::InternalError(_) | Web3Error::MethodNotImplemented => Self::Internal, } } } diff --git a/core/node/api_server/src/web3/mod.rs b/core/node/api_server/src/web3/mod.rs index 0f9126344791..b86666ea6868 100644 --- a/core/node/api_server/src/web3/mod.rs +++ b/core/node/api_server/src/web3/mod.rs @@ -645,10 +645,10 @@ impl ApiServer { let cors = is_http.then(|| { CorsLayer::new() // Allow `POST` when accessing the resource - .allow_methods([reqwest::Method::POST]) + .allow_methods([http::Method::POST]) // Allow requests from any origin .allow_origin(tower_http::cors::Any) - .allow_headers([reqwest::header::CONTENT_TYPE]) + .allow_headers([http::header::CONTENT_TYPE]) }); // Setup metrics for the number of in-flight requests. let (in_flight_requests, counter) = InFlightRequestsLayer::pair(); diff --git a/core/node/api_server/src/web3/namespaces/eth.rs b/core/node/api_server/src/web3/namespaces/eth.rs index 0ca9c30055c9..ff2403051de0 100644 --- a/core/node/api_server/src/web3/namespaces/eth.rs +++ b/core/node/api_server/src/web3/namespaces/eth.rs @@ -179,7 +179,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; // We clone the filter to not hold the filter lock for an extended period of time. let maybe_filter = installed_filters.lock().await.get_and_update_stats(idx); @@ -483,7 +483,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; let mut storage = self.state.acquire_connection().await?; let last_block_number = storage .blocks_dal() @@ -505,7 +505,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; if let Some(topics) = filter.topics.as_ref() { if topics.len() > EVENT_TOPIC_NUMBER_LIMIT { return Err(Web3Error::TooManyTopics); @@ -525,7 +525,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; Ok(installed_filters .lock() .await @@ -539,7 +539,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; let mut filter = installed_filters .lock() .await @@ -565,7 +565,7 @@ impl EthNamespace { .state .installed_filters .as_ref() - .ok_or(Web3Error::NotImplemented)?; + .ok_or(Web3Error::MethodNotImplemented)?; Ok(installed_filters.lock().await.remove(idx)) } diff --git a/core/node/api_server/src/web3/namespaces/zks.rs b/core/node/api_server/src/web3/namespaces/zks.rs index 5fe91da899ea..f65dcb2525ce 100644 --- a/core/node/api_server/src/web3/namespaces/zks.rs +++ b/core/node/api_server/src/web3/namespaces/zks.rs @@ -492,11 +492,11 @@ impl ZksNamespace { .state .tree_api .as_deref() - .ok_or(Web3Error::TreeApiUnavailable)?; + .ok_or(Web3Error::MethodNotImplemented)?; let proofs_result = tree_api.get_proofs(l1_batch_number, hashed_keys).await; let proofs = match proofs_result { Ok(proofs) => proofs, - Err(TreeApiError::NotReady) => return Err(Web3Error::TreeApiUnavailable), + Err(TreeApiError::NotReady(_)) => return Err(Web3Error::TreeApiUnavailable), Err(TreeApiError::NoVersion(err)) => { return if err.missing_version > err.version_count { Ok(None) @@ -536,7 +536,7 @@ impl ZksNamespace { self.state .api_config .base_token_address - .ok_or(Web3Error::NotImplemented) + .ok_or(Web3Error::MethodNotImplemented) } #[tracing::instrument(skip(self))] diff --git a/core/node/api_server/src/web3/tests/filters.rs b/core/node/api_server/src/web3/tests/filters.rs index 4358e99be422..7342ce7e979f 100644 --- a/core/node/api_server/src/web3/tests/filters.rs +++ b/core/node/api_server/src/web3/tests/filters.rs @@ -1,9 +1,14 @@ //! Tests for filter-related methods in the `eth` namespace. -use std::fmt::Debug; +use std::fmt; -use jsonrpsee::{core::client::Error, types::error::ErrorCode}; -use zksync_web3_decl::{jsonrpsee::core::ClientError as RpcError, types::FilterChanges}; +use zksync_web3_decl::{ + jsonrpsee::{ + core::{client::Error, ClientError as RpcError}, + types::error::ErrorCode, + }, + types::FilterChanges, +}; use super::*; @@ -279,10 +284,10 @@ async fn log_filter_changes_with_block_boundaries() { test_http_server(LogFilterChangesWithBlockBoundariesTest).await; } -fn assert_not_implemented(result: Result) { +fn assert_not_implemented(result: Result) { assert_matches!(result, Err(Error::Call(e)) => { assert_eq!(e.code(), ErrorCode::MethodNotFound.code()); - assert_eq!(e.message(), "Not implemented"); + assert_eq!(e.message(), "Method not implemented"); }); } diff --git a/core/node/api_server/src/web3/tests/mod.rs b/core/node/api_server/src/web3/tests/mod.rs index 0225734b3fc8..b9e8c96a3b1b 100644 --- a/core/node/api_server/src/web3/tests/mod.rs +++ b/core/node/api_server/src/web3/tests/mod.rs @@ -7,11 +7,6 @@ use std::{ use assert_matches::assert_matches; use async_trait::async_trait; -use jsonrpsee::{ - core::{client::ClientT, params::BatchRequestBuilder, ClientError}, - rpc_params, - types::{error::OVERSIZED_RESPONSE_CODE, ErrorObjectOwned}, -}; use multivm::zk_evm_latest::ethereum_types::U256; use tokio::sync::watch; use zksync_config::{ @@ -47,7 +42,15 @@ use zksync_types::{ use zksync_utils::u256_to_h256; use zksync_web3_decl::{ client::{Client, DynClient, L2}, - jsonrpsee::{http_client::HttpClient, types::error::ErrorCode}, + jsonrpsee::{ + core::{client::ClientT, params::BatchRequestBuilder, ClientError}, + http_client::HttpClient, + rpc_params, + types::{ + error::{ErrorCode, OVERSIZED_RESPONSE_CODE}, + ErrorObjectOwned, + }, + }, namespaces::{EnNamespaceClient, EthNamespaceClient, ZksNamespaceClient}, }; @@ -984,13 +987,9 @@ impl HttpTest for RpcCallsTracingTest { assert_eq!(calls[0].metadata.block_diff, None); // Check protocol-level errors. - ClientT::request::( - &client, - "eth_unknownMethod", - jsonrpsee::rpc_params![], - ) - .await - .unwrap_err(); + ClientT::request::(&client, "eth_unknownMethod", rpc_params![]) + .await + .unwrap_err(); let calls = self.tracer.recorded_calls().take(); assert_eq!(calls.len(), 1); @@ -1000,13 +999,9 @@ impl HttpTest for RpcCallsTracingTest { ); assert!(!calls[0].metadata.has_app_error); - ClientT::request::( - &client, - "eth_getBlockByNumber", - jsonrpsee::rpc_params![0], - ) - .await - .unwrap_err(); + ClientT::request::(&client, "eth_getBlockByNumber", rpc_params![0]) + .await + .unwrap_err(); let calls = self.tracer.recorded_calls().take(); assert_eq!(calls.len(), 1); @@ -1020,7 +1015,7 @@ impl HttpTest for RpcCallsTracingTest { ClientT::request::( &client, "eth_getFilterLogs", - jsonrpsee::rpc_params![U256::from(1)], + rpc_params![U256::from(1)], ) .await .unwrap_err(); @@ -1035,8 +1030,8 @@ impl HttpTest for RpcCallsTracingTest { // Check batch RPC request. let mut batch = BatchRequestBuilder::new(); - batch.insert("eth_blockNumber", jsonrpsee::rpc_params![])?; - batch.insert("zks_L1BatchNumber", jsonrpsee::rpc_params![])?; + batch.insert("eth_blockNumber", rpc_params![])?; + batch.insert("zks_L1BatchNumber", rpc_params![])?; let response = ClientT::batch_request::(&client, batch).await?; for response_part in response { assert_eq!(response_part.unwrap(), U64::from(0)); diff --git a/core/node/api_server/src/web3/tests/ws.rs b/core/node/api_server/src/web3/tests/ws.rs index ff3fc465811b..93f6b536c34b 100644 --- a/core/node/api_server/src/web3/tests/ws.rs +++ b/core/node/api_server/src/web3/tests/ws.rs @@ -3,8 +3,7 @@ use std::collections::HashSet; use async_trait::async_trait; -use jsonrpsee::core::{client::ClientT, params::BatchRequestBuilder, ClientError}; -use reqwest::StatusCode; +use http::StatusCode; use tokio::sync::watch; use zksync_config::configs::chain::NetworkConfig; use zksync_dal::ConnectionPool; @@ -12,7 +11,11 @@ use zksync_types::{api, Address, L1BatchNumber, H256, U64}; use zksync_web3_decl::{ client::{WsClient, L2}, jsonrpsee::{ - core::client::{Subscription, SubscriptionClientT}, + core::{ + client::{ClientT, Subscription, SubscriptionClientT}, + params::BatchRequestBuilder, + ClientError, + }, rpc_params, }, namespaces::{EthNamespaceClient, ZksNamespaceClient}, diff --git a/core/node/metadata_calculator/src/api_server/mod.rs b/core/node/metadata_calculator/src/api_server/mod.rs index c427397b72c0..77773ffa37c6 100644 --- a/core/node/metadata_calculator/src/api_server/mod.rs +++ b/core/node/metadata_calculator/src/api_server/mod.rs @@ -127,13 +127,26 @@ impl IntoResponse for TreeApiServerError { pub enum TreeApiError { #[error(transparent)] NoVersion(NoVersionError), - #[error("tree API is temporarily not available because the Merkle tree isn't initialized; repeat request later")] - NotReady, + #[error("tree API is temporarily unavailable")] + NotReady(#[source] Option), /// Catch-all variant for internal errors. #[error("internal error")] Internal(#[from] anyhow::Error), } +impl TreeApiError { + fn for_request(err: reqwest::Error, request_description: impl fmt::Display) -> Self { + let is_not_ready = err.is_timeout() || err.is_connect(); + let err = + anyhow::Error::new(err).context(format!("failed requesting {request_description}")); + if is_not_ready { + Self::NotReady(Some(err)) + } else { + Self::Internal(err) + } + } +} + /// Client accessing Merkle tree API. #[async_trait] pub trait TreeApiClient: 'static + Send + Sync + fmt::Debug { @@ -155,7 +168,7 @@ impl TreeApiClient for LazyAsyncTreeReader { if let Some(reader) = self.read() { Ok(reader.info().await) } else { - Err(TreeApiError::NotReady) + Err(TreeApiError::NotReady(None)) } } @@ -170,7 +183,7 @@ impl TreeApiClient for LazyAsyncTreeReader { .await .map_err(TreeApiError::NoVersion) } else { - Err(TreeApiError::NotReady) + Err(TreeApiError::NotReady(None)) } } } @@ -184,9 +197,15 @@ pub struct TreeApiHttpClient { } impl TreeApiHttpClient { + /// Creates a new HTTP client with default settings. pub fn new(url_base: &str) -> Self { + Self::from_client(reqwest::Client::new(), url_base) + } + + /// Wraps a provided HTTP client. + pub fn from_client(client: reqwest::Client, url_base: &str) -> Self { Self { - inner: reqwest::Client::new(), + inner: client, info_url: url_base.to_owned(), proofs_url: format!("{url_base}/proofs"), } @@ -202,9 +221,11 @@ impl CheckHealth for TreeApiHttpClient { async fn check_health(&self) -> Health { match self.get_info().await { Ok(info) => Health::from(HealthStatus::Ready).with_details(info), - Err(TreeApiError::NotReady) => HealthStatus::Affected.into(), - Err(err) => Health::from(HealthStatus::NotReady).with_details(serde_json::json!({ + // Tree API is not a critical component, so its errors are not considered fatal for the app health. + Err(err) => Health::from(HealthStatus::Affected).with_details(serde_json::json!({ "error": err.to_string(), + // Transient error detection is a best-effort estimate + "is_transient_error": matches!(err, TreeApiError::NotReady(_)), })), } } @@ -218,7 +239,7 @@ impl TreeApiClient for TreeApiHttpClient { .get(&self.info_url) .send() .await - .context("Failed requesting tree info")?; + .map_err(|err| TreeApiError::for_request(err, "tree info"))?; let response = response .error_for_status() .context("Requesting tree info returned non-OK response")?; @@ -242,7 +263,12 @@ impl TreeApiClient for TreeApiHttpClient { }) .send() .await - .with_context(|| format!("failed requesting proofs for L1 batch #{l1_batch_number}"))?; + .map_err(|err| { + TreeApiError::for_request( + err, + format_args!("proofs for L1 batch #{l1_batch_number}"), + ) + })?; let is_problem = response .headers() diff --git a/core/node/metadata_calculator/src/api_server/tests.rs b/core/node/metadata_calculator/src/api_server/tests.rs index ce7ad03ada07..26782e446f3f 100644 --- a/core/node/metadata_calculator/src/api_server/tests.rs +++ b/core/node/metadata_calculator/src/api_server/tests.rs @@ -1,9 +1,13 @@ //! Tests for the Merkle tree API. -use std::net::Ipv4Addr; +use std::{net::Ipv4Addr, time::Duration}; use assert_matches::assert_matches; use tempfile::TempDir; +use tokio::{ + io::AsyncWriteExt, + net::{TcpListener, TcpSocket}, +}; use zksync_dal::{ConnectionPool, Core}; use super::*; @@ -72,6 +76,40 @@ async fn merkle_tree_api() { api_server_task.await.unwrap().unwrap(); } +#[tokio::test] +async fn api_client_connection_error() { + // Use an address that will definitely fail on a timeout. + let socket = TcpSocket::new_v4().unwrap(); + socket.bind((Ipv4Addr::LOCALHOST, 0).into()).unwrap(); + let local_addr = socket.local_addr().unwrap(); + + let client = reqwest::Client::builder() + .timeout(Duration::from_secs(1)) + .build() + .unwrap(); + let api_client = TreeApiHttpClient::from_client(client, &format!("http://{local_addr}")); + let err = api_client.get_info().await.unwrap_err(); + assert_matches!(err, TreeApiError::NotReady(Some(_))); +} + +#[tokio::test] +async fn api_client_unparesable_response_error() { + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await.unwrap(); + let local_addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + while let Ok((mut stream, _)) = listener.accept().await { + stream + .write_all(b"HTTP/1.1 200 OK\ncontent-type: application/json\ncontent-length: 13\n\nNot JSON, lol") + .await + .ok(); + } + }); + + let api_client = TreeApiHttpClient::new(&format!("http://{local_addr}")); + let err = api_client.get_info().await.unwrap_err(); + assert_matches!(err, TreeApiError::Internal(_)); +} + #[tokio::test] async fn local_merkle_tree_client() { let pool = ConnectionPool::::test_pool().await; @@ -82,7 +120,7 @@ async fn local_merkle_tree_client() { let tree_reader = calculator.tree_reader(); let err = tree_reader.get_info().await.unwrap_err(); - assert_matches!(err, TreeApiError::NotReady); + assert_matches!(err, TreeApiError::NotReady(None)); // Wait until the calculator processes initial L1 batches. run_calculator(calculator).await;