Skip to content

feat(rpc): structured JSON-RPC error codes for all endpoints #119

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.

Sign up for GitHub

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

Open
prestwich wants to merge 1 commit into
base: develop
Choose a base branch
from
+469 −364
Open

feat(rpc): structured JSON-RPC error codes for all endpoints #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
.DS_Store
Cargo.lock
.idea/
docs/superpowers/
docs/
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ signet-cold-mdbx = "0.6.5"
signet-storage-types = "0.6.5"

# ajj
ajj = { version = "0.6.0" }
ajj = "0.7.0"

# trevm
trevm = { version = "0.34.0", features = ["full_env_cfg"] }
Expand Down
99 changes: 50 additions & 49 deletions crates/rpc/src/debug/endpoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@ use crate::{
DebugError,
types::{TraceBlockParams, TraceTransactionParams},
},
eth::helpers::{CfgFiller, await_handler, response_tri},
eth::helpers::{CfgFiller, await_handler},
};
use ajj::{HandlerCtx, ResponsePayload};
use ajj::HandlerCtx;
use alloy::{
consensus::BlockHeader,
eips::BlockId,
rpc::types::trace::geth::{GethTrace, TraceResult},
};
use itertools::Itertools;
use signet_evm::EvmErrored;
use signet_hot::{HotKv, model::HotKvRead};
use signet_types::MagicSig;
use tracing::Instrument;
Expand All @@ -26,13 +25,13 @@ pub(super) async fn trace_block<T, H>(
hctx: HandlerCtx,
TraceBlockParams(id, opts): TraceBlockParams<T>,
ctx: StorageRpcCtx<H>,
) -> ResponsePayload<Vec<TraceResult>, DebugError>
) -> Result<Vec<TraceResult>, DebugError>
where
T: Into<BlockId>,
H: HotKv + Send + Sync + 'static,
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
{
let opts = response_tri!(opts.ok_or(DebugError::InvalidTracerConfig));
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;

// Acquire a tracing semaphore permit to limit concurrent debug
// requests. The permit is held for the entire handler lifetime and
Expand All @@ -44,41 +43,37 @@ where

let fut = async move {
let cold = ctx.cold();
let block_num = response_tri!(ctx.resolve_block_id(id).map_err(|e| {
let block_num = ctx.resolve_block_id(id).map_err(|e| {
tracing::warn!(error = %e, ?id, "block resolution failed");
DebugError::BlockNotFound(id)
}));
DebugError::Resolve(e)
})?;

let sealed =
response_tri!(ctx.resolve_header(BlockId::Number(block_num.into())).map_err(|e| {
tracing::warn!(error = %e, block_num, "header resolution failed");
DebugError::BlockNotFound(id)
}));
let sealed = ctx.resolve_header(BlockId::Number(block_num.into())).map_err(|e| {
tracing::warn!(error = %e, block_num, "header resolution failed");
DebugError::Resolve(e)
})?;

let Some(sealed) = sealed else {
return ResponsePayload::internal_error_message(
format!("block not found: {id}").into(),
);
return Err(DebugError::BlockNotFound(id));
};

let block_hash = sealed.hash();
let header = sealed.into_inner();

let txs = response_tri!(cold.get_transactions_in_block(block_num).await.map_err(|e| {
let txs = cold.get_transactions_in_block(block_num).await.map_err(|e| {
tracing::warn!(error = %e, block_num, "cold storage read failed");
DebugError::from(e)
}));
})?;

tracing::debug!(number = header.number, "Loaded block");

let mut frames = Vec::with_capacity(txs.len());

// State BEFORE this block.
let db =
response_tri!(ctx.revm_state_at_height(header.number.saturating_sub(1)).map_err(|e| {
tracing::warn!(error = %e, block_num, "hot storage read failed");
DebugError::from(e)
}));
let db = ctx.revm_state_at_height(header.number.saturating_sub(1)).map_err(|e| {
tracing::warn!(error = %e, block_num, "hot storage read failed");
DebugError::from(e)
})?;

let spec_id = ctx.spec_id_for_header(&header);
let mut evm = signet_evm::signet_evm(db, ctx.constants().clone());
Expand All @@ -100,30 +95,33 @@ where

let t = trevm.fill_tx(tx);
let frame;
(frame, trevm) = response_tri!(crate::debug::tracer::trace(t, &opts, tx_info));
(frame, trevm) = crate::debug::tracer::trace(t, &opts, tx_info)?;
frames.push(TraceResult::Success { result: frame, tx_hash: Some(*tx.tx_hash()) });

tracing::debug!(tx_index = idx, tx_hash = ?tx.tx_hash(), "Traced transaction");
}

ResponsePayload(Ok(frames))
Ok(frames)
}
.instrument(span);

await_handler!(@response_option hctx.spawn(fut))
await_handler!(
hctx.spawn(fut),
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
)
}

