Conversation
WalkthroughThis pull request consolidates Ethereum spec test fixtures to a single v5.4.0 source, replaces the intrinsic_gas API with an intrinsic_gas_and_gas_floor dual-return API, removes the EIP-7623 module, and extends transaction validation and assertion logic to handle AccessList/AuthorizationList restrictions and new InvalidTxReason variants. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant State as PostState
participant Tx as Transaction
participant Validator as Validation
participant Assertions as Assertions
TestRunner->>State: prepare post-state & tx data
TestRunner->>Tx: provide config & state for intrinsic check
Tx->>Tx: intrinsic_gas_and_gas_floor(config, state)
Tx-->>Validator: (intrinsic_gas, floor_gas)
Validator->>Validator: check tx type vs spec (AccessList / AuthorizationList)
alt invalid for spec
Validator-->>Assertions: return InvalidTxReason (AccessListNotSupported / AuthorizationListNotSupportedForCreate)
else valid
Validator-->>Assertions: proceed with call/create and expected exit reason
end
Assertions->>TestRunner: assert result (spec-aware messages)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@evm-tests/src/assertions.rs`:
- Around line 437-444: The current catch-all for ExitReason::Succeed and
ExitReason::Revert prints an expected-exception value but does not fail the
test, allowing mismatches to pass; restore the original assertion logic around
expect_exception (or replace it with explicit handling) so that when handling
ExitReason::Succeed or ExitReason::Revert the code asserts
expect_exception.is_none() and panics with a clear message including
expect_exception, reason and name (use the existing symbols expect_exception,
reason, name, and the ExitReason::Succeed/ExitReason::Revert branches to locate
the code) — alternatively, if you prefer explicit handling, add a branch that,
when expect_exception.is_some(), panics with a descriptive message instead of
only printing.
🧹 Nitpick comments (1)
.github/workflows/rust.yml (1)
8-11: Consider adding tarball integrity verification.While the workflow now depends on a specific external tarball, supply-chain safety can be improved by verifying its integrity. However, note that the SHA256 checksum for this artifact is not published in standard locations (release assets or checksum files). To implement this, you would need to:
- Manually download
fixtures_stable.tar.gzfrom the v5.4.0 release- Compute its SHA256:
sha256sum fixtures_stable.tar.gz- Add the hash as a workflow variable and verify it in the download step
This adds robustness but requires a one-time manual setup. If the release publishes checksums in the future, this becomes straightforward to implement.
Summary by CodeRabbit
Bug Fixes
Refactor