Skip to content

Commit f945ca7

Browse files
prestwichclaude
andcommitted
fix(rpc): address code review findings on debug namespace
- Replace EvmHalt with Internal for task panic/cancellation errors - Replace EvmHalt with Internal for resolve_evm_block error mapping - Use unwrap_or_default() for optional tracer options per Geth spec - Change pub(super) to pub(crate) on all debug handler functions - Replace glob import with explicit imports in web3 test module - Remove accidentally committed planning/spec documents Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c2a7712 commit f945ca7

4 files changed

Lines changed: 21 additions & 1832 deletions

File tree

crates/rpc/src/debug/endpoints.rs

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ where
7979
}
8080

8181
/// `debug_traceBlockByNumber` and `debug_traceBlockByHash` handler.
82-
pub(super) async fn trace_block<T, H>(
82+
pub(crate) async fn trace_block<T, H>(
8383
hctx: HandlerCtx,
8484
TraceBlockParams(id, opts): TraceBlockParams<T>,
8585
ctx: StorageRpcCtx<H>,
@@ -89,7 +89,7 @@ where
8989
H: HotKv + Send + Sync + 'static,
9090
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
9191
{
92-
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
92+
let opts = opts.unwrap_or_default();
9393

9494
// Acquire a tracing semaphore permit to limit concurrent debug
9595
// requests. The permit is held for the entire handler lifetime and
@@ -149,7 +149,7 @@ where
149149
}
150150

151151
/// `debug_traceTransaction` handler.
152-
pub(super) async fn trace_transaction<H>(
152+
pub(crate) async fn trace_transaction<H>(
153153
hctx: HandlerCtx,
154154
TraceTransactionParams(tx_hash, opts): TraceTransactionParams,
155155
ctx: StorageRpcCtx<H>,
@@ -158,7 +158,7 @@ where
158158
H: HotKv + Send + Sync + 'static,
159159
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
160160
{
161-
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
161+
let opts = opts.unwrap_or_default();
162162

163163
// Held for the handler duration; dropped when the async block completes.
164164
let _permit = ctx.acquire_tracing_permit().await;
@@ -240,7 +240,7 @@ where
240240
}
241241

242242
/// `debug_traceBlock` — trace all transactions in a raw RLP-encoded block.
243-
pub(super) async fn trace_block_rlp<H>(
243+
pub(crate) async fn trace_block_rlp<H>(
244244
hctx: HandlerCtx,
245245
(rlp_bytes, opts): (Bytes, Option<GethDebugTracingOptions>),
246246
ctx: StorageRpcCtx<H>,
@@ -249,7 +249,7 @@ where
249249
H: HotKv + Send + Sync + 'static,
250250
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
251251
{
252-
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
252+
let opts = opts.unwrap_or_default();
253253
let _permit = ctx.acquire_tracing_permit().await;
254254

255255
let span = tracing::debug_span!("traceBlock(RLP)", bytes_len = rlp_bytes.len());
@@ -288,18 +288,15 @@ where
288288
}
289289
.instrument(span);
290290

291-
await_handler!(
292-
hctx.spawn(fut),
293-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
294-
)
291+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
295292
}
296293

297294
/// `debug_getRawBlock` handler.
298295
///
299296
/// Resolves the given [`BlockId`], fetches header and transactions from cold
300297
/// storage, assembles them into an [`alloy::consensus::Block`], and returns
301298
/// the RLP-encoded bytes.
302-
pub(super) async fn get_raw_block<H>(
299+
pub(crate) async fn get_raw_block<H>(
303300
hctx: HandlerCtx,
304301
(id,): (BlockId,),
305302
ctx: StorageRpcCtx<H>,
@@ -346,17 +343,14 @@ where
346343
}
347344
.instrument(span);
348345

349-
await_handler!(
350-
hctx.spawn(fut),
351-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
352-
)
346+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
353347
}
354348

355349
/// `debug_getRawReceipts` handler.
356350
///
357351
/// Fetches all receipts for the given [`BlockId`] and returns a list of
358352
/// EIP-2718 encoded consensus receipt envelopes (one per transaction).
359-
pub(super) async fn get_raw_receipts<H>(
353+
pub(crate) async fn get_raw_receipts<H>(
360354
hctx: HandlerCtx,
361355
(id,): (BlockId,),
362356
ctx: StorageRpcCtx<H>,
@@ -407,16 +401,13 @@ where
407401
}
408402
.instrument(span);
409403

410-
await_handler!(
411-
hctx.spawn(fut),
412-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
413-
)
404+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
414405
}
415406

416407
/// `debug_getRawHeader` handler.
417408
///
418409
/// Resolves the given [`BlockId`] and returns the RLP-encoded block header.
419-
pub(super) async fn get_raw_header<H>(
410+
pub(crate) async fn get_raw_header<H>(
420411
hctx: HandlerCtx,
421412
(id,): (BlockId,),
422413
ctx: StorageRpcCtx<H>,
@@ -447,10 +438,7 @@ where
447438
}
448439
.instrument(span);
449440

450-
await_handler!(
451-
hctx.spawn(fut),
452-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
453-
)
441+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
454442
}
455443

456444
/// `debug_traceCall` — trace a call without submitting a transaction.
@@ -459,7 +447,7 @@ where
459447
/// from a [`alloy::rpc::types::TransactionRequest`], then routes through
460448
/// the tracer. State overrides are not supported in this initial
461449
/// implementation.
462-
pub(super) async fn debug_trace_call<H>(
450+
pub(crate) async fn debug_trace_call<H>(
463451
hctx: HandlerCtx,
464452
(request, block_id, opts): (
465453
alloy::rpc::types::TransactionRequest,
@@ -472,7 +460,7 @@ where
472460
H: HotKv + Send + Sync + 'static,
473461
<H::RoTx as HotKvRead>::Error: DBErrorMarker,
474462
{
475-
let opts = opts.ok_or(DebugError::InvalidTracerConfig)?;
463+
let opts = opts.unwrap_or_default();
476464
let _permit = ctx.acquire_tracing_permit().await;
477465

478466
let id = block_id.unwrap_or(BlockId::latest());
@@ -484,7 +472,7 @@ where
484472
let EvmBlockContext { header, db, spec_id } =
485473
ctx.resolve_evm_block(id).map_err(|e| match e {
486474
crate::eth::EthError::BlockNotFound(id) => DebugError::BlockNotFound(id),
487-
other => DebugError::EvmHalt { reason: other.to_string() },
475+
other => DebugError::Internal(other.to_string()),
488476
})?;
489477

490478
let mut evm = signet_evm::signet_evm(db, ctx.constants().clone());
@@ -507,17 +495,14 @@ where
507495
}
508496
.instrument(span);
509497

510-
await_handler!(
511-
hctx.spawn(fut),
512-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
513-
)
498+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
514499
}
515500

516501
/// `debug_getRawTransaction` handler.
517502
///
518503
/// Fetches the transaction by hash from cold storage and returns the
519504
/// EIP-2718 encoded bytes.
520-
pub(super) async fn get_raw_transaction<H>(
505+
pub(crate) async fn get_raw_transaction<H>(
521506
hctx: HandlerCtx,
522507
(hash,): (B256,),
523508
ctx: StorageRpcCtx<H>,
@@ -544,8 +529,5 @@ where
544529
}
545530
.instrument(span);
546531

547-
await_handler!(
548-
hctx.spawn(fut),
549-
DebugError::EvmHalt { reason: "task panicked or cancelled".into() }
550-
)
532+
await_handler!(hctx.spawn(fut), DebugError::Internal("task panicked or cancelled".into()))
551533
}

crates/rpc/src/web3/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ pub(crate) async fn sha3((data,): (Bytes,)) -> Result<B256, ()> {
2626

2727
#[cfg(test)]
2828
mod tests {
29-
use super::*;
29+
use super::{client_version, sha3};
30+
use alloy::primitives::{Bytes, keccak256};
3031

3132
#[tokio::test]
3233
async fn client_version_format() {

0 commit comments

Comments
 (0)