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

Consolidate MPT update lookups in state circuit#236

Merged
z2trillion merged 15 commits into
privacy-ethereum:masterfrom
scroll-tech:feat/mpt_lookups
Aug 28, 2022
Merged

Consolidate MPT update lookups in state circuit#236
z2trillion merged 15 commits into
privacy-ethereum:masterfrom
scroll-tech:feat/mpt_lookups

Conversation

@z2trillion
Copy link
Copy Markdown
Contributor

Only make one mpt update lookup per access group in the state circuit, which allows us to remove the mpt counter

@z2trillion z2trillion marked this pull request as draft July 26, 2022 02:45
@ed255
Copy link
Copy Markdown
Contributor

ed255 commented Jul 26, 2022

Could you update specs/tables.md with:

  • In rw_table rename column Aux0 to CommittedValue
  • In mpt_table remove all the field-specific tables, and leave only a single table that reflects the changes in this pr (mainly adding root and prev_root, removing counter.

Thanks!

@ed255
Copy link
Copy Markdown
Contributor

ed255 commented Jul 26, 2022

@z2trillion please ping me once this PR is ready for review (no longer draft).

@z2trillion z2trillion marked this pull request as ready for review July 27, 2022 05:03
@z2trillion
Copy link
Copy Markdown
Contributor Author

@ed255, sorry about that. I realized that I had forgotten to change the state roots in the assignment function.

@ed255
Copy link
Copy Markdown
Contributor

ed255 commented Jul 27, 2022

@ed255, sorry about that. I realized that I had forgotten to change the state roots in the assignment function.

ah no worries :)

@z2trillion
Copy link
Copy Markdown
Contributor Author

@ed255, when will you be able to take a look at this?

@CPerezz CPerezz requested review from CPerezz and miha-stopar and removed request for CPerezz August 18, 2022 07:13
Copy link
Copy Markdown
Contributor

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

In general look good but I believe a constraint is missing to limit when the root value can be changed.

Comment thread specs/tables.md Outdated
Comment thread src/zkevm_specs/state.py Outdated
Comment thread src/zkevm_specs/state.py
Copy link
Copy Markdown
Contributor

@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, but agree with Edu's comment.

Comment thread specs/tables.md Outdated
@CPerezz CPerezz mentioned this pull request Aug 19, 2022
z2trillion and others added 3 commits August 25, 2022 01:02
Co-authored-by: Eduard S. <eduardsanou@posteo.net>
Co-authored-by: Eduard S. <eduardsanou@posteo.net>
Copy link
Copy Markdown
Contributor

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM!

@z2trillion z2trillion merged commit 5654c4a into privacy-ethereum:master Aug 28, 2022
@z2trillion z2trillion deleted the feat/mpt_lookups branch August 28, 2022 01:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants