Skip to content

Commit

Permalink
fix(merkle-tree): Fix tree API health check status (#1973)
Browse files Browse the repository at this point in the history
## What ❔

- Makes health status "affected" if the tree API is not available.
- Distinguishes between "tree API not enabled on the node" and "tree API
temporarily unavailable" errors.

## Why ❔

Right now, the status is "not ready", meaning that the entire app health
status is "not ready" as well. This makes API servers excluded from
serving traffic, which is not desirable (they can still serve all of
requests other than `zks_getProof`).

## 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`.
  • Loading branch information
slowli authored May 17, 2024
1 parent d62a2b0 commit 6235561
Show file tree
Hide file tree
Showing 15 changed files with 136 additions and 71 deletions.
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions core/lib/web3_decl/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
7 changes: 3 additions & 4 deletions core/node/api_server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
8 changes: 3 additions & 5 deletions core/node/api_server/src/web3/backend_jsonrpsee/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand Down Expand Up @@ -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::*;

Expand Down
2 changes: 1 addition & 1 deletion core/node/api_server/src/web3/backend_jsonrpsee/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion core/node/api_server/src/web3/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions core/node/api_server/src/web3/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 6 additions & 6 deletions core/node/api_server/src/web3/namespaces/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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()
Expand All @@ -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);
Expand All @@ -525,7 +525,7 @@ impl EthNamespace {
.state
.installed_filters
.as_ref()
.ok_or(Web3Error::NotImplemented)?;
.ok_or(Web3Error::MethodNotImplemented)?;
Ok(installed_filters
.lock()
.await
Expand All @@ -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
Expand All @@ -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))
}

Expand Down
6 changes: 3 additions & 3 deletions core/node/api_server/src/web3/namespaces/zks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))]
Expand Down
15 changes: 10 additions & 5 deletions core/node/api_server/src/web3/tests/filters.rs
Original file line number Diff line number Diff line change
@@ -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::*;

Expand Down Expand Up @@ -279,10 +284,10 @@ async fn log_filter_changes_with_block_boundaries() {
test_http_server(LogFilterChangesWithBlockBoundariesTest).await;
}

fn assert_not_implemented<T: Debug>(result: Result<T, Error>) {
fn assert_not_implemented<T: fmt::Debug>(result: Result<T, Error>) {
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");
});
}

Expand Down
41 changes: 18 additions & 23 deletions core/node/api_server/src/web3/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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},
};

Expand Down Expand Up @@ -984,13 +987,9 @@ impl HttpTest for RpcCallsTracingTest {
assert_eq!(calls[0].metadata.block_diff, None);

// Check protocol-level errors.
ClientT::request::<serde_json::Value, _>(
&client,
"eth_unknownMethod",
jsonrpsee::rpc_params![],
)
.await
.unwrap_err();
ClientT::request::<serde_json::Value, _>(&client, "eth_unknownMethod", rpc_params![])
.await
.unwrap_err();

let calls = self.tracer.recorded_calls().take();
assert_eq!(calls.len(), 1);
Expand All @@ -1000,13 +999,9 @@ impl HttpTest for RpcCallsTracingTest {
);
assert!(!calls[0].metadata.has_app_error);

ClientT::request::<serde_json::Value, _>(
&client,
"eth_getBlockByNumber",
jsonrpsee::rpc_params![0],
)
.await
.unwrap_err();
ClientT::request::<serde_json::Value, _>(&client, "eth_getBlockByNumber", rpc_params![0])
.await
.unwrap_err();

let calls = self.tracer.recorded_calls().take();
assert_eq!(calls.len(), 1);
Expand All @@ -1020,7 +1015,7 @@ impl HttpTest for RpcCallsTracingTest {
ClientT::request::<serde_json::Value, _>(
&client,
"eth_getFilterLogs",
jsonrpsee::rpc_params![U256::from(1)],
rpc_params![U256::from(1)],
)
.await
.unwrap_err();
Expand All @@ -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::<U64>(&client, batch).await?;
for response_part in response {
assert_eq!(response_part.unwrap(), U64::from(0));
Expand Down
9 changes: 6 additions & 3 deletions core/node/api_server/src/web3/tests/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
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;
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},
Expand Down
Loading

0 comments on commit 6235561

Please sign in to comment.