Skip to content

Commit

Permalink
feat: Include missing block id in error responses (#7416)
Browse files Browse the repository at this point in the history
Co-authored-by: Emilia Hane <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
  • Loading branch information
3 people committed Sep 7, 2024
1 parent e7defb2 commit 7a20b41
Show file tree
Hide file tree
Showing 25 changed files with 301 additions and 251 deletions.
263 changes: 125 additions & 138 deletions Cargo.lock

Large diffs are not rendered by default.

14 changes: 8 additions & 6 deletions crates/optimism/rpc/src/eth/pending_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use reth_chainspec::ChainSpec;
use reth_evm::ConfigureEvm;
use reth_node_api::{FullNodeComponents, NodeTypes};
use reth_primitives::{
revm_primitives::BlockEnv, BlockNumber, Receipt, SealedBlockWithSenders, B256,
revm_primitives::BlockEnv, BlockNumber, BlockNumberOrTag, Receipt, SealedBlockWithSenders, B256,
};
use reth_provider::{
BlockReader, BlockReaderIdExt, ChainSpecProvider, EvmEnvProvider, ExecutionOutcome,
Expand Down Expand Up @@ -58,19 +58,21 @@ where
.provider()
.latest_header()
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(BlockNumberOrTag::Latest.into()))?;
let block_id = latest.hash().into();
let block = self
.provider()
.block_with_senders(latest.hash().into(), Default::default())
.block_with_senders(block_id, Default::default())
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?
.ok_or(EthApiError::HeaderNotFound(block_id.into()))?
.seal(latest.hash());

let receipts = self
.provider()
.receipts_by_block(block.hash().into())
.receipts_by_block(block_id)
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::ReceiptsNotFound(block_id.into()))?;

Ok(Some((block, receipts)))
}

Expand Down
4 changes: 3 additions & 1 deletion crates/optimism/rpc/src/eth/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ where
.get_block_and_receipts(meta.block_hash)
.await
.map_err(Self::Error::from_eth_err)?
.ok_or(Self::Error::from_eth_err(EthApiError::UnknownBlockNumber))?;
.ok_or(Self::Error::from_eth_err(EthApiError::HeaderNotFound(
meta.block_hash.into(),
)))?;

