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

Refactor: use StepRws in evm circuits to replace rw indices #1786

Merged

Conversation

maschad
Copy link
Contributor

@maschad maschad commented Mar 1, 2024

Description

Continuing the work originally started in #1616 this PR seeks to introduce StepRws across the evm circuits to reduce the need for tracking rw indices through code.

Issue Link

Closes #1586

Type of change

Refactor (no updates to logic)

Contents

The change has been made to the following files

  • error_oog_call.rs
  • error_oog_memory_copy.rs
  • error_oog_sload_sstore.rs
  • error_return_data_oo_bound.rs
  • error_write_protection.rs
  • extcodecopy.rs
  • extcodehash.rs
  • extcodesize.rs
  • logs.rs
  • return_revert.rs
  • returndatacopy.rs
  • sha3.rs
  • sload.rs
  • sstore.rs

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Mar 1, 2024
@maschad maschad force-pushed the feat/use-steprws branch from ff650a3 to 86adab3 Compare March 1, 2024 03:26
@maschad maschad changed the title feat: use StepRws in evm circuits to replace rw indices Refactor: use StepRws in evm circuits to replace rw indices Mar 1, 2024
@maschad maschad marked this pull request as draft March 4, 2024 04:25
@maschad maschad marked this pull request as ready for review March 19, 2024 21:15
@maschad
Copy link
Contributor Author

maschad commented Mar 19, 2024

@ChihChengLiang this should be ready for review now

@ed255 ed255 requested a review from miha-stopar March 21, 2024 10:38
Copy link
Collaborator

@miha-stopar miha-stopar left a comment

Choose a reason for hiding this comment

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

LGTM! Just two nitpicks.

@maschad
Copy link
Contributor Author

maschad commented Mar 25, 2024

Thanks for the review @miha-stopar ! I've made the changes.

@ChihChengLiang ChihChengLiang self-requested a review March 26, 2024 05:26
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.

@maschad Great work replacing the manual counting of read-write indices with the automatic one. The assignment looks cleaner now.

I added some comments on fixing the APIs and some style fixes. We can merge the PR after the feedback are addressed.

@maschad maschad requested a review from ChihChengLiang March 27, 2024 01:25
@maschad
Copy link
Contributor Author

maschad commented Mar 27, 2024

Thanks for your review @ChihChengLiang I've made the requested adjustments.

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. Thanks for the fix

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Mar 28, 2024
Merged via the queue into privacy-scaling-explorations:main with commit 55754a9 Mar 28, 2024
15 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use more StepRws in EVM circuits
3 participants