/// `debug_traceTransaction` handler.
pub(super) async fn trace_transaction<H>(
hctx: HandlerCtx,
TraceTransactionParams(tx_hash, opts): TraceTransactionParams,
ctx: StorageRpcCtx<H>,
) -> ResponsePayload<GethTrace, DebugError>
) -> Result<GethTrace, DebugError>
where
H: HotKv + Send + Sync + 'static,
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
{
let opts = response_tri!(opts.ok_or(DebugError::InvalidTracerConfig));
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;

// Held for the handler duration; dropped when the async block completes.
let _permit = ctx.acquire_tracing_permit().await;
Expand All @@ -134,37 +132,36 @@ where
let cold = ctx.cold();

// Look up the transaction and its containing block.
let confirmed = response_tri!(cold.get_tx_by_hash(tx_hash).await.map_err(|e| {
let confirmed = cold.get_tx_by_hash(tx_hash).await.map_err(|e| {
tracing::warn!(error = %e, %tx_hash, "cold storage read failed");
DebugError::from(e)
}));
})?;

let confirmed = response_tri!(confirmed.ok_or(DebugError::TransactionNotFound));
let confirmed = confirmed.ok_or(DebugError::TransactionNotFound(tx_hash))?;
let (_tx, meta) = confirmed.into_parts();

let block_num = meta.block_number();
let block_hash = meta.block_hash();

let block_id = BlockId::Number(block_num.into());
let sealed = response_tri!(ctx.resolve_header(block_id).map_err(|e| {
let sealed = ctx.resolve_header(block_id).map_err(|e| {
tracing::warn!(error = %e, block_num, "header resolution failed");
DebugError::BlockNotFound(block_id)
}));
let header = response_tri!(sealed.ok_or(DebugError::BlockNotFound(block_id))).into_inner();
})?;
let header = sealed.ok_or(DebugError::BlockNotFound(block_id))?.into_inner();

let txs = response_tri!(cold.get_transactions_in_block(block_num).await.map_err(|e| {
let txs = cold.get_transactions_in_block(block_num).await.map_err(|e| {
tracing::warn!(error = %e, block_num, "cold storage read failed");
DebugError::from(e)
}));
})?;

tracing::debug!(number = block_num, "Loaded containing block");

// State BEFORE this block.
let db =
response_tri!(ctx.revm_state_at_height(block_num.saturating_sub(1)).map_err(|e| {
tracing::warn!(error = %e, block_num, "hot storage read failed");
DebugError::from(e)
}));
let db = ctx.revm_state_at_height(block_num.saturating_sub(1)).map_err(|e| {
tracing::warn!(error = %e, block_num, "hot storage read failed");
DebugError::from(e)
})?;

let spec_id = ctx.spec_id_for_header(&header);
let mut evm = signet_evm::signet_evm(db, ctx.constants().clone());
Expand All @@ -175,15 +172,16 @@ where
let mut txns = txs.iter().enumerate().peekable();
for (_idx, tx) in txns.by_ref().peeking_take_while(|(_, t)| t.tx_hash() != &tx_hash) {
if MagicSig::try_from_signature(tx.signature()).is_some() {
return ResponsePayload::internal_error_message(
DebugError::TransactionNotFound.to_string().into(),
);
return Err(DebugError::TransactionNotFound(tx_hash));
}

trevm = response_tri!(trevm.run_tx(tx).map_err(EvmErrored::into_error)).accept_state();
trevm = trevm
.run_tx(tx)
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?
.accept_state();
}

let (index, tx) = response_tri!(txns.next().ok_or(DebugError::TransactionNotFound));
let (index, tx) = txns.next().ok_or(DebugError::TransactionNotFound(tx_hash))?;

let trevm = trevm.fill_tx(tx);

Expand All @@ -195,11 +193,14 @@ where
base_fee: header.base_fee_per_gas(),
};

let res = response_tri!(crate::debug::tracer::trace(trevm, &opts, tx_info)).0;
let res = crate::debug::tracer::trace(trevm, &opts, tx_info)?.0;

ResponsePayload(Ok(res))
Ok(res)
}
.instrument(span);

await_handler!(@response_option hctx.spawn(fut))
await_handler!(
hctx.spawn(fut),
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
)
}
57 changes: 40 additions & 17 deletions crates/rpc/src/debug/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
//! Error types for the debug namespace.

use alloy::eips::BlockId;
use alloy::{eips::BlockId, primitives::B256};
use std::borrow::Cow;