let block = block.unseal();
let l1_block_info =
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-api/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub trait DebugApi {
async fn debug_trace_call(
&self,
request: TransactionRequest,
block_number: Option<BlockId>,
block_id: Option<BlockId>,
opts: Option<GethDebugTracingCallOptions>,
) -> RpcResult<GethTrace>;

Expand Down
4 changes: 2 additions & 2 deletions crates/rpc/rpc-api/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,14 +238,14 @@ pub trait EngineEthApi<B: RpcObject> {
async fn call(
&self,
request: TransactionRequest,
block_number: Option<BlockId>,
block_id: Option<BlockId>,
state_overrides: Option<StateOverride>,
block_overrides: Option<Box<BlockOverrides>>,
) -> RpcResult<Bytes>;

/// Returns code at a given address at given block number.
#[method(name = "getCode")]
async fn get_code(&self, address: Address, block_number: Option<BlockId>) -> RpcResult<Bytes>;
async fn get_code(&self, address: Address, block_id: Option<BlockId>) -> RpcResult<Bytes>;

/// Returns information about a block by hash.
#[method(name = "getBlockByHash")]
Expand Down
4 changes: 2 additions & 2 deletions crates/rpc/rpc-api/src/otterscan.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use jsonrpsee::{core::RpcResult, proc_macros::rpc};
use reth_primitives::{Address, Bytes, TxHash, B256};
use reth_primitives::{Address, BlockId, Bytes, TxHash, B256};
use reth_rpc_types::{
trace::otterscan::{
BlockDetails, ContractCreator, InternalOperation, OtsBlockTransactions, TraceEntry,
Expand All @@ -24,7 +24,7 @@ pub trait Otterscan {

/// Check if a certain address contains a deployed code.
#[method(name = "hasCode")]
async fn has_code(&self, address: Address, block_number: Option<u64>) -> RpcResult<bool>;
async fn has_code(&self, address: Address, block_id: Option<BlockId>) -> RpcResult<bool>;

/// Very simple API versioning scheme. Every time we add a new capability, the number is
/// incremented. This allows for Otterscan to check if the node contains all API it
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-builder/tests/it/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ where
OtterscanClient::get_header_by_number(client, block_number).await.unwrap();

OtterscanClient::has_code(client, address, None).await.unwrap();
OtterscanClient::has_code(client, address, Some(block_number)).await.unwrap();
OtterscanClient::has_code(client, address, Some(block_number.into())).await.unwrap();

OtterscanClient::get_api_level(client).await.unwrap();

Expand Down
7 changes: 2 additions & 5 deletions crates/rpc/rpc-eth-api/src/helpers/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,12 @@ pub trait EthBlocks: LoadBlock {
) -> impl Future<Output = Result<Option<RpcBlock<Self::NetworkTypes>>, Self::Error>> + Send
{
async move {
let block = match self.block_with_senders(block_id).await? {
Some(block) => block,
None => return Ok(None),
};
let Some(block) = self.block_with_senders(block_id).await? else { return Ok(None) };
let block_hash = block.hash();
let total_difficulty = EthBlocks::provider(self)
.header_td_by_number(block.number)
.map_err(Self::Error::from_eth_err)?
.ok_or(EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(block_id))?;
let block = from_block(block.unseal(), total_difficulty, full.into(), Some(block_hash))
.map_err(Self::Error::from_eth_err)?;
Ok(Some(block))
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-api/src/helpers/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub trait EthCall: Call + LoadPendingBlock {
self.block_with_senders(target_block)
)?;

let Some(block) = block else { return Err(EthApiError::UnknownBlockNumber.into()) };
let block = block.ok_or(EthApiError::HeaderNotFound(target_block))?;
let gas_limit = self.call_gas_limit();

// we're essentially replaying the transactions in the block here, hence we need the
Expand Down
16 changes: 6 additions & 10 deletions crates/rpc/rpc-eth-api/src/helpers/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,10 @@ pub trait EthFees: LoadFee {
block_count = block_count.saturating_sub(1);
}

let Some(end_block) = LoadFee::provider(self)
let end_block = LoadFee::provider(self)
.block_number_for_id(newest_block.into())
.map_err(Self::Error::from_eth_err)?
else {
return Err(EthApiError::UnknownBlockNumber.into())
};
.ok_or(EthApiError::HeaderNotFound(newest_block.into()))?;

// need to add 1 to the end block to get the correct (inclusive) range
let end_block_plus = end_block + 1;
Expand Down Expand Up @@ -293,13 +291,11 @@ pub trait LoadFee: LoadBlock {
let base_fee = self
.block(BlockNumberOrTag::Pending.into())
.await?
.ok_or(EthApiError::UnknownBlockNumber)?
.ok_or(EthApiError::HeaderNotFound(BlockNumberOrTag::Pending.into()))?
.base_fee_per_gas
.ok_or_else(|| {
EthApiError::InvalidTransaction(
RpcInvalidTransactionError::TxTypeNotSupported,
)
})?;
.ok_or(EthApiError::InvalidTransaction(
RpcInvalidTransactionError::TxTypeNotSupported,
))?;
U256::from(base_fee)
}
};
Expand Down
3 changes: 2 additions & 1 deletion crates/rpc/rpc-eth-api/src/helpers/pending_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use reth_revm::{
database::StateProviderDatabase, state_change::post_block_withdrawals_balance_increments,
};
use reth_rpc_eth_types::{EthApiError, PendingBlock, PendingBlockEnv, PendingBlockEnvOrigin};
use reth_rpc_types::BlockNumberOrTag;
use reth_transaction_pool::{BestTransactionsAttributes, TransactionPool};
use reth_trie::HashedPostState;
use revm::{db::states::bundle_state::BundleRetention, DatabaseCommit, State};
Expand Down Expand Up @@ -82,7 +83,7 @@ pub trait LoadPendingBlock: EthApiTypes {
.provider()
.latest_header()
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(BlockNumberOrTag::Latest.into()))?;

let (mut latest_header, block_hash) = latest.split();
// child block
Expand Down
5 changes: 3 additions & 2 deletions crates/rpc/rpc-eth-api/src/helpers/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ pub trait LoadReceipt: EthApiTypes + Send + Sync {
receipt: Receipt,
) -> impl Future<Output = Result<AnyTransactionReceipt, Self::Error>> + Send {
async move {
let hash = meta.block_hash;
// get all receipts for the block
let all_receipts = self
.cache()
.get_receipts(meta.block_hash)
.get_receipts(hash)
.await
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(hash.into()))?;

Ok(ReceiptBuilder::new(&tx, meta, &receipt, &all_receipts)?.build())
}
Expand Down
7 changes: 4 additions & 3 deletions crates/rpc/rpc-eth-api/src/helpers/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub trait EthState: LoadState + SpawnBlocking {
let block_number = LoadState::provider(self)
.block_number_for_id(block_id)
.map_err(Self::Error::from_eth_err)?
.ok_or(EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(block_id))?;
let max_window = self.max_proof_window();
if chain_info.best_number.saturating_sub(block_number) > max_window {
return Err(EthApiError::ExceedsMaxProofWindow.into())
Expand All @@ -108,7 +108,8 @@ pub trait EthState: LoadState + SpawnBlocking {
let _permit = self
.acquire_owned()
.await
.map_err(|err| EthApiError::Internal(RethError::other(err)))?;
.map_err(RethError::other)
.map_err(EthApiError::Internal)?;
self.spawn_blocking_io(move |this| {
let state = this.state_at_block_id(block_id)?;
let storage_keys = keys.iter().map(|key| key.0).collect::<Vec<_>>();
Expand Down Expand Up @@ -222,7 +223,7 @@ pub trait LoadState: EthApiTypes {
let block_hash = LoadPendingBlock::provider(self)
.block_hash_for_id(at)
.map_err(Self::Error::from_eth_err)?
.ok_or_else(|| EthApiError::UnknownBlockNumber)?;
.ok_or(EthApiError::HeaderNotFound(at))?;
let (cfg, env) = self
.cache()
.get_evm_env(block_hash)
Expand Down
10 changes: 6 additions & 4 deletions crates/rpc/rpc-eth-api/src/helpers/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use reth_rpc_types::{
EIP1559TransactionRequest, EIP2930TransactionRequest, EIP4844TransactionRequest,
LegacyTransactionRequest,
},
AnyTransactionReceipt, TransactionInfo, TransactionRequest, TypedTransactionRequest,
AnyTransactionReceipt, BlockNumberOrTag, TransactionInfo, TransactionRequest,
TypedTransactionRequest,
};
use reth_rpc_types_compat::transaction::{from_recovered, from_recovered_with_block_context};
use reth_transaction_pool::{PoolTransaction, TransactionOrigin, TransactionPool};
Expand Down Expand Up @@ -249,7 +250,7 @@ pub trait EthTransactions: LoadTransaction {
}

let Ok(high) = LoadBlock::provider(self).best_block_number() else {
return Err(EthApiError::UnknownBlockNumber.into());
return Err(EthApiError::HeaderNotFound(BlockNumberOrTag::Latest.into()).into());
};

// Perform a binary search over the block range to find the block in which the sender's
Expand All @@ -262,7 +263,8 @@ pub trait EthTransactions: LoadTransaction {
})
.await?;

self.block_with_senders(num.into())
let block_id = num.into();
self.block_with_senders(block_id)
.await?
.and_then(|block| {
let block_hash = block.hash();
Expand All @@ -284,7 +286,7 @@ pub trait EthTransactions: LoadTransaction {
from_recovered_with_block_context(tx, tx_info)
})
})
.ok_or(EthApiError::UnknownBlockNumber.into())
.ok_or(EthApiError::HeaderNotFound(block_id).into())
.map(Some)
}
}
Expand Down
88 changes: 63 additions & 25 deletions crates/rpc/rpc-eth-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
#[error("unknown block")]
UnknownSafeOrFinalizedBlock,
/// Header not found for block hash/number/tag
#[error("header not found")]
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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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()),
}
}
Expand Down Expand Up @@ -712,11 +720,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");
}
}
Loading

0 comments on commit 7a20b41

Please sign in to comment.