Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Fix TStore for reverts and in static calls #1811

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Fix TStore for reverts and in static calls #1811

merged 4 commits into from
Apr 30, 2024

Conversation

z2trillion
Copy link
Collaborator

@z2trillion z2trillion commented Apr 30, 2024

Description

The bus mapping TSTORE did not handle the cases where the opcode's effect is revert and the error that happens if TSTORE is called in a static call. This adds tests for those two cases and then fixes the bus mapping.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Refactor (no updates to logic)

How Has This Been Tested?

Added tests that fail before the bus mapping fixes are applied.

This PR contains:
- [Add failing revert test](https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1811/commits/be4b13dd1251a370456c6ed86f04226af48aea08)
- [Add failing write protection test for TStore](https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1811/commits/18845eb1a5fd624bddb0ac626c22a5d32389ea8f)
- [Handle reverts and write protection failure for TSTORE in bus mapping](https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1811/commits/01050195879550fa68d9abd3d669ffc7b658d3f5)

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Apr 30, 2024
@z2trillion z2trillion marked this pull request as draft April 30, 2024 02:02
@github-actions github-actions bot added the crate-bus-mapping Issues related to the bus-mapping workspace member label Apr 30, 2024
@z2trillion z2trillion marked this pull request as ready for review April 30, 2024 02:15
Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after the test is fixed. The test is failing because the TSTORE in zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs hasn't patched.

scroll-tech@53a5bd7#diff-cbb75f42395a4f4a252305db5c34489f688990548821f0e18bf7d99ae6590e46

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Apr 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 30, 2024
@z2trillion z2trillion added this pull request to the merge queue Apr 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 30, 2024
@z2trillion z2trillion added this pull request to the merge queue Apr 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 30, 2024
@z2trillion z2trillion added this pull request to the merge queue Apr 30, 2024
Merged via the queue into main with commit 9a13854 Apr 30, 2024
15 checks passed
@z2trillion z2trillion deleted the fix/tstore branch April 30, 2024 18:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants