-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Include missing block id in error responses #7416
Changes from 36 commits
a4b73ea
a0183d6
3a91c22
acd3a4b
f9486f6
edd01d7
8ab7af3
fba8a46
b649820
e49d451
a8cc2d2
c593c04
973c45f
f1802db
e68a6ff
c9d8bfd
df1bfb1
d0dd461
ed30e89
e6bfa8c
bf8a553
e449fb4
4b5953b
7c27ad4
9a5ba17
144c222
23dd8f9
15b51f4
ff3007d
f3284c8
7753285
4307649
9ca4680
3be86c8
d2a040a
42ef3c4
c46c44a
18c1a68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,9 @@ use std::time::Duration; | |
|
||
use alloy_sol_types::decode_revert_reason; | ||
use reth_errors::RethError; | ||
use reth_primitives::{revm_primitives::InvalidHeader, Address, Bytes}; | ||
use reth_primitives::{revm_primitives::InvalidHeader, Address, BlockId, Bytes}; | ||
use reth_rpc_server_types::result::{ | ||
internal_rpc_err, invalid_params_rpc_err, rpc_err, rpc_error_with_code, | ||
block_id_to_str, internal_rpc_err, invalid_params_rpc_err, rpc_err, rpc_error_with_code, | ||
}; | ||
use reth_rpc_types::{ | ||
error::EthRpcErrorCode, request::TransactionInputError, BlockError, ToRpcError, | ||
|
@@ -37,18 +37,15 @@ pub enum EthApiError { | |
/// Errors related to the transaction pool | ||
#[error(transparent)] | ||
PoolError(RpcPoolError), | ||
/// When an unknown block number is encountered | ||
#[error("unknown block number")] | ||
UnknownBlockNumber, | ||
/// Thrown when querying for `finalized` or `safe` block before the merge transition is | ||
/// finalized, <https://github.com/ethereum/execution-apis/blob/6d17705a875e52c26826124c2a8a15ed542aeca2/src/schemas/block.yaml#L109> | ||
/// | ||
/// op-node now checks for either `Unknown block` OR `unknown block`: | ||
/// <https://github.com/ethereum-optimism/optimism/blob/3b374c292e2b05cc51b52212ba68dd88ffce2a3b/op-service/sources/l2_client.go#L105> | ||
/// | ||
/// TODO(#8045): Temporary, until a version of <https://github.com/ethereum-optimism/optimism/pull/10071> is pushed through that doesn't require this to figure out the EL sync status. | ||
Comment on lines
-43
to
-49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we're changing the error now, does this affect op logic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. input pls @joshie as author of ethereum-optimism/optimism#10071 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
#[error("unknown block")] | ||
UnknownSafeOrFinalizedBlock, | ||
/// Header not found for block hash/number/tag | ||
#[error("header not found")] | ||
emhane marked this conversation as resolved.
Show resolved
Hide resolved
|
||
HeaderNotFound(BlockId), | ||
/// Header range not found for start block hash/number/tag to end block hash/number/tag | ||
#[error("header range not found, start block {0:?}, end block {1:?}")] | ||
HeaderRangeNotFound(BlockId, BlockId), | ||
/// Receipts not found for block hash/number/tag | ||
#[error("receipts not found")] | ||
ReceiptsNotFound(BlockId), | ||
/// Thrown when an unknown block or transaction index is encountered | ||
#[error("unknown block or tx index")] | ||
UnknownBlockOrTxIndex, | ||
|
@@ -168,12 +165,23 @@ impl From<EthApiError> for jsonrpsee_types::error::ErrorObject<'static> { | |
EthApiError::EvmCustom(_) | | ||
EthApiError::EvmPrecompile(_) | | ||
EthApiError::InvalidRewardPercentiles => internal_rpc_err(error.to_string()), | ||
EthApiError::UnknownBlockNumber | EthApiError::UnknownBlockOrTxIndex => { | ||
EthApiError::UnknownBlockOrTxIndex => { | ||
rpc_error_with_code(EthRpcErrorCode::ResourceNotFound.code(), error.to_string()) | ||
} | ||
EthApiError::UnknownSafeOrFinalizedBlock => { | ||
rpc_error_with_code(EthRpcErrorCode::UnknownBlock.code(), error.to_string()) | ||
EthApiError::HeaderNotFound(id) | EthApiError::ReceiptsNotFound(id) => { | ||
rpc_error_with_code( | ||
EthRpcErrorCode::ResourceNotFound.code(), | ||
format!("{error}: {}", block_id_to_str(id)), | ||
) | ||
} | ||
EthApiError::HeaderRangeNotFound(start_id, end_id) => rpc_error_with_code( | ||
EthRpcErrorCode::ResourceNotFound.code(), | ||
format!( | ||
"{error}: start block: {}, end block: {}", | ||
block_id_to_str(start_id), | ||
block_id_to_str(end_id), | ||
), | ||
), | ||
EthApiError::Unsupported(msg) => internal_rpc_err(msg), | ||
EthApiError::InternalJsTracerError(msg) => internal_rpc_err(msg), | ||
EthApiError::InvalidParams(msg) => invalid_params_rpc_err(msg), | ||
|
@@ -216,15 +224,15 @@ impl From<reth_errors::ProviderError> for EthApiError { | |
fn from(error: reth_errors::ProviderError) -> Self { | ||
use reth_errors::ProviderError; | ||
match error { | ||
ProviderError::HeaderNotFound(_) | | ||
ProviderError::BlockHashNotFound(_) | | ||
ProviderError::BestBlockNotFound | | ||
ProviderError::BlockNumberForTransactionIndexNotFound | | ||
ProviderError::TotalDifficultyNotFound { .. } | | ||
ProviderError::UnknownBlockHash(_) => Self::UnknownBlockNumber, | ||
ProviderError::FinalizedBlockNotFound | ProviderError::SafeBlockNotFound => { | ||
Self::UnknownSafeOrFinalizedBlock | ||
ProviderError::HeaderNotFound(hash) => Self::HeaderNotFound(hash.into()), | ||
ProviderError::BlockHashNotFound(hash) | ProviderError::UnknownBlockHash(hash) => { | ||
Self::HeaderNotFound(hash.into()) | ||
} | ||
ProviderError::BestBlockNotFound => Self::HeaderNotFound(BlockId::latest()), | ||
ProviderError::BlockNumberForTransactionIndexNotFound => Self::UnknownBlockOrTxIndex, | ||
ProviderError::TotalDifficultyNotFound(num) => Self::HeaderNotFound(num.into()), | ||
ProviderError::FinalizedBlockNotFound => Self::HeaderNotFound(BlockId::finalized()), | ||
ProviderError::SafeBlockNotFound => Self::HeaderNotFound(BlockId::safe()), | ||
err => Self::Internal(err.into()), | ||
} | ||
} | ||
|
@@ -696,11 +704,41 @@ pub fn ensure_success(result: ExecutionResult) -> EthResult<Bytes> { | |
|
||
#[cfg(test)] | ||
mod tests { | ||
use revm_primitives::b256; | ||
|
||
use super::*; | ||
|
||
#[test] | ||
fn timed_out_error() { | ||
let err = EthApiError::ExecutionTimedOut(Duration::from_secs(10)); | ||
assert_eq!(err.to_string(), "execution aborted (timeout = 10s)"); | ||
} | ||
|
||
#[test] | ||
fn header_not_found_message() { | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::hash(b256!( | ||
"1a15e3c30cf094a99826869517b16d185d45831d3a494f01030b0001a9d3ebb9" | ||
))) | ||
.into(); | ||
assert_eq!(err.message(), "header not found: hash 0x1a15e3c30cf094a99826869517b16d185d45831d3a494f01030b0001a9d3ebb9"); | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::hash_canonical(b256!( | ||
"1a15e3c30cf094a99826869517b16d185d45831d3a494f01030b0001a9d3ebb9" | ||
))) | ||
.into(); | ||
assert_eq!(err.message(), "header not found: canonical hash 0x1a15e3c30cf094a99826869517b16d185d45831d3a494f01030b0001a9d3ebb9"); | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::number(100000)).into(); | ||
assert_eq!(err.message(), "header not found: number 0x186a0"); | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::latest()).into(); | ||
assert_eq!(err.message(), "header not found: latest"); | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::safe()).into(); | ||
assert_eq!(err.message(), "header not found: safe"); | ||
let err: jsonrpsee_types::error::ErrorObject<'static> = | ||
EthApiError::HeaderNotFound(BlockId::finalized()).into(); | ||
assert_eq!(err.message(), "header not found: finalized"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incorrect, this makes it incompatible with the otterscan API this must be u64 not hex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay this is actually no longer the case, so BlockId is correct now
https://otterscan.github.io/execution-apis/api-documentation/