-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add signet-rpc-storage crate with ETH RPC endpoints #75
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.
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
base: main
Are you sure you want to change the base?
Conversation
| @@ -0,0 +1,578 @@ | |||
| //! Integration tests for the `signet-rpc-storage` ETH RPC endpoints. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.
|
[Claude Code] Review SummaryWhat's here
Issues flagged (inline comments)
Codebase health
|
Add the foundational scaffolding for the signet-rpc-storage crate, which provides an Ethereum JSON-RPC server backed by signet-storage's unified storage backend, independent of reth's FullNodeComponents. This includes: - Workspace dependency additions (signet-storage, signet-cold, signet-hot, signet-storage-types) - StorageRpcCtx context struct with Arc<Inner> pattern - BlockTags atomic block tag tracker for Latest/Safe/Finalized - Block ID and block tag resolution utilities - Stub eth module (endpoints to be added in follow-up) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement all ETH namespace JSON-RPC endpoints backed by cold/hot storage instead of reth. Converts eth.rs placeholder into eth/ directory module with error types, helpers, and 24 supported endpoint handlers: - Simple queries: blockNumber, chainId - Block queries: getBlockByHash/Number, getBlockReceipts, headers - Transaction queries: getTransactionByHash, getTransactionReceipt, raw txs - Account state (hot storage): getBalance, getStorageAt, getCode, getTransactionCount - EVM execution: call, estimateGas (via signet-evm/trevm) - Transaction submission: sendRawTransaction (via TxCache) - Logs: getLogs with bloom filter matching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 23 integration tests covering all endpoint categories: simple queries, block/transaction lookups, account state, logs, and error cases. Tests exercise the router through the axum service layer using tower's oneshot(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- send_raw_transaction: log warning on forwarding failure instead of silently discarding the error - get_logs: reject reversed block ranges (from > to) with an explicit error instead of silently returning empty results - build_receipt_envelope: remove catch-all arm so new TxType variants from alloy produce a compile error instead of a runtime panic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Regular txs execute before system txs, not the other way around. Drive-by from #74 (comment) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4341320 to
601c156
Compare
crates/rpc-storage/src/resolve.rs
Outdated
| ) -> Result<u64, ResolveError> { | ||
| match id { | ||
| BlockId::Number(tag) => resolve_block_number_or_tag(tag, tags), | ||
| BlockId::Hash(h) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this cause a double header lookup when calling getHeader? can we remove that?
- Extract `resolve_evm_block` method on `StorageRpcCtx` to deduplicate the block resolution + header fetch + revm db creation shared by `call()` and `estimate_gas()`. Resolves headers directly (by hash or by tag→number) to avoid redundant cold storage lookups. - Replace glob import `use endpoints::*` with explicit imports. - Remove unused `revm_state()` method from `StorageRpcCtx`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ounds - Move `resolve_block_id` and `resolve_block_number_or_tag` from free functions in resolve.rs to `resolve_block_id` and `resolve_block_tag` methods on `StorageRpcCtx`. This eliminates repeated `ctx.tags()` and `ctx.cold()` threading at every call site. - `resolve_block_tag` returns `u64` directly (infallible) instead of `Result`, simplifying callers like `get_logs`. - Remove `H::RoTx: Send + Sync + 'static` bounds from all endpoint functions, router, and ctx methods — the trait already provides these. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the bare `rpc_gas_cap` constructor parameter with a `StorageRpcConfig` struct that bundles all RPC configuration. This moves `max_blocks_per_filter` from a hard-coded constant to a configurable value, adds `max_logs_per_response` enforcement in `eth_getLogs`, and pre-creates a tracing semaphore for future debug endpoint concurrency limiting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| let mut all_logs = Vec::new(); | ||
|
|
||
| for (chunk_start, chunk_end) in | ||
| BlockRangeInclusiveIter::new(from..=to, MAX_HEADERS_RANGE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is basically ported from reth, so i'll take a deeper look here
| // --------------------------------------------------------------------------- | ||
|
|
||
| pub(crate) async fn not_supported() -> ResponsePayload<(), ()> { | ||
| ResponsePayload::internal_error_message(Cow::Borrowed("Method not supported.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be method_not_found() rather than an internal error, so callers don't retry.
| &meta, | ||
| receipt, | ||
| gas_used, | ||
| 0, // log_index_offset: single receipt, no prior logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this offset be relative to all the logs for the block as opposed to just this tx?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes 🤮
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gonna have to update Storage to return this info
Add gas oracle, filters, subscriptions, debug tracing, and signet namespaces to signet-rpc-storage. Port 15 endpoints from the old reth-backed signet-rpc crate to the storage-backed architecture. New modules: - gas_oracle: cold-storage gas price oracle (suggest_tip_cap) - interest/: filter manager, subscription manager, block notifications - debug/: traceBlockByNumber, traceBlockByHash, traceTransaction - signet/: sendOrder, callBundle Wired eth endpoints: gasPrice, maxPriorityFeePerGas, feeHistory, newFilter, newBlockFilter, uninstallFilter, getFilterChanges, getFilterLogs, subscribe, unsubscribe. Integration tests cover gas/fee queries, filter lifecycle, and debug tracing with noop tracer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
signet-rpc-storagecrate providing Ethereum JSON-RPC endpoints backed bysignet-storage(hot + cold), independent of reth'sFullNodeComponentsStorageRpcCtxwrapsUnifiedStorage,BlockTags, system constants, optionalTxCache, and RPC gas capBlockTagsprovides atomic latest/safe/finalized block tracking with sync/async resolutionblockNumber,chainIdgetBlockByHash/Number,getBlockTransactionCount*,getBlockReceipts,getBlockHeader*getTransactionByHash,getRawTransactionByHash,*ByBlockAndIndex,getTransactionReceiptgetBalance,getStorageAt,getTransactionCount,getCodecall,estimateGasviasignet-evm/trevmsendRawTransactionviaTxCachegetLogswith bloom filter matchingReceiptContextsupportTest plan
cargo clippy -p signet-rpc-storage --all-features --all-targets— cleancargo clippy -p signet-rpc-storage --no-default-features --all-targets— cleancargo +nightly fmt— cleancargo t -p signet-rpc-storage— all tests passcargo doc -p signet-rpc-storage— docs build successfully🤖 Generated with Claude Code