feat: add signet-rpc-storage crate with ETH RPC endpoints

[prestwich]

Summary

• Adds the signet-rpc-storage crate providing Ethereum JSON-RPC endpoints backed by signet-storage (hot + cold), independent of reth's FullNodeComponents
• StorageRpcCtx wraps UnifiedStorage, BlockTags, system constants, optional TxCache, and RPC gas cap
• BlockTags provides atomic latest/safe/finalized block tracking with sync/async resolution
• Implements 24 supported ETH namespace endpoints:
  • Simple queries: blockNumber, chainId
  • Block queries: getBlockByHash/Number, getBlockTransactionCount*, getBlockReceipts, getBlockHeader*
  • Transaction queries: getTransactionByHash, getRawTransactionByHash, *ByBlockAndIndex, getTransactionReceipt
  • Account state (hot storage): getBalance, getStorageAt, getTransactionCount, getCode
  • EVM execution: call, estimateGas via signet-evm/trevm
  • Transaction submission: sendRawTransaction via TxCache
  • Logs: getLogs with bloom filter matching
• 30 unsupported methods return explicit "not supported" errors
• Uses signet-storage 0.3.0 from crates.io

Test plan

• cargo clippy -p signet-rpc-storage --all-features --all-targets — clean
• cargo clippy -p signet-rpc-storage --no-default-features --all-targets — clean
• cargo +nightly fmt — clean
• cargo t -p signet-rpc-storage — 33 tests pass
• cargo doc -p signet-rpc-storage — docs build successfully

🤖 Generated with Claude Code

[prestwich]

[Claude Code]

Test coverage gap: eth_call and eth_estimateGas are the only two supported endpoints without integration tests. They require full EVM state setup (deployed contract bytecode, proper storage) to exercise meaningfully. Worth adding as a follow-up.

The current tests cover all 22 other supported endpoints plus error paths.

[prestwich]

[Claude Code]

Review Summary

What's here

• 24 ETH JSON-RPC endpoints (22 supported + 23 explicitly unsupported)
• Clean crate structure: ctx, resolve, eth/{endpoints, helpers, error, mod}
• 23 integration tests covering all supported endpoints except eth_call and eth_estimateGas
• No TODOs, no todo!(), no unimplemented!(), no naked unwrap() in production code
• Clippy clean on both --all-features and --no-default-features

Issues flagged (inline comments)

Severity	Location	Issue
High	endpoints.rs:647-651	sendRawTransaction fire-and-forget — forwarding errors silently discarded
Medium	endpoints.rs:720-722	getLogs range validation — reversed from > to silently accepted
Low	helpers.rs:274-276	unreachable!() in build_receipt_envelope — future alloy TxType variants will panic
Info	tests/eth_rpc.rs	eth_call and eth_estimateGas not yet tested (EVM state setup needed)

Codebase health

• Zero TODO/FIXME comments
• Zero panic!/unwrap in non-test code
• Consistent error handling pattern (.map_err(|e| e.to_string()))
• Good use of tokio::try_join! for concurrent cold storage reads
• Tracing spans on EVM execution paths (eth_call, eth_estimateGas)

[Fraser999]

I'm guessing the intent here is to return batches of 1000, but the way BlockRangeInclusiveIter is implemented, we'd need to pass MAX_HEADERS_RANGE - 1 as the step value.

I think it might be worth changing the BlockRangeInclusiveIter to treat the step arg the same way as the std lib - i.e. to make it mean "batch size" and not increment it by 1 internally. That would mean we'd also need to update BlockRangeInclusiveIter::next() to have let end = (start + self.step - 1).min(self.end);

[prestwich]

this is basically ported from reth, so i'll take a deeper look here

[Fraser999]

This should probably be method_not_found() rather than an internal error, so callers don't retry.

[prestwich]

is there a way to set all of these atomically at once

[prestwich]

are there ways that we can DRY endpoint logic or definitions?

[prestwich]

can we add syncing status information to this struct

[prestwich]

add newlines after each pub use

[prestwich]

these are not appropriately namespaced. we need to nest them with a prefix, as done in signet-rpc

[prestwich]

these functions do not need to exist. we only need the main fully built router at crate root

[prestwich]

make a config/ module folder containing the contents of ctx.rs gas_oracle.rs config.rs and resolve.rs

[prestwich]

the 1000 should be a config parameter

[prestwich]

is this error swallowed?

[prestwich]

how and where are resources managed for this? are these ever cloned?

[prestwich]

make comprehensive documentation for this module

[prestwich]

include details about resource usage expectations, etc

[prestwich]

where is this used? how can we remove it

[prestwich]

where is this permit dropped?

[prestwich]

we are collecting these frames only to immediately serailize them. is there a way we could feed these to the serializer via an iterator instead?

[prestwich]

the same question applies to other instances where we return vecs

[prestwich]

can we avoid string-typing these?

[prestwich]

we should also avoid leaking the full error into the API return. ensure that there is a log of the full error, and the API returns a simple description

[prestwich]

add many more comments throughout the endpoint code, explaining what is happening and why

[prestwich]

where is this used? these reads may be expensive. do we cache these?

[prestwich]

