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

Refactor MulAddWordsGadget #548

Conversation

davidnevadoc
Copy link
Contributor

The words a, b, c, d are now passed as arguments in construct instead of being fetched via query_word().
Closes #547

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jun 1, 2022
@davidnevadoc davidnevadoc marked this pull request as ready for review June 1, 2022 15:23
Copy link
Member

@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.

Overall looks good but I added two comments to discuss about some patterns that I think gadgets can follow.

zkevm-circuits/src/evm_circuit/util/math_gadget.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/evm_circuit/util/math_gadget.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@icemelon icemelon 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
Member

@CPerezz CPerezz 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 words `a, b, c, d` are now passed as arguments in `construct`
instead of being fetched via `query_word()`.
  - Move `assignment` out of `MulAddWordsGadget`
  - Consistent arguments for `construct` and `assign`
@CPerezz CPerezz force-pushed the refactor/MulAddWordsGadget branch from 444cf99 to 8e16874 Compare June 7, 2022 06:46
@CPerezz CPerezz merged commit 519ad61 into privacy-scaling-explorations:main Jun 7, 2022
@davidnevadoc davidnevadoc deleted the refactor/MulAddWordsGadget branch June 7, 2022 07:38
davidnevadoc added a commit to davidnevadoc/zkevm-circuits that referenced this pull request Jun 8, 2022
* Refactor `MulAddWordsGadget`

The words `a, b, c, d` are now passed as arguments in `construct`
instead of being fetched via `query_word()`.

* Address review comments

  - Move `assignment` out of `MulAddWordsGadget`
  - Consistent arguments for `construct` and `assign`

* Store `words` in `MulDivModGadget` instead
jonathanpwang pushed a commit to axiom-crypto/zkevm-circuits that referenced this pull request Aug 1, 2023
* Add trace dumper test that shows mpt enter invalid state

* Add test for both code hashes changed

* Add example from reading balance

* Change default of AccountData to be all zeros

* Add method for fill_state_roots_from_generator

* Add balance traces

* Add trace generators for code related tags

* Use bigger value for balance

* Add more storage trace dumpers

* Add empty storage trace dumps

* try

* fix build

* fixing

* Remove generic F

* Use update mpt circuit and re-order tags so that only nonce and balance updates can create accounts in the mpt

* Handle case of an empty account for an empty storage proof

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* re-integration of mpt circuit

* retain most generic in supercircuit

* update cargo and fix post-merge

* wip

* wip

* wip

* Write empty keccak code hash when transfering to empty account.

* wip

* wip

* wip

* wip

* wip

* merge develop

* Fix compile issues (privacy-scaling-explorations#550)

* purge accident added file

* apply new poseidon dep

* update all poseidon table dep

* lint

* lint

* Remove dbg's

* Lookup mpt table entries in mpt circuit

* cleanup

* upgrade zktrie circuit

* fmt

* upgrade circuits

* fix build

* Use max_mpt_row instead of max_evm_row

`max_evm_row` is not set in unittest, it cause some test failed

* fix merge

* clean deps

* upgrade zktrie circuit

* ignore some tests

* fmt

* disable mpt lookup temporarily

* fix

* fix

* add the scroll-trace feature

* fix

* misc fixes

* fix

* clean

---------

Co-authored-by: Mason Liang <mason@scroll.io>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Co-authored-by: Ho Vei <noelwei@gmail.com>
Co-authored-by: Ho <noel.wei@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

MulAddWordsGadget refactor
4 participants