/// Errors that can occur in the `debug` namespace.
///
/// The [`serde::Serialize`] impl emits sanitized messages suitable for
/// API responses — internal storage details are not exposed to callers.
/// Use [`tracing`] to log the full error chain before constructing the
/// variant.
#[derive(Debug, thiserror::Error)]
pub enum DebugError {
/// Cold storage error.
Expand All @@ -16,28 +12,55 @@ pub enum DebugError {
/// Hot storage error.
#[error("hot storage error")]
Hot(#[from] signet_storage::StorageError),
/// Block resolution error.
#[error("resolve: {0}")]
Resolve(crate::config::resolve::ResolveError),
/// Invalid tracer configuration.
#[error("invalid tracer config")]
InvalidTracerConfig,
/// Unsupported tracer type.
#[error("unsupported: {0}")]
Unsupported(&'static str),
/// EVM execution error.
#[error("evm execution error")]
Evm(String),
/// EVM execution halted.
#[error("execution halted: {reason}")]
EvmHalt {
/// Debug-formatted halt reason.
reason: String,
},
/// Block not found.
#[error("block not found: {0}")]
BlockNotFound(BlockId),
/// Transaction not found.
#[error("transaction not found")]
TransactionNotFound,
#[error("transaction not found: {0}")]
TransactionNotFound(B256),
}

impl serde::Serialize for DebugError {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(&self.to_string())
impl ajj::IntoErrorPayload for DebugError {
type ErrData = ();

fn error_code(&self) -> i64 {
match self {
Self::Cold(_) | Self::Hot(_) | Self::EvmHalt { .. } => -32000,
Self::Resolve(r) => crate::eth::error::resolve_error_code(r),
Self::InvalidTracerConfig => -32602,
Self::Unsupported(_) => -32601,
Self::BlockNotFound(_) | Self::TransactionNotFound(_) => -32001,
}
}

fn error_message(&self) -> Cow<'static, str> {
match self {
Self::Cold(_) | Self::Hot(_) => "server error".into(),
Self::Resolve(r) => crate::eth::error::resolve_error_message(r),
Self::InvalidTracerConfig => "invalid tracer config".into(),
Self::Unsupported(msg) => format!("unsupported: {msg}").into(),
Self::EvmHalt { reason } => format!("execution halted: {reason}").into(),
Self::BlockNotFound(id) => format!("block not found: {id}").into(),
Self::TransactionNotFound(h) => format!("transaction not found: {h}").into(),
}
}

fn error_data(self) -> Option<Self::ErrData> {
None
}
}
18 changes: 9 additions & 9 deletions crates/rpc/src/debug/tracer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ where
NoopFrame::default().into(),
trevm
.run()
.map_err(|err| DebugError::Evm(err.into_error().to_string()))?
.map_err(|err| DebugError::EvmHalt { reason: err.into_error().to_string() })?
.accept_state(),
)),
GethDebugBuiltInTracerType::MuxTracer => trace_mux(&config.tracer_config, trevm, tx_info),
Expand All @@ -70,7 +70,7 @@ where
let mut four_byte = FourByteInspector::default();
let trevm = trevm
.try_with_inspector(&mut four_byte, |trevm| trevm.run())
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;
Ok((FourByteFrame::from(four_byte).into(), trevm.accept_state()))
}

Expand All @@ -90,7 +90,7 @@ where

let trevm = trevm
.try_with_inspector(&mut inspector, |trevm| trevm.run())
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;

let frame = inspector
.with_transaction_gas_limit(trevm.gas_limit())
Expand Down Expand Up @@ -118,7 +118,7 @@ where

let trevm = trevm
.try_with_inspector(&mut inspector, |trevm| trevm.run())
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;
let gas_limit = trevm.gas_limit();

// NB: state must be UNCOMMITTED for prestate diff computation.
Expand All @@ -128,7 +128,7 @@ where
.with_transaction_gas_limit(gas_limit)
.into_geth_builder()
.geth_prestate_traces(&result, &prestate_config, trevm.inner_mut_unchecked().db_mut())
.map_err(|err| DebugError::Evm(err.to_string()))?;
.map_err(|err| DebugError::EvmHalt { reason: err.to_string() })?;

// Equivalent to `trevm.accept_state()`.
trevm.inner_mut_unchecked().db_mut().commit(result.state);
Expand All @@ -155,7 +155,7 @@ where

let trevm = trevm
.try_with_inspector(&mut inspector, |trevm| trevm.run())
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;

let frame = inspector
.with_transaction_gas_limit(trevm.gas_limit())
Expand All @@ -178,18 +178,18 @@ where
tracer_config.clone().into_mux_config().map_err(|_| DebugError::InvalidTracerConfig)?;

let mut inspector = MuxInspector::try_from_config(mux_config)
.map_err(|err| DebugError::Evm(err.to_string()))?;
.map_err(|err| DebugError::EvmHalt { reason: err.to_string() })?;

let trevm = trevm
.try_with_inspector(&mut inspector, |trevm| trevm.run())
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;

// NB: state must be UNCOMMITTED for prestate diff computation.
let (result, mut trevm) = trevm.take_result_and_state();

let frame = inspector
.try_into_mux_frame(&result, trevm.inner_mut_unchecked().db_mut(), tx_info)
.map_err(|err| DebugError::Evm(err.to_string()))?;
.map_err(|err| DebugError::EvmHalt { reason: err.to_string() })?;

// Equivalent to `trevm.accept_state()`.
trevm.inner_mut_unchecked().db_mut().commit(result.state);
Expand Down
Loading