fix: spec compliant engine api errors (#5188)

Cargo.lock

@@ -6484,6 +6484,7 @@ dependencies = [
  "reth-rpc-types",
  "reth-rpc-types-compat",
  "reth-tasks",
+ "serde",
  "thiserror",
  "tokio",
  "tracing",

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

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:
+/// <https://github.com/ethereum/execution-apis/blob/main/src/engine/common.md#errors>
+#[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<EngineApiError> 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<EngineApiError> 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<jsonrpsee_types::error::ErrorObject<'static>>,
+    ) {
+        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
+    // <https://github.com/ethereum/execution-apis/blob/main/src/engine/common.md#errors>
+    #[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,
+        );
+    }
+}