Skip to content

fix: move JournalInspectTr to host.rs with proper error stashing#172

Merged
Troublor merged 4 commits intomainfrom
william/fix/journal-inspect-error-stashing
Feb 28, 2026
Merged

fix: move JournalInspectTr to host.rs with proper error stashing#172
Troublor merged 4 commits intomainfrom
william/fix/journal-inspect-error-stashing

Conversation

@Troublor
Copy link
Collaborator

Summary

  • Bug fix: Custom instruction wrappers in instructions.rs previously had a bug where some paths halted with FatalExternalError without stashing the underlying DB error in ctx.error(), causing panics when revm tried to drain the stashed error.
  • Refactoring: Moved JournalInspectTr trait to host.rs with two implementations:
    • Journal<DB>: returns Result<T, DB::Error> for direct error propagation
    • MegaContext: stashes errors in self.error() as ContextError::Custom and returns Err(())
  • Simplified all 4 instruction call sites from manual error stashing to let Ok(...) = context.host.inspect_*(...) else { halt(FatalExternalError); return; }
  • Changed HostExt gas methods (sstore_set_storage_gas, new_account_storage_gas, create_contract_storage_gas) to return Option instead of Result, stashing errors internally (consistent with revm's Host trait pattern)
  • Replaced load_account with inspect_account_delegated in storage_gas_ext::create to avoid marking the creator account warm
  • Added ErrorInjectingDatabase test utility for simulating DB errors
  • Added 3 integration tests verifying that DB errors during inspect_storage (SSTORE) and inspect_account_delegated (CALL, STATICCALL) are properly stashed and surfaced as EVMError::Custom

Test plan

  • cargo check -p mega-evm passes
  • cargo check -p mega-evm --target riscv64imac-unknown-none-elf --no-default-features passes (no_std)
  • cargo test -p mega-evm — all tests pass (including 3 new db_error tests)
  • cargo clippy --workspace --lib --examples --tests --benches --all-features --locked — no warnings
  • cargo fmt --all --check — formatted

Previously, custom instruction wrappers in instructions.rs manually
stashed DB errors via match + ctx.error() when calling journal inspect
methods. This was error-prone and had a bug where some paths halted with
FatalExternalError without stashing the error first, causing panics.

This refactoring:
- Defines JournalInspectTr trait in host.rs with two implementations:
  - Journal<DB>: returns Result<T, DB::Error> directly
  - MegaContext: stashes errors in self.error() and returns Err(())
- Simplifies all 4 instruction call sites to let Ok(...) = ... else { halt }
- Changes HostExt gas methods to return Option (stash errors internally)
- Replaces load_account with inspect_account_delegated in CREATE
- Adds ErrorInjectingDatabase test utility and 3 integration tests
@Troublor Troublor merged commit 8588f57 into main Feb 28, 2026
2 checks passed
@Troublor Troublor deleted the william/fix/journal-inspect-error-stashing branch February 28, 2026 00:56
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.

2 participants