could we use the resolve context method here?

[prestwich]

could this use the reoslve method as well?

[prestwich]

we can remove this, as this is default behavior when a method is unregistered.

[prestwich]

remove all non-supported. leave a comment with a list of non-supported methods. move supported methods up

[prestwich]

update these. instead list unsupported methods and why

[prestwich]

we're going to add an api to ajj to get permits. this code should be restored to prior state then modified to use SubscriptionNotificstion

[prestwich]

remove this. it's always known, and never needs to be a struct prop.

[prestwich]

we're going to re-add permitting to ajj. this code should be revised to the previous permit-acquisition behavior, modified to use SubscriptionNotification

[prestwich]

why state? do we need that wrapper for anything?

[prestwich]

[Claude Code]

PR Review: signet-rpc-storage crate

The crate is well-organized and test coverage is solid. However, there are two likely bugs (leaked thread in SubCleanerTask, unsubscribe is a no-op), a conformance gap (eth_syncing), and a design issue with spawn_blocking being used for async work. See inline comments for details.

Design-level observations (not tied to a specific line):

StorageRpcConfig should have a builder — Per project conventions: "A struct is considered complex enough to warrant a builder if it has more than 4 fields." StorageRpcConfig has 11 fields.

Duplicated Either enum — filters.rs and subs.rs both define an Either enum with nearly identical semantics but different variants. Should be unified or renamed (FilterItem / SubscriptionItem).

Duplicated buffer/output types — FilterOutput and SubscriptionBuffer are structurally identical VecDeque-backed enums with Log/Block variants, len(), is_empty(), extend(), pop_front(). This could be a single generic type.

Error handling via String is inconsistent — Most endpoints return Result<T, String>, losing structured error info, while run_call/estimate_gas/create_access_list return ResponsePayload<T, CallErrorData>, and debug endpoints return ResponsePayload<T, DebugError>. The crate defines three good error types (EthError, DebugError, SignetError) but most endpoints discard them with .map_err(|e| e.to_string()).

[prestwich]

[Claude Code]

Bug: SubCleanerTask::spawn never terminates when SubscriptionManager is dropped.

Compare with the correct pattern in FilterCleanTask::spawn (filters.rs:288-299), which checks Weak::upgrade() and breaks when it returns None. This version skips the retain but loops forever, leaking an OS thread per SubscriptionManager drop.

fn spawn(self) {
    std::thread::spawn(move || {
        loop {
            std::thread::sleep(self.interval);
            if let Some(inner) = self.inner.upgrade() {
                inner.tasks.retain(|_, task| !task.is_cancelled());
            }
            // missing: else { break; }
        }
    });
}

[prestwich]

[Claude Code]

Bug: subscribe() never inserts the CancellationToken into self.tasks.

The token is created and cloned into the SubscriptionTask, but never stored in the DashMap. This means unsubscribe() will always return false (the remove finds nothing), and the SubCleanerTask has nothing to clean. The subscription can never be cancelled by the server.

// Should be:
self.tasks.insert(id, token.clone());

[prestwich]

[Claude Code]

eth_syncing should return false (when synced) or a sync-status object (when syncing), not method_not_found. You already have BlockTags::sync_status() and SyncStatus wired up for exactly this. Any client that calls eth_syncing — virtually all of them do — gets a confusing error instead of the expected false.

[prestwich]

[Claude Code]

gas_price, max_priority_fee_per_gas, and fee_history all use hctx.spawn_blocking for tasks that contain .await calls (cold storage reads). spawn_blocking is for CPU-bound work — using it for IO-bound async work means the task runs on a blocking thread pool that doesn't poll futures efficiently. These should use hctx.spawn or hctx.spawn_with_ctx.

[prestwich]

[Claude Code]

cumulative_gas and tx_idx are never reset between percentile iterations. This is correct for sorted-ascending traversal, but once tx_idx increments past tx_gas_and_tip.len(), the while loop exits immediately for all subsequent percentiles, and tx_gas_and_tip.get(tx_idx) returns None → unwrap_or_default() → zero. Higher percentiles silently return 0 for blocks with few transactions.

[prestwich]

[Claude Code]

Nit: #[allow(dead_code)] on Either, FilterOutput impls, and is_filter method. If they're not used, remove them. If they're part of the intended API, use them. allow(dead_code) on internal types is a code smell suggesting premature API surface.

[prestwich]

[Claude Code]

Nit: .ok_or("receipt found but transaction missing") uses a string literal error. This should be a proper error variant on EthError.

[prestwich]

[Claude Code]

Nit: This iterates headers sequentially, making one get_transactions_in_block call per block. For the default 20-block lookback, that's 20 sequential cold-storage reads. These could be parallelized with futures::join_all or similar.

[prestwich]

[Claude Code]

Nit: tokio-stream and tokio-util specify exact versions ("0.1", "0.7") rather than using workspace dependencies. Inconsistent with the rest of the manifest.

[prestwich]

[Claude Code]

Nit: base_fee_per_blob_gas and blob_gas_used_ratio are allocated as vec![0; ...] even though Signet has no blob transactions. Could be Vec::new() or empty if FeeHistory permits.