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

constrain rw_counter_end_of_reversion for failed step #1030

Conversation

DreamWuGit
Copy link
Collaborator

@github-actions github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels Jan 5, 2023
@DreamWuGit DreamWuGit marked this pull request as ready for review January 12, 2023 02:13
Copy link
Contributor

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

LGTM! I have left some style suggestions.

The logics in these exceptions seems to be duplicated, somehow we should be able to refactor them into a gadget (perhaps to refactor RestoreContextGadget to also support that), but I think it's fine to leave it to the future.

Also as you mentioned, the same constraint should be enforced in REVERT when it's root, are you planning to do that in further PR?

@DreamWuGit
Copy link
Collaborator Author

DreamWuGit commented Jan 13, 2023

The logics in these exceptions seems to be duplicated, somehow we should be able to refactor them into a gadget (perhaps to refactor RestoreContextGadget to also support that), but I think it's fine to leave it to the future.

Also as you mentioned, the same constraint should be enforced in REVERT when it's root, are you planning to do that in further PR?

good point! yes, as more error gadgets added, I feel more common operations among them:

  • look up false is_success
  • go to end_tx when root
  • restore context when internal
  • this rw_counter_end_of_reversion constraint.
    i am thinking refactor new gadget like CommonErrorGadget to cover them, add an issue (refactor to common error gadget #1059 )to trace it. also will have a look about revert in future PR.

Copy link
Contributor

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

LGTM

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. The proposed error gadget refactor sounds like a great idea.

@ChihChengLiang ChihChengLiang merged commit f7941c1 into privacy-scaling-explorations:main Jan 18, 2023
SuccinctPaul pushed a commit to SuccinctPaul/zkevm-circuits that referenced this pull request Feb 15, 2024
* setup multiple test bytes

* add debug bytes

* retrace debug: add ad sk idx

* retrace debug: add access list idx wit

* add additional witness columns

* add additional witness columns

* add constraints

* add constraints

* add constraints

* add constraints

* generate correct stack states

* correct stack witness generation

* add stack accumulator

* fmt

* fmt

* remove debug flag

* correct tx circuit lookup
:

* decoding table

* correct degree

* add decoding table

* add decoding table assignment

* reduce degree

* unnecessary import

* fmt

* fix constraints

* fix constraints

* add stack ptr increase

* fix constraints

* fmt

* add stack op witness

* add stack op witness

* add stack constraints

* fmt

* add stack constraints

* add stack end condition

* fmt

* fmt

* rename beginlist and endlist

* adjust test case

* change address naming to depth

* add instrument booleans

* add consistency constraints for access list

* add consistency constraints for access list

* adjust test

* remove debug flag

* fmt

* refactor accumulating coeff

* add stack op enum

* refactor decoding table witness

* refactor access list indicator columns

* remove auto code

* refactor syntax

* remove comment

* add comment

* add missing constraints

* refactor syntax

* debug commit

* remove debug line

* refactor constructor

* refactor push constructor

* refactor pop constructor

* refactor update

* fmt

* remove import

* debug commit

* restore test. fix lookup conditional

* restore column in tx table

* restore access list idx columns:

* add access list dynamic assignment

* restore dynamic access list assignment

* resolve merge conflict

* add section denoter for access list

* add access list rlc helper

* adjust column and new section init

* add boolean instruments, degree fix

* add constraints

* add constraints

* add rlp lookup

* add section_rlc starting for access list

* add section_rlc accumulation

* recover section constraint

* look up access list address

* lookup access list storage key len

* fmt

* fmt clippy

* remove debug tags

* correct lookup of address and storage keys

* remove debug flag

* add extra gas fields for 1559

* comment

* fmt

* fmt

* add option guard

* fmt

* unwrap guard

* correct witness rows

* fmt

* add comment

* add instrument to fix degree

* fix rlp lookup

* remove debug tag

* remove debug tag

* style fmt

* adjust section transition

* cargo

* clip cargo lock

* correct merge

* debug commit

* uncomment test

* correct fn name

* remove output

* refactor assignment

* add lookup

* fmt

* fmt

* restore test

* remove duplicate

* add comments

* add comments'

* fmt

* remove engagement of stack acc

* remove stack_acc and add new witness

* add lookup from rlp_table

* comment correction

* add depth constraint decoding table

* add cross-depth lookup in decoding table

* fmt

* clippy

* rm comments

* stack op order correction

* add more strict constraints

* remove debug flags

* remove debug output

* fmt

* adjust stack table columns

* add stack op id check

* adjust access list idx position

* revert "adjust access list idx position"

This reverts commit aecd544.

* remove debug flags

* recover init

* recover push and pop lookups

* add op constraints

* fmt

* degree reduction boolean constraints

* add constraints

* fmt

* clippy

---------

Co-authored-by: xkx <[email protected]>
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