Skip to content

fix(rpc): surface StorageOverrideNotPermitted from seismic-evm#384

Open
HenryMBaldwin wants to merge 3 commits intoveridise-audit-april-2026from
hbai__disallow-storage-overrides
Open

fix(rpc): surface StorageOverrideNotPermitted from seismic-evm#384
HenryMBaldwin wants to merge 3 commits intoveridise-audit-april-2026from
hbai__disallow-storage-overrides

Conversation

@HenryMBaldwin
Copy link
Copy Markdown
Contributor

@HenryMBaldwin HenryMBaldwin commented Apr 23, 2026

Summary

Companion to seismic-evm#52, which rejects state / stateDiff overrides at the apply_account_override chokepoint to close an audit finding where signed reads could be evaluated against attacker-controlled storage.

  • Adds EthApiError::StorageOverrideNotPermitted(Address) next to the existing CodeOverrideNotPermitted variant.
  • Includes it in the invalid_params_rpc_err arm so it surfaces as -32602 (matching the code-override behavior).
  • Maps the underlying StateOverrideError::StorageOverrideNotPermitted through From<StateOverrideError> for EthApiError.
  • Bumps alloy-evm / alloy-seismic-evm to the seismic-evm branch tip (e644240) so the new variant resolves.

Merge order

Do not merge this PR until:

  1. seismic-evm#52 is merged to seismic.
  2. The alloy-evm / alloy-seismic-evm rev in Cargo.toml (lines 794-795) is re-bumped to the post-merge commit on seismic — the current pin points at the branch commit, which will differ from the merge commit (squash / merge / rebase).

Test plan

  • After dep re-bump: cargo check -p reth-rpc-eth-types builds clean for the error module changes. (Note: the base veridise-audit-april-2026 has pre-existing Url: serde errors unrelated to this PR — verified by building the baseline without these changes.)
  • Signed eth_call with a stateDiff override returns -32602 invalid params with the storage-override message.

henry-ai added 2 commits April 23, 2026 19:01
Companion to the seismic-evm change that rejects `state` / `stateDiff`
overrides at the `apply_account_override` chokepoint. Adds the
corresponding `EthApiError::StorageOverrideNotPermitted` variant,
includes it in the `invalid_params_rpc_err` arm so it surfaces as
-32602 (matching `CodeOverrideNotPermitted`), and maps the underlying
`StateOverrideError` variant through `From<StateOverrideError>`.

The seismic-evm dep bump is a separate change and must land before
this compiles.
Bumps `alloy-evm` / `alloy-seismic-evm` to e644240, which adds
`StateOverrideError::StorageOverrideNotPermitted` and rejects `state` /
`stateDiff` overrides at the `apply_account_override` chokepoint.
Required for the preceding commit to compile.
@HenryMBaldwin HenryMBaldwin requested a review from cdrappi as a code owner April 23, 2026 21:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

Updates seismic-evm dependency to pick up storage-override rejection and adds corresponding RPC error handling with e2e test coverage.

Phase 1
No critical issues found.

Phase 2

  • crates/rpc/rpc-eth-types/src/error/mod.rs:121 — New StorageOverrideNotPermitted error variant follows the existing pattern for CodeOverrideNotPermitted. The error mapping and JSON-RPC conversion are correctly implemented.

Phase 3
No polish issues to note.

The changes are well-structured:

  • Dependency update properly aligns both alloy-evm and alloy-seismic-evm to the same revision
  • Error handling follows the established pattern with proper JSON-RPC error code mapping
  • Test coverage mirrors the existing code override rejection tests with appropriate setup using SEISMIC_DEV chainspec and setup_test_node()
  • All three tests correctly validate the expected "storage overrides are not permitted" error message

LGTM

Adds three integration tests mirroring the existing code-override
coverage — asserts that `eth_call`, `eth_estimateGas`, and
`eth_simulateV1` reject requests carrying a `stateDiff` override and
surface the "storage overrides are not permitted" error through the
full RPC wiring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant