Skip to content

Commit 9f0f30f

Browse files
prestwichclaude
prestwich
and
claude
committed
feat(rpc): structured JSON-RPC error codes for all endpoints (ENG-1899)
Restructure EthError, DebugError, and SignetError with IntoErrorPayload impls. Migrate all 45 RPC endpoints from Result<T, String> / ResponsePayload<T, E> to Result<T, ErrorType> with proper JSON-RPC error codes: - Code 3: EVM reverts (with revert bytes in data field) - Code -32000: Server/infrastructure errors - Code -32001: Resource not found (block, tx, receipt) - Code -32602: Invalid client parameters - Code -32601: Unsupported operations Remove CallErrorData, response_tri! macro, EthError::into_string. Simplify await_handler! to single parameterized form. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8aaaa6f commit 9f0f30f

9 files changed

Lines changed: 529 additions & 328 deletions

File tree

‎crates/rpc/src/debug/endpoints.rs‎

Lines changed: 50 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,15 @@ use crate::{
66
DebugError,
77
types::{TraceBlockParams, TraceTransactionParams},
88
},
9-
eth::helpers::{CfgFiller, await_handler, response_tri},
9+
eth::helpers::{CfgFiller, await_handler},
1010
};
11-
use ajj::{HandlerCtx, ResponsePayload};
11+
use ajj::HandlerCtx;
1212
use alloy::{
1313
consensus::BlockHeader,
1414
eips::BlockId,
1515
rpc::types::trace::geth::{GethTrace, TraceResult},
1616
};
1717
use itertools::Itertools;
18-
use signet_evm::EvmErrored;
1918
use signet_hot::{HotKv, model::HotKvRead};
2019
use signet_types::MagicSig;
2120
use tracing::Instrument;
@@ -26,13 +25,13 @@ pub(super) async fn trace_block<T, H>(
2625
hctx: HandlerCtx,
2726
TraceBlockParams(id, opts): TraceBlockParams<T>,
2827
ctx: StorageRpcCtx<H>,
29-
) -> ResponsePayload<Vec<TraceResult>, DebugError>
28+
) -> Result<Vec<TraceResult>, DebugError>
3029
where
3130
T: Into<BlockId>,
3231
H: HotKv + Send + Sync + 'static,
3332
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
3433
{
35-
let opts = response_tri!(opts.ok_or(DebugError::InvalidTracerConfig));
34+
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
3635

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

4544
let fut = async move {
4645
let cold = ctx.cold();
47-
let block_num = response_tri!(ctx.resolve_block_id(id).map_err(|e| {
46+
let block_num = ctx.resolve_block_id(id).map_err(|e| {
4847
tracing::warn!(error = %e, ?id, "block resolution failed");
49-
DebugError::BlockNotFound(id)
50-
}));
48+
DebugError::Resolve(e)
49+
})?;
5150

52-
let sealed =
53-
response_tri!(ctx.resolve_header(BlockId::Number(block_num.into())).map_err(|e| {
54-
tracing::warn!(error = %e, block_num, "header resolution failed");
55-
DebugError::BlockNotFound(id)
56-
}));
51+
let sealed = ctx.resolve_header(BlockId::Number(block_num.into())).map_err(|e| {
52+
tracing::warn!(error = %e, block_num, "header resolution failed");
53+
DebugError::Resolve(e)
54+
})?;
5755

5856
let Some(sealed) = sealed else {
59-
return ResponsePayload::internal_error_message(
60-
format!("block not found: {id}").into(),
61-
);
57+
return Err(DebugError::BlockNotFound(id));
6258
};
6359

6460
let block_hash = sealed.hash();
6561
let header = sealed.into_inner();
6662

67-
let txs = response_tri!(cold.get_transactions_in_block(block_num).await.map_err(|e| {
63+
let txs = cold.get_transactions_in_block(block_num).await.map_err(|e| {
6864
tracing::warn!(error = %e, block_num, "cold storage read failed");
6965
DebugError::from(e)
70-
}));
66+
})?;
7167

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

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

7672
// State BEFORE this block.
77-
let db =
78-
response_tri!(ctx.revm_state_at_height(header.number.saturating_sub(1)).map_err(|e| {
79-
tracing::warn!(error = %e, block_num, "hot storage read failed");
80-
DebugError::from(e)
81-
}));
73+
let db = ctx.revm_state_at_height(header.number.saturating_sub(1)).map_err(|e| {
74+
tracing::warn!(error = %e, block_num, "hot storage read failed");
75+
DebugError::from(e)
76+
})?;
8277

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

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

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

109-
ResponsePayload(Ok(frames))
104+
Ok(frames)
110105
}
111106
.instrument(span);
112107

113-
await_handler!(@response_option hctx.spawn(fut))
108+
await_handler!(
109+
hctx.spawn(fut),
110+
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
111+
)
114112
}
115113

116114
/// `debug_traceTransaction` handler.
117115
pub(super) async fn trace_transaction<H>(
118116
hctx: HandlerCtx,
119117
TraceTransactionParams(tx_hash, opts): TraceTransactionParams,
120118
ctx: StorageRpcCtx<H>,
121-
) -> ResponsePayload<GethTrace, DebugError>
119+
) -> Result<GethTrace, DebugError>
122120
where
123121
H: HotKv + Send + Sync + 'static,
124122
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
125123
{
126-
let opts = response_tri!(opts.ok_or(DebugError::InvalidTracerConfig));
124+
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
127125

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

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

142-
let confirmed = response_tri!(confirmed.ok_or(DebugError::TransactionNotFound));
140+
let confirmed = confirmed.ok_or(DebugError::TransactionNotFound(tx_hash))?;
143141
let (_tx, meta) = confirmed.into_parts();
144142

145143
let block_num = meta.block_number();
146144
let block_hash = meta.block_hash();
147145

148146
let block_id = BlockId::Number(block_num.into());
149-
let sealed = response_tri!(ctx.resolve_header(block_id).map_err(|e| {
147+
let sealed = ctx.resolve_header(block_id).map_err(|e| {
150148
tracing::warn!(error = %e, block_num, "header resolution failed");
151149
DebugError::BlockNotFound(block_id)
152-
}));
153-
let header = response_tri!(sealed.ok_or(DebugError::BlockNotFound(block_id))).into_inner();
150+
})?;
151+
let header = sealed.ok_or(DebugError::BlockNotFound(block_id))?.into_inner();
154152

155-
let txs = response_tri!(cold.get_transactions_in_block(block_num).await.map_err(|e| {
153+
let txs = cold.get_transactions_in_block(block_num).await.map_err(|e| {
156154
tracing::warn!(error = %e, block_num, "cold storage read failed");
157155
DebugError::from(e)
158-
}));
156+
})?;
159157

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

162160
// State BEFORE this block.
163-
let db =
164-
response_tri!(ctx.revm_state_at_height(block_num.saturating_sub(1)).map_err(|e| {
165-
tracing::warn!(error = %e, block_num, "hot storage read failed");
166-
DebugError::from(e)
167-
}));
161+
let db = ctx.revm_state_at_height(block_num.saturating_sub(1)).map_err(|e| {
162+
tracing::warn!(error = %e, block_num, "hot storage read failed");
163+
DebugError::from(e)
164+
})?;
168165

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

183-
trevm = response_tri!(trevm.run_tx(tx).map_err(EvmErrored::into_error)).accept_state();
178+
trevm = trevm
179+
.run_tx(tx)
180+
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?
181+
.accept_state();
184182
}
185183

186-
let (index, tx) = response_tri!(txns.next().ok_or(DebugError::TransactionNotFound));
184+
let (index, tx) = txns.next().ok_or(DebugError::TransactionNotFound(tx_hash))?;
187185

188186
let trevm = trevm.fill_tx(tx);
189187

@@ -195,11 +193,14 @@ where
195193
base_fee: header.base_fee_per_gas(),
196194
};
197195

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

200-
ResponsePayload(Ok(res))
198+
Ok(res)
201199
}
202200
.instrument(span);
203201

204-
await_handler!(@response_option hctx.spawn(fut))
202+
await_handler!(
203+
hctx.spawn(fut),
204+
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
205+
)
205206
}

‎crates/rpc/src/debug/error.rs‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
//! Error types for the debug namespace.
22
3-
use alloy::eips::BlockId;
3+
use alloy::{
4+
eips::BlockId,
5+
primitives::{B256, Bytes},
6+
};
7+
use std::borrow::Cow;
48

59
/// Errors that can occur in the `debug` namespace.
6-
///
7-
/// The [`serde::Serialize`] impl emits sanitized messages suitable for
8-
/// API responses — internal storage details are not exposed to callers.
9-
/// Use [`tracing`] to log the full error chain before constructing the
10-
/// variant.
1110
#[derive(Debug, thiserror::Error)]
1211
pub enum DebugError {
1312
/// Cold storage error.
@@ -16,28 +15,66 @@ pub enum DebugError {
1615
/// Hot storage error.
1716
#[error("hot storage error")]
1817
Hot(#[from] signet_storage::StorageError),
18+
/// Block resolution error.
19+
#[error("resolve: {0}")]
20+
Resolve(crate::config::resolve::ResolveError),
1921
/// Invalid tracer configuration.
2022
#[error("invalid tracer config")]
2123
InvalidTracerConfig,
2224
/// Unsupported tracer type.
2325
#[error("unsupported: {0}")]
2426
Unsupported(&'static str),
25-
/// EVM execution error.
26-
#[error("evm execution error")]
27-
Evm(String),
27+
/// EVM execution reverted with output data.
28+
#[error("execution reverted")]
29+
EvmRevert {
30+
/// ABI-encoded revert reason bytes.
31+
output: Bytes,
32+
},
33+
/// EVM execution halted.
34+
#[error("execution halted: {reason}")]
35+
EvmHalt {
36+
/// Debug-formatted halt reason.
37+
reason: String,
38+
},
2839
/// Block not found.
2940
#[error("block not found: {0}")]
3041
BlockNotFound(BlockId),
3142
/// Transaction not found.
32-
#[error("transaction not found")]
33-
TransactionNotFound,
43+
#[error("transaction not found: {0}")]
44+
TransactionNotFound(B256),
3445
}
3546

36-
impl serde::Serialize for DebugError {
37-
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
38-
where
39-
S: serde::Serializer,
40-
{
41-
serializer.serialize_str(&self.to_string())
47+
impl ajj::IntoErrorPayload for DebugError {
48+
type ErrData = Bytes;
49+
50+
fn error_code(&self) -> i64 {
51+
match self {
52+
Self::Cold(_) | Self::Hot(_) | Self::EvmHalt { .. } => -32000,
53+
Self::Resolve(r) => crate::eth::error::resolve_error_code(r),
54+
Self::InvalidTracerConfig => -32602,
55+
Self::Unsupported(_) => -32601,
56+
Self::EvmRevert { .. } => 3,
57+
Self::BlockNotFound(_) | Self::TransactionNotFound(_) => -32001,
58+
}
59+
}
60+
61+
fn error_message(&self) -> Cow<'static, str> {
62+
match self {
63+
Self::Cold(_) | Self::Hot(_) => "server error".into(),
64+
Self::Resolve(r) => crate::eth::error::resolve_error_message(r),
65+
Self::InvalidTracerConfig => "invalid tracer config".into(),
66+
Self::Unsupported(msg) => format!("unsupported: {msg}").into(),
67+
Self::EvmRevert { .. } => "execution reverted".into(),
68+
Self::EvmHalt { reason } => format!("execution halted: {reason}").into(),
69+
Self::BlockNotFound(id) => format!("block not found: {id}").into(),
70+
Self::TransactionNotFound(h) => format!("transaction not found: {h}").into(),
71+
}
72+
}
73+
74+
fn error_data(self) -> Option<Self::ErrData> {
75+
match self {
76+
Self::EvmRevert { output } => Some(output),
77+
_ => None,
78+
}
4279
}
4380
}

‎crates/rpc/src/debug/tracer.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ where
5353
NoopFrame::default().into(),
5454
trevm
5555
.run()
56-
.map_err(|err| DebugError::Evm(err.into_error().to_string()))?
56+
.map_err(|err| DebugError::EvmHalt { reason: err.into_error().to_string() })?
5757
.accept_state(),
5858
)),
5959
GethDebugBuiltInTracerType::MuxTracer => trace_mux(&config.tracer_config, trevm, tx_info),
@@ -70,7 +70,7 @@ where
7070
let mut four_byte = FourByteInspector::default();
7171
let trevm = trevm
7272
.try_with_inspector(&mut four_byte, |trevm| trevm.run())
73-
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
73+
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;
7474
Ok((FourByteFrame::from(four_byte).into(), trevm.accept_state()))
7575
}
7676

@@ -90,7 +90,7 @@ where
9090

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

9595
let frame = inspector
9696
.with_transaction_gas_limit(trevm.gas_limit())
@@ -118,7 +118,7 @@ where
118118

119119
let trevm = trevm
120120
.try_with_inspector(&mut inspector, |trevm| trevm.run())
121-
.map_err(|e| DebugError::Evm(e.into_error().to_string()))?;
121+
.map_err(|e| DebugError::EvmHalt { reason: e.into_error().to_string() })?;
122122
let gas_limit = trevm.gas_limit();
123123

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

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

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

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

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

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

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

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

194194
// Equivalent to `trevm.accept_state()`.
195195
trevm.inner_mut_unchecked().db_mut().commit(result.state);

0 commit comments

Comments
 (0)