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

Missing check curr.rw_counter + reversible_write_counter == rw_counter_end_of_reversion when the step is failing #346

Open
han0110 opened this issue Dec 20, 2022 · 4 comments
Assignees

Comments

@han0110
Copy link
Collaborator

han0110 commented Dec 20, 2022

To ensure rw_counter_end_of_reversion is correct in the timeline, in the end of failing step we need to check if there is a gap of rw_counter to the next step, where the gap size is exactly the reversible_write_counter. And in the end of the gap, the rw_counter should be rw_counter_end_of_reversion.

Need to add this check into Instruction.step_state_transition_to_restored_context:

if not is_success:
    assert rw_counter_end_of_reversion == curr.rw_counter + reversible_write_counter
@CPerezz CPerezz added the bug label Dec 20, 2022
@aguzmant103 aguzmant103 moved this to 🆕 Product Backlog Items in zkEVM Community Edition Dec 22, 2022
@DreamWuGit
Copy link
Collaborator

I will take a look at this one !

@DreamWuGit
Copy link
Collaborator

for circuit side work privacy-scaling-explorations/zkevm-circuits#1030, it should be
assert rw_counter_end_of_reversion == curr.rw_counter + rw_counter_offset + reversible_write_counter - 1
as buss mapping set rw_counter_end_of_reversion = current_rw_counter -1 for call in https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/main/bus-mapping/src/circuit_input_builder/input_state_ref.rs#L798
I am thinking also need to add check for root call failure, what do you think? @han0110

@han0110
Copy link
Collaborator Author

han0110 commented Jan 6, 2023

I am thinking also need to add check for root call failure

That's right! We need to check this when it's reverting step no matter it's root or not. Thanks for catching this!

@CPerezz CPerezz removed the bug label Feb 14, 2023
@ed255
Copy link
Member

ed255 commented Jun 2, 2023

This issue has been resolved in the circuits via privacy-scaling-explorations/zkevm-circuits#1030 and privacy-scaling-explorations/zkevm-circuits#1177 but it's still missing in the specs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: 🆕 Product Backlog Items
Development

No branches or pull requests

4 participants