Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

state lexicographic ordering can overlap for tag #520

Closed
DreamWuGit opened this issue May 18, 2022 · 3 comments
Closed

state lexicographic ordering can overlap for tag #520

DreamWuGit opened this issue May 18, 2022 · 3 comments

Comments

@DreamWuGit
Copy link
Collaborator

code line in https://github.com/appliedzkp/zkevm-circuits/blob/main/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs#L322
be_bytes[0] = row.field_tag().unwrap_or_default() as u8 + ((row.tag() as u8) << 4);.

For RwTableTag::CallContext , its corresponding field tag CallContextFieldTag consists of more than 20 items. when current row is CallContext tag, next row is tag behind RwTableTag::CallContext , like TxLog, it is maybe not distinguishable to use be_bytes[0].
i.e.

  • current row is CallContext with CallContextFieldTag::CodeSource ,
    be_bytes[0] = 20 + 10 <<4 = 180.
  • next row is TxLog with TxLogFieldTag::Address ,
    be_bytes[0] = 1 + 11 <<4 = 177.

after RW sort , it breaks next row value should be bigger than current row.

@ed255
Copy link
Member

ed255 commented May 24, 2022

I noticed there's already a comment here about this:

// + 4 bits for field_tag // TODO: this actually needs 5 bits. Either reduce id

So I would think that @z2trillion (he implemented the current state circuit) is already aware of this and maybe is planning an update?

As long as 5 bits (or more) are reserved for field_tag, this issue should be resolved right?

@z2trillion
Copy link
Collaborator

#523 will fixes the TODO. 4 bits for the field tag was previously enough because none of the tests used larger field tags, but #476 does, so we had to fix this issue now.

icemelon added a commit that referenced this issue May 30, 2022
* fix tag byte overlap issue

* fix clippy

* Update constraints and bit packing for 5 bit field tags

* Update comments

* minor update for comment

minor update for comment

Co-authored-by: Haichen Shen <shenhaichen@gmail.com>

Co-authored-by: z2trillion <mason@scroll.tech>
Co-authored-by: z2trillion <z2trillion@users.noreply.github.com>
Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
@DreamWuGit
Copy link
Collaborator Author

closing as #523 merged!

jonathanpwang pushed a commit to axiom-crypto/zkevm-circuits that referenced this issue Aug 1, 2023
* feat: preliminary work

* wip

* finish assignment to ecrecover

* fix: input length may be different than call data length (for precompiles)

* fix: input bytes to ecrecover

* minor edits

* modexp table

* wip

wip

wip

wip

* add auxdata of modexp

* assign in modexp gadget

* wip: fix most constraint failure

* wip

* wip: fix constraints

* pass primary test

* optimize phase2 cell usage and add more tests

* add invalid detection and test

* wip: induce u256 modexp

* modexp event

* wip: induce modexp table to evm

* fix super circuit

* induce modexp table done

* regression pass

* modexp circuit done

* wip: test modexp table lookup

* wip: testing modexp table lookup

* pass modexptable lookup

* pass u256 modexp circuit

* refine dependencies

* fix for merging develop

* add modexp into super circuit

* clippy

* fmt

* disable ecrecover temporarily.
(lint not pass in CI but tests should be able to move forword)

* post merging fixing

* fmt and clippy

* for invalid input

* upgrade modexp circuit dep

* fixes according to review

* fmt

* update dep, prune ff 0.13

* optimize by powofrand table

* constraint for nil output

* reverse geth step error indication for precompile

* trivial fix for invalid case

* fmt

* error should not be throw for precompile failure

* fix for merge

* post-merge fixes for compile errors

* reverse throwing precompile error in get_step_err

* fmt

* clippy lint

* update `dev_load` of modexp table and some file structures

* resume size limit for modexp

* refactor and support garbage bytes in input

* prune and lint

* + resume the precompile error detection. + trivial logs

* update modexp chip rows usage constant

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants