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

* feat(rpc): structured JSON-RPC error codes and ajj 0.7.0 (ENG-1899)

Replace string-based RPC errors with structured error types implementing
ajj's IntoErrorPayload trait. Each namespace (eth, debug, signet) gets a
dedicated error enum with spec-compliant error codes.

- Add EthError, DebugError, SignetError with thiserror + IntoErrorPayload
- Replace response_tri! macro and string errors with typed error propagation
- Add resolve_block_id/resolve_header helpers with ResolveError
- Update ajj dependency from git branch to published 0.7.0
- Preserve hot storage reader consistency from #118

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(rpc): address review feedback on error types and test coverage

- Use DebugError::Resolve(e) instead of BlockNotFound for resolve_header
  failures in trace_transaction
- Add Internal variant to DebugError and SignetError for task panic
  fallbacks, replacing misuse of EvmHalt and Timeout
- Change SignetError::Resolve to wrap ResolveError instead of String,
  with proper error code/message delegation
- Add From<EthError> for SignetError to handle resolve_evm_block errors
- Expand error code tests to cover all variants of EthError, DebugError,
  and SignetError

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: prestwich

.gitignore

@@ -2,4 +2,4 @@
 .DS_Store
 Cargo.lock
 .idea/
-docs/superpowers/
+docs/

Cargo.toml

@@ -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"] }

crates/rpc/src/debug/endpoints.rs

@@ -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;
@@ -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
@@ -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());
@@ -100,30 +95,30 @@ 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::Internal("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;
@@ -134,37 +129,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();
+            DebugError::Resolve(e)
+        })?;
+        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());
@@ -175,15 +169,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);
 
@@ -195,11 +190,11 @@ 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::Internal("task panicked or cancelled".into()))
 }

crates/rpc/src/debug/error.rs

@@ -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.
@@ -16,28 +12,122 @@ 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),
+    /// Internal server error.
+    #[error("{0}")]
+    Internal(String),
 }
 
-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 { .. } | Self::Internal(_) => -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::Internal(msg) => Cow::Owned(msg.clone()),
+            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
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::config::resolve::ResolveError;
+    use ajj::IntoErrorPayload;
+
+    #[test]
+    fn cold_storage_error() {
+        let err = DebugError::Cold(signet_cold::ColdStorageError::NotFound("test".into()));
+        assert_eq!(err.error_code(), -32000);
+        assert_eq!(err.error_message(), "server error");
+    }
+
+    #[test]
+    fn resolve_hash_not_found() {
+        let err = DebugError::Resolve(ResolveError::HashNotFound(B256::ZERO));
+        assert_eq!(err.error_code(), -32001);
+        assert!(err.error_message().contains("block hash not found"));
+    }
+
+    #[test]
+    fn invalid_tracer_config() {
+        let err = DebugError::InvalidTracerConfig;
+        assert_eq!(err.error_code(), -32602);
+        assert_eq!(err.error_message(), "invalid tracer config");
+    }
+
+    #[test]
+    fn unsupported() {
+        let err = DebugError::Unsupported("flatCallTracer");
+        assert_eq!(err.error_code(), -32601);
+        assert!(err.error_message().contains("flatCallTracer"));
+    }
+
+    #[test]
+    fn evm_halt() {
+        let err = DebugError::EvmHalt { reason: "OutOfGas".into() };
+        assert_eq!(err.error_code(), -32000);
+        assert!(err.error_message().contains("OutOfGas"));
+    }
+
+    #[test]
+    fn block_not_found() {
+        let err = DebugError::BlockNotFound(BlockId::latest());
+        assert_eq!(err.error_code(), -32001);
+        assert!(err.error_message().contains("block not found"));
+    }
+
+    #[test]
+    fn transaction_not_found() {
+        let err = DebugError::TransactionNotFound(B256::ZERO);
+        assert_eq!(err.error_code(), -32001);
+        assert!(err.error_message().contains("transaction not found"));
+    }
+
+    #[test]
+    fn internal() {
+        let err = DebugError::Internal("task panicked or cancelled".into());
+        assert_eq!(err.error_code(), -32000);
+        assert!(err.error_message().contains("task panicked"));
     }
 }