chore(rpc): add proper engine_newPayload error messages (#2118)

This commit is contained in:
Matthias Seitz
2023-04-05 11:59:55 +02:00
committed by GitHub
parent 1913536e81
commit b5f4283f79
6 changed files with 73 additions and 42 deletions

2
Cargo.lock generated
View File

@@ -5137,6 +5137,8 @@ version = "0.1.0"
dependencies = [
"assert_matches",
"futures",
"jsonrpsee-core",
"jsonrpsee-types",
"reth-beacon-consensus",
"reth-interfaces",
"reth-primitives",

View File

@@ -186,10 +186,10 @@ where
/// When the Consensus layer receives a new block via the consensus gossip protocol,
/// the transactions in the block are sent to the execution layer in the form of a
/// `ExecutionPayload`. The Execution layer executes the transactions and validates the
/// [`ExecutionPayload`]. The Execution layer executes the transactions and validates the
/// state in the block header, then passes validation data back to Consensus layer, that
/// adds the block to the head of its own blockchain and attests to it. The block is then
/// broadcasted over the consensus p2p network in the form of a "Beacon block".
/// broadcast over the consensus p2p network in the form of a "Beacon block".
///
/// These responses should adhere to the [Engine API Spec for
/// `engine_newPayload`](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#specification).

View File

@@ -21,6 +21,8 @@ tokio-stream = "0.1"
# misc
thiserror = "1.0.37"
jsonrpsee-types = "0.16"
jsonrpsee-core = "0.16"
[dev-dependencies]
reth-interfaces = { path = "../../interfaces", features = ["test-utils"] }

View File

@@ -98,7 +98,7 @@ impl<Client: HeaderProvider + BlockProvider + StateProviderFactory + EvmEnvProvi
}
if start == 0 || count == 0 {
return Err(EngineApiError::InvalidParams)
return Err(EngineApiError::InvalidBodiesRange { start, count })
}
let mut result = Vec::with_capacity(count as usize);
@@ -295,7 +295,10 @@ mod tests {
handle.send_message(EngineApiMessage::GetPayloadBodiesByRange(
start, count, result_tx,
));
assert_matches!(result_rx.await, Ok(Err(EngineApiError::InvalidParams)));
assert_matches!(
result_rx.await,
Ok(Err(EngineApiError::InvalidBodiesRange { .. }))
);
}
}

View File

@@ -1,3 +1,4 @@
use jsonrpsee_types::error::{INTERNAL_ERROR_CODE, INVALID_PARAMS_CODE};
use reth_beacon_consensus::BeaconEngineError;
use reth_primitives::{H256, U256};
use thiserror::Error;
@@ -22,9 +23,23 @@ pub enum EngineApiError {
/// The length that was requested.
len: u64,
},
/// The params are invalid.
#[error("Invalid params")]
InvalidParams,
/// Thrown if engine_getPayloadBodiesByRangeV1 contains an invalid range
#[error("invalid start or count, start: {start} count: {count}")]
InvalidBodiesRange {
/// Start of the range
start: u64,
/// requested number of items
count: u64,
},
/// Thrown if engine_forkchoiceUpdatedV1 contains withdrawals
#[error("withdrawals not supported in V1")]
WithdrawalsNotSupportedInV1,
/// Thrown if engine_forkchoiceUpdated contains no withdrawals after Shanghai
#[error("no withdrawals post-shanghai")]
NoWithdrawalsPostShanghai,
/// Thrown if engine_forkchoiceUpdated contains withdrawals before Shanghai
#[error("withdrawals pre-shanghai")]
HasWithdrawalsPreShanghai,
/// Terminal total difficulty mismatch during transition configuration exchange.
#[error(
"Invalid transition terminal total difficulty. Execution: {execution}. Consensus: {consensus}"
@@ -52,3 +67,29 @@ pub enum EngineApiError {
#[error(transparent)]
Internal(Box<dyn std::error::Error + Send + Sync>),
}
impl From<EngineApiError> for jsonrpsee_types::error::CallError {
fn from(error: EngineApiError) -> Self {
let code = match error {
EngineApiError::InvalidBodiesRange { .. } |
EngineApiError::WithdrawalsNotSupportedInV1 |
EngineApiError::NoWithdrawalsPostShanghai |
EngineApiError::HasWithdrawalsPreShanghai => INVALID_PARAMS_CODE,
EngineApiError::PayloadUnknown => UNKNOWN_PAYLOAD_CODE,
EngineApiError::PayloadRequestTooLarge { .. } => REQUEST_TOO_LARGE_CODE,
// Any other server error
_ => INTERNAL_ERROR_CODE,
};
jsonrpsee_types::error::CallError::Custom(jsonrpsee_types::error::ErrorObject::owned(
code,
error.to_string(),
None::<()>,
))
}
}
impl From<EngineApiError> for jsonrpsee_core::error::Error {
fn from(error: EngineApiError) -> Self {
jsonrpsee_types::error::CallError::from(error).into()
}
}

View File

@@ -1,15 +1,10 @@
use crate::result::rpc_err;
use async_trait::async_trait;
use jsonrpsee::{
core::{Error, RpcResult as Result},
types::error::INVALID_PARAMS_CODE,
};
use jsonrpsee::core::{Error, RpcResult as Result};
use reth_interfaces::consensus::ForkchoiceState;
use reth_primitives::{BlockHash, ChainSpec, Hardfork, H64, U64};
use reth_rpc_api::EngineApiServer;
use reth_rpc_engine_api::{
EngineApiError, EngineApiHandle, EngineApiMessage, EngineApiMessageVersion, EngineApiResult,
REQUEST_TOO_LARGE_CODE, UNKNOWN_PAYLOAD_CODE,
};
use reth_rpc_types::engine::{
ExecutionPayload, ExecutionPayloadBodies, ForkchoiceUpdated, PayloadAttributes, PayloadStatus,
@@ -18,18 +13,6 @@ use reth_rpc_types::engine::{
use std::sync::Arc;
use tokio::sync::oneshot::{self, Receiver};
fn to_rpc_error<E: Into<EngineApiError>>(error: E) -> Error {
let error = error.into();
let code = match error {
EngineApiError::InvalidParams => INVALID_PARAMS_CODE,
EngineApiError::PayloadUnknown => UNKNOWN_PAYLOAD_CODE,
EngineApiError::PayloadRequestTooLarge { .. } => REQUEST_TOO_LARGE_CODE,
// Any other server error
_ => jsonrpsee::types::error::INTERNAL_ERROR_CODE,
};
rpc_err(code, error.to_string(), None)
}
/// The server implementation of Engine API
pub struct EngineApi {
/// Chain spec
@@ -65,15 +48,19 @@ impl EngineApi {
match version {
EngineApiMessageVersion::V1 => {
if is_shanghai || has_withdrawals {
return Err(EngineApiError::InvalidParams)
if has_withdrawals {
return Err(EngineApiError::WithdrawalsNotSupportedInV1)
}
if is_shanghai {
return Err(EngineApiError::NoWithdrawalsPostShanghai)
}
}
EngineApiMessageVersion::V2 => {
let shanghai_with_no_withdrawals = is_shanghai && !has_withdrawals;
let not_shanghai_with_withdrawals = !is_shanghai && has_withdrawals;
if shanghai_with_no_withdrawals || not_shanghai_with_withdrawals {
return Err(EngineApiError::InvalidParams)
if is_shanghai && !has_withdrawals {
return Err(EngineApiError::NoWithdrawalsPostShanghai)
}
if !is_shanghai && has_withdrawals {
return Err(EngineApiError::HasWithdrawalsPreShanghai)
}
}
};
@@ -87,13 +74,13 @@ impl EngineApi {
rx: Receiver<std::result::Result<T, E>>,
) -> Result<T> {
let _ = self.engine_tx.send(msg);
rx.await.map_err(|err| Error::Custom(err.to_string()))?.map_err(|err| to_rpc_error(err))
Ok(rx.await.map_err(|err| Error::Custom(err.to_string()))?.map_err(Into::into)?)
}
}
#[async_trait]
impl EngineApiServer for EngineApi {
/// Handler for `engine_getPayloadV1`
/// Handler for `engine_newPayloadV1`
/// See also <https://github.com/ethereum/execution-apis/blob/8db51dcd2f4bdfbd9ad6e4a7560aac97010ad063/src/engine/specification.md#engine_newpayloadv1>
/// Caution: This should not accept the `withdrawals` field
async fn new_payload_v1(&self, payload: ExecutionPayload) -> Result<PayloadStatus> {
@@ -101,21 +88,19 @@ impl EngineApiServer for EngineApi {
EngineApiMessageVersion::V1,
payload.timestamp.as_u64(),
payload.withdrawals.is_some(),
)
.map_err(to_rpc_error)?;
)?;
let (tx, rx) = oneshot::channel();
self.delegate_request(EngineApiMessage::NewPayload(payload, tx), rx).await
}
/// Handler for `engine_getPayloadV2`
/// Handler for `engine_newPayloadV1`
/// See also <https://github.com/ethereum/execution-apis/blob/8db51dcd2f4bdfbd9ad6e4a7560aac97010ad063/src/engine/specification.md#engine_newpayloadv1>
async fn new_payload_v2(&self, payload: ExecutionPayload) -> Result<PayloadStatus> {
self.validate_withdrawals_presence(
EngineApiMessageVersion::V2,
payload.timestamp.as_u64(),
payload.withdrawals.is_some(),
)
.map_err(to_rpc_error)?;
)?;
let (tx, rx) = oneshot::channel();
self.delegate_request(EngineApiMessage::NewPayload(payload, tx), rx).await
}
@@ -134,8 +119,7 @@ impl EngineApiServer for EngineApi {
EngineApiMessageVersion::V1,
attrs.timestamp.as_u64(),
attrs.withdrawals.is_some(),
)
.map_err(to_rpc_error)?;
)?;
}
let (tx, rx) = oneshot::channel();
self.delegate_request(
@@ -157,8 +141,7 @@ impl EngineApiServer for EngineApi {
EngineApiMessageVersion::V2,
attrs.timestamp.as_u64(),
attrs.withdrawals.is_some(),
)
.map_err(to_rpc_error)?;
)?;
}
let (tx, rx) = oneshot::channel();
self.delegate_request(