✨ refactor: replace std.debug.assert with if (\!condition) unreachable #1796

roninjin10 · 2025-06-08T00:31:27Z

Description

Replace std.debug.assert with if statements and unreachable in EVM opcodes implementation. This change maintains the same runtime behavior while making the assertion logic more explicit.

The pattern std.debug.assert(frame.stack.size >= N) is replaced with if (frame.stack.size < N) unreachable, and similarly std.debug.assert(frame.stack.size < Stack.CAPACITY) is replaced with if (frame.stack.size >= Stack.CAPACITY) unreachable.

Testing

The code changes maintain identical runtime behavior since both approaches (assert vs if+unreachable) result in the same program termination when the condition is violated. This refactoring improves code clarity by making the assertion checks more explicit.

Additional Information

I read the contributing docs (if this is your first contribution)

Your ENS/address:

Summary by CodeRabbit

Bug Fixes
- Improved stack size validation for all EVM opcodes by enforcing runtime checks, ensuring that operations abort immediately if stack requirements are not met, rather than only in debug builds.
Chores
- Updated internal error handling for stack operations to use unconditional runtime checks, increasing reliability and consistency in execution behavior.

Replace all instances of std.debug.assert with explicit if (\!condition) unreachable; pattern across EVM opcode modules for more explicit error handling. Changed files: - src/evm/opcodes/arithmetic.zig - src/evm/opcodes/bitwise.zig - src/evm/opcodes/comparison.zig - src/evm/opcodes/control.zig - src/evm/opcodes/memory.zig - src/evm/opcodes/stack.zig - src/evm/opcodes/storage.zig Original prompt: "Grep src/**/*.zig for any use of std.debug.assert. Instead use if (\!thing) unreachable; which is the same thing. Make a commit. Include this prompt in the commit message. Use emoji conventional commit. You are not allowed to commit unless \`zig build test-all\` passes" 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

vercel · 2025-06-08T00:31:30Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
tevm-monorepo-app	✅ Ready (Inspect)	Visit Preview	Jun 8, 2025 0:33am

2 Skipped Deployments

Name	Status	Preview	Updated (UTC)
tevm-monorepo-tevm	⬜️ Ignored (Inspect)		Jun 8, 2025 0:33am
node	⬜️ Skipped (Inspect)		Jun 8, 2025 0:33am

changeset-bot · 2025-06-08T00:31:32Z

⚠️ No Changeset found

Latest commit: 1096cea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2025-06-08T00:31:36Z

Walkthrough

All EVM opcode implementation files have been updated to replace std.debug.assert stack size checks with explicit runtime checks using if statements that call unreachable when the stack size is insufficient. This change enforces stack preconditions at runtime for arithmetic, bitwise, comparison, control flow, memory, stack, and storage opcodes.

Changes

Files	Change Summary
src/evm/opcodes/arithmetic.zig src/evm/opcodes/bitwise.zig src/evm/opcodes/comparison.zig src/evm/opcodes/control.zig src/evm/opcodes/memory.zig src/evm/opcodes/stack.zig src/evm/opcodes/storage.zig	Replaced all `std.debug.assert` stack size checks with explicit runtime checks using `if (...) unreachable;`. No changes to public API or function signatures.

Sequence Diagram(s)

sequenceDiagram
    participant VM
    participant Stack

    VM->>Stack: Check stack.size
    alt stack.size < required
        VM->>VM: unreachable (abort execution)
    else stack.size >= required
        VM->>Stack: Pop/peek operands
        VM->>VM: Execute opcode logic
        VM->>Stack: Push result (if applicable)
    end

Possibly related PRs

feat: implement complete stack ipmlementation #1695: Refactored the stack implementation to use a fixed-size array and explicit size management, which is directly relevant to enforcing stack size checks at runtime in opcode implementations.

Poem

A hop and a skip, the stack must be right,
No more debug-only checks hidden from sight!
Now runtime will catch, if things go astray—
Unreachable called, the VM hops away.
With every opcode, the stack's in control,
Zig-zagging safely, that's every bunny's goal!
🐇✨

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between beaf9c2 and 1096cea.

📒 Files selected for processing (7)

src/evm/opcodes/arithmetic.zig (11 hunks)
src/evm/opcodes/bitwise.zig (8 hunks)
src/evm/opcodes/comparison.zig (6 hunks)
src/evm/opcodes/control.zig (6 hunks)
src/evm/opcodes/memory.zig (12 hunks)
src/evm/opcodes/stack.zig (3 hunks)
src/evm/opcodes/storage.zig (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (3)

GitHub Check: CI Checks
GitHub Check: Nx Cloud - Main Job
GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (21)

src/evm/opcodes/stack.zig (3)

38-38: LGTM: Correct conversion of capacity check.

The logic correctly converts std.debug.assert(frame.stack.size < Stack.CAPACITY) to an explicit runtime check that calls unreachable when the stack is at capacity.

71-72: LGTM: Proper dual validation for dup operation.

Both stack size checks are correctly converted:

Minimum size validation before accessing stack elements

Capacity validation before pushing duplicate element

The logic ensures safe execution of the unsafe dup operation.

92-92: LGTM: Correct minimum size validation for swap.

The logic correctly converts the assertion to ensure at least n + 1 elements are available before performing the swap operation.

src/evm/opcodes/storage.zig (4)

45-45: LGTM: Proper stack validation for SLOAD.

The check ensures at least one element is available before the unsafe peek operation to read the storage slot.

83-83: LGTM: Correct validation for SSTORE operation.

The check ensures at least two elements (value and slot) are available before the unsafe pop2_unsafe() operation.

126-126: LGTM: Appropriate validation for TLOAD.

The check ensures sufficient stack elements before accessing the transient storage slot.

149-149: LGTM: Proper precondition for TSTORE.

The validation ensures two elements are available before the unsafe pop operation for transient storage.

src/evm/opcodes/comparison.zig (2)

18-18: LGTM: Consistent validation for binary comparison operations.

All binary comparison operations (LT, GT, SLT, SGT, EQ) correctly validate that at least 2 elements are available before performing unsafe stack operations.

Also applies to: 40-40, 62-62, 87-87, 112-112

133-133: LGTM: Proper validation for unary operation.

The ISZERO operation correctly validates that at least 1 element is available before the unsafe peek operation.

src/evm/opcodes/memory.zig (4)

24-24: LGTM: Proper validation for load operations.

Both MLOAD and CALLDATALOAD correctly validate that at least 1 element is available before accessing the offset from the stack.

Also applies to: 187-187

58-58: LGTM: Correct validation for store operations.

Both MSTORE and MSTORE8 properly validate that at least 2 elements (offset and value) are available before the unsafe pop2_unsafe() operation.

Also applies to: 92-92

127-127: LGTM: Appropriate capacity checks for size operations.

All size operations (MSIZE, CALLDATASIZE, CODESIZE, RETURNDATASIZE) correctly check stack capacity before pushing the result, preventing stack overflow.

Also applies to: 223-223, 278-278, 333-333

146-146: LGTM: Proper validation for copy operations.

All copy operations (MCOPY, CALLDATACOPY, CODECOPY, RETURNDATACOPY) correctly validate that at least 3 elements are available before accessing the required parameters.

Also applies to: 237-237, 292-292, 347-347

src/evm/opcodes/bitwise.zig (2)

13-13: LGTM! Excellent refactoring to improve runtime safety.

Replacing std.debug.assert with explicit unreachable checks is a significant improvement:

Unconditional enforcement: Stack size invariants are now enforced in all builds, not just debug builds

Better optimization: The unreachable hints help the compiler optimize better

Clearer intent: The explicit condition makes the precondition more obvious

The logic is correct for all operations:

Binary operations (AND, OR, XOR, BYTE, SHL, SHR, SAR) properly check for 2 stack elements

Unary operation (NOT) properly checks for 1 stack element

Also applies to: 31-31, 49-49, 67-67, 84-84, 112-112, 136-136, 160-160

13-13: Excellent safety improvement - stack checks are now always active.

The replacement of std.debug.assert with explicit runtime checks ensures that stack underflow conditions are caught in both debug and release builds. All stack size checks correctly match the number of elements each operation requires:

Binary operations (AND, OR, XOR, BYTE, SHL, SHR, SAR): Check for 2 elements ✓

Unary operation (NOT): Check for 1 element ✓

This change improves code safety with minimal performance overhead.

Also applies to: 31-31, 49-49, 67-67, 84-84, 112-112, 136-136, 160-160

src/evm/opcodes/control.zig (3)

31-31: LGTM! Proper stack underflow protection.

The refactoring correctly enforces minimum stack size requirements:

op_jump: Requires 1 element (destination)

op_jumpi: Requires 2 elements (destination, condition)

op_return/op_revert: Require 2 elements (offset, size)

op_selfdestruct: Requires 1 element (beneficiary)

Making these checks unconditional significantly improves runtime safety.

Also applies to: 53-53, 101-101, 143-143, 202-202

78-78: LGTM! Proper stack overflow protection.

The check if (frame.stack.size >= Stack.CAPACITY) unreachable; correctly prevents stack overflow before pushing the program counter value. This is the inverse condition compared to the underflow checks, which is appropriate since op_pc pushes rather than pops.

31-31: Stack safety checks are correctly implemented for all control flow operations.

The refactoring properly handles both stack underflow and overflow conditions:

Underflow protection: Jump operations check for sufficient stack elements before popping

Overflow protection: op_pc correctly checks size >= Stack.CAPACITY before pushing

Correct element counts: All checks match the exact number of stack operations performed

This ensures robust stack management in both debug and release builds.

Also applies to: 53-53, 78-78, 101-101, 143-143, 202-202

src/evm/opcodes/arithmetic.zig (3)

90-90: LGTM! Comprehensive stack underflow protection for binary operations.

The refactoring correctly enforces stack size requirements for all binary arithmetic operations:

ADD, MUL, SUB, DIV, SDIV, MOD, SMOD, EXP, SIGNEXTEND all require exactly 2 operands

Making these critical safety checks unconditional is an excellent improvement for production reliability.

Also applies to: 131-131, 171-171, 219-219, 270-270, 330-330, 381-381, 594-594, 672-672

440-440: LGTM! Correct stack validation for ternary operations.

The refactoring properly enforces the 3-operand requirement for modular arithmetic:

ADDMOD: Requires a, b, and n (modulus)

MULMOD: Requires a, b, and n (modulus)

The < 3 check correctly validates sufficient stack depth for these operations.

Also applies to: 507-507

90-90: Comprehensive stack validation for all arithmetic operations.

The stack size checks are meticulously aligned with each operation's requirements:

Binary operations: Correctly check for 2 stack elements (ADD, MUL, SUB, DIV, SDIV, MOD, SMOD, EXP, SIGNEXTEND)

Ternary operations: Correctly check for 3 stack elements (ADDMOD, MULMOD)

The extensive documentation in this file demonstrates excellent code quality, and the runtime checks ensure these well-documented behaviors are safely enforced.

Also applies to: 131-131, 171-171, 219-219, 270-270, 330-330, 381-381, 440-440, 507-507, 594-594, 672-672

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

roninjin10 · 2025-06-08T00:31:41Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

vercel bot temporarily deployed to Preview – node June 8, 2025 00:31 Inactive

This was referenced Jun 8, 2025

📦 perf: remove explicit inline keywords to reduce bundle size #1797

Merged

✨ feat: implement isomorphic logging system and add comprehensive debug logging #1798

Merged

⚡ perf: Add @branchHint optimizations to EVM execution paths #1799

Merged

roninjin10 marked this pull request as ready for review June 8, 2025 00:32

vercel bot deployed to Preview – tevm-monorepo-app June 8, 2025 00:33 View deployment

roninjin10 merged commit 0a3206b into main Jun 8, 2025
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

✨ refactor: replace std.debug.assert with if (\!condition) unreachable #1796

✨ refactor: replace std.debug.assert with if (\!condition) unreachable #1796

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

✨ refactor: replace std.debug.assert with if (\!condition) unreachable #1796

✨ refactor: replace std.debug.assert with if (\!condition) unreachable #1796

Uh oh!

Conversation

Uh oh!

Description

Testing

Additional Information

Summary by CodeRabbit

Uh oh!

Uh oh!

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CodeRabbit Configuration File (`.coderabbit.yaml`)