From d06931a38cb403073cd7e8496dd33f761da9fc60 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 26 Oct 2023 15:05:03 +0200 Subject: [PATCH] fix: spec compliant engine api errors (#5188) --- Cargo.lock | 1 + crates/rpc/rpc-engine-api/Cargo.toml | 1 + crates/rpc/rpc-engine-api/src/error.rs | 156 ++++++++++++++++++++++--- 3 files changed, 141 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7a6f9d39..1237826ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6484,6 +6484,7 @@ dependencies = [ "reth-rpc-types", "reth-rpc-types-compat", "reth-tasks", + "serde", "thiserror", "tokio", "tracing", diff --git a/crates/rpc/rpc-engine-api/Cargo.toml b/crates/rpc/rpc-engine-api/Cargo.toml index ad2e876e6..65e81ad74 100644 --- a/crates/rpc/rpc-engine-api/Cargo.toml +++ b/crates/rpc/rpc-engine-api/Cargo.toml @@ -32,6 +32,7 @@ thiserror.workspace = true jsonrpsee-types.workspace = true jsonrpsee-core.workspace = true tracing.workspace = true +serde.workspace = true [dev-dependencies] alloy-rlp.workspace = true diff --git a/crates/rpc/rpc-engine-api/src/error.rs b/crates/rpc/rpc-engine-api/src/error.rs index 59aeb1507..fe1637e77 100644 --- a/crates/rpc/rpc-engine-api/src/error.rs +++ b/crates/rpc/rpc-engine-api/src/error.rs @@ -1,4 +1,6 @@ -use jsonrpsee_types::error::{INTERNAL_ERROR_CODE, INVALID_PARAMS_CODE}; +use jsonrpsee_types::error::{ + INTERNAL_ERROR_CODE, INVALID_PARAMS_CODE, INVALID_PARAMS_MSG, SERVER_ERROR_MSG, +}; use reth_beacon_consensus::{BeaconForkChoiceUpdateError, BeaconOnNewPayloadError}; use reth_payload_builder::error::PayloadBuilderError; use reth_primitives::{B256, U256}; @@ -14,6 +16,9 @@ pub const UNKNOWN_PAYLOAD_CODE: i32 = -38001; /// Request too large error code. pub const REQUEST_TOO_LARGE_CODE: i32 = -38004; +/// Error message for the request too large error. +const REQUEST_TOO_LARGE_MESSAGE: &str = "Too large request"; + /// Error returned by [`EngineApi`][crate::EngineApi] /// /// Note: This is a high-fidelity error type which can be converted to an RPC error that adheres to @@ -25,7 +30,7 @@ pub enum EngineApiError { #[error("Unknown payload")] UnknownPayload, /// The payload body request length is too large. - #[error("payload request too large: {len}")] + #[error("requested count too large: {len}")] PayloadRequestTooLarge { /// The length that was requested. len: u64, @@ -57,7 +62,7 @@ pub enum EngineApiError { NoParentBeaconBlockRootPostCancun, /// Thrown if `PayloadAttributes` were provided with a timestamp, but the version of the engine /// method called is meant for a fork that occurs after the provided timestamp. - #[error("unsupported fork")] + #[error("Unsupported fork")] UnsupportedFork, /// Terminal total difficulty mismatch during transition configuration exchange. #[error( @@ -95,37 +100,97 @@ pub enum EngineApiError { GetPayloadError(#[from] PayloadBuilderError), } +/// Helper type to represent the `error` field in the error response: +/// +#[derive(serde::Serialize)] +struct ErrorData { + err: String, +} + +impl ErrorData { + #[inline] + fn new(err: impl std::fmt::Display) -> Self { + Self { err: err.to_string() } + } +} + impl From for jsonrpsee_types::error::ErrorObject<'static> { fn from(error: EngineApiError) -> Self { - let code = match error { + match error { EngineApiError::InvalidBodiesRange { .. } | EngineApiError::WithdrawalsNotSupportedInV1 | EngineApiError::ParentBeaconBlockRootNotSupportedBeforeV3 | EngineApiError::NoParentBeaconBlockRootPostCancun | EngineApiError::NoWithdrawalsPostShanghai | - EngineApiError::HasWithdrawalsPreShanghai => INVALID_PARAMS_CODE, - EngineApiError::UnknownPayload => UNKNOWN_PAYLOAD_CODE, - EngineApiError::PayloadRequestTooLarge { .. } => REQUEST_TOO_LARGE_CODE, - EngineApiError::UnsupportedFork => UNSUPPORTED_FORK_CODE, - + EngineApiError::HasWithdrawalsPreShanghai => { + // Note: the data field is not required by the spec, but is also included by other + // clients + jsonrpsee_types::error::ErrorObject::owned( + INVALID_PARAMS_CODE, + INVALID_PARAMS_MSG, + Some(ErrorData::new(error)), + ) + } + EngineApiError::UnknownPayload => jsonrpsee_types::error::ErrorObject::owned( + UNKNOWN_PAYLOAD_CODE, + error.to_string(), + None::<()>, + ), + EngineApiError::PayloadRequestTooLarge { .. } => { + jsonrpsee_types::error::ErrorObject::owned( + REQUEST_TOO_LARGE_CODE, + REQUEST_TOO_LARGE_MESSAGE, + Some(ErrorData::new(error)), + ) + } + EngineApiError::UnsupportedFork => jsonrpsee_types::error::ErrorObject::owned( + UNSUPPORTED_FORK_CODE, + error.to_string(), + None::<()>, + ), // Error responses from the consensus engine EngineApiError::ForkChoiceUpdate(ref err) => match err { - BeaconForkChoiceUpdateError::ForkchoiceUpdateError(err) => return (*err).into(), + BeaconForkChoiceUpdateError::ForkchoiceUpdateError(err) => (*err).into(), BeaconForkChoiceUpdateError::EngineUnavailable | - BeaconForkChoiceUpdateError::Internal(_) => INTERNAL_ERROR_CODE, + BeaconForkChoiceUpdateError::Internal(_) => { + jsonrpsee_types::error::ErrorObject::owned( + INTERNAL_ERROR_CODE, + SERVER_ERROR_MSG, + Some(ErrorData::new(error)), + ) + } }, EngineApiError::NewPayload(ref err) => match err { - BeaconOnNewPayloadError::Internal(_) | - BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions => INVALID_PARAMS_CODE, - BeaconOnNewPayloadError::EngineUnavailable => INTERNAL_ERROR_CODE, + BeaconOnNewPayloadError::Internal(_) => jsonrpsee_types::error::ErrorObject::owned( + INTERNAL_ERROR_CODE, + SERVER_ERROR_MSG, + Some(ErrorData::new(error)), + ), + BeaconOnNewPayloadError::PreCancunBlockWithBlobTransactions => { + jsonrpsee_types::error::ErrorObject::owned( + INVALID_PARAMS_CODE, + INVALID_PARAMS_MSG, + Some(ErrorData::new(error)), + ) + } + BeaconOnNewPayloadError::EngineUnavailable => { + jsonrpsee_types::error::ErrorObject::owned( + INTERNAL_ERROR_CODE, + SERVER_ERROR_MSG, + Some(ErrorData::new(error)), + ) + } }, // Any other server error EngineApiError::TerminalTD { .. } | EngineApiError::TerminalBlockHash { .. } | EngineApiError::Internal(_) | - EngineApiError::GetPayloadError(_) => INTERNAL_ERROR_CODE, - }; - jsonrpsee_types::error::ErrorObject::owned(code, error.to_string(), None::<()>) + EngineApiError::GetPayloadError(_) => jsonrpsee_types::error::ErrorObject::owned( + INTERNAL_ERROR_CODE, + SERVER_ERROR_MSG, + Some(ErrorData::new(error)), + ), + } } } @@ -134,3 +199,60 @@ impl From for jsonrpsee_core::error::Error { jsonrpsee_core::error::Error::Call(error.into()) } } + +#[cfg(test)] +mod tests { + use super::*; + use reth_rpc_types::engine::ForkchoiceUpdateError; + + #[track_caller] + fn ensure_engine_rpc_error( + code: i32, + message: &str, + err: impl Into>, + ) { + let err = err.into(); + dbg!(&err); + assert_eq!(err.code(), code); + assert_eq!(err.message(), message); + } + + // Tests that engine errors are formatted correctly according to the engine API spec + // + #[test] + fn engine_error_rpc_error_test() { + ensure_engine_rpc_error( + UNSUPPORTED_FORK_CODE, + "Unsupported fork", + EngineApiError::UnsupportedFork, + ); + + ensure_engine_rpc_error( + REQUEST_TOO_LARGE_CODE, + "Too large request", + EngineApiError::PayloadRequestTooLarge { len: 0 }, + ); + + ensure_engine_rpc_error( + -38002, + "Invalid forkchoice state", + EngineApiError::ForkChoiceUpdate(BeaconForkChoiceUpdateError::ForkchoiceUpdateError( + ForkchoiceUpdateError::InvalidState, + )), + ); + + ensure_engine_rpc_error( + -38003, + "Invalid payload attributes", + EngineApiError::ForkChoiceUpdate(BeaconForkChoiceUpdateError::ForkchoiceUpdateError( + ForkchoiceUpdateError::UpdatedInvalidPayloadAttributes, + )), + ); + + ensure_engine_rpc_error( + UNKNOWN_PAYLOAD_CODE, + "Unknown payload", + EngineApiError::UnknownPayload, + ); + } +}