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

Opcode SHR #519

Merged
Merged

Conversation

silathdiir
Copy link
Contributor

@silathdiir silathdiir requested a review from a team as a code owner May 18, 2022 00:01
@silathdiir silathdiir requested review from ChihChengLiang and removed request for a team May 18, 2022 00:01
@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label May 18, 2022
@silathdiir silathdiir marked this pull request as draft May 18, 2022 00:02
@silathdiir silathdiir marked this pull request as ready for review May 18, 2022 06:26
@ChihChengLiang
Copy link
Collaborator

Reviewing this now. Sorry for the delay.

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.

overall LGTM

zkevm-circuits/src/evm_circuit/execution/shr.rs Outdated Show resolved Hide resolved
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
zkevm-circuits/src/evm_circuit/util/math_gadget.rs Outdated Show resolved Hide resolved
@silathdiir silathdiir requested a review from icemelon May 27, 2022 04:06
@ChihChengLiang
Copy link
Collaborator

Sorry, will defer my review to @miha-stopar. Was too busy working on other stuff.

@ChihChengLiang ChihChengLiang requested review from miha-stopar and removed request for ChihChengLiang June 1, 2022 12:00
Copy link
Collaborator

@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! Only some minor comments.

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

@icemelon icemelon merged commit 6b7c8d4 into privacy-scaling-explorations:main Jun 6, 2022
@icemelon icemelon deleted the feat/opcode-shr branch June 6, 2022 01:43
davidnevadoc pushed a commit to davidnevadoc/zkevm-circuits that referenced this pull request Jun 8, 2022
* Add `SHR` circuit.

* Add `ShrWordsGadget` to `math_gadget.rs`.

* Add more contraints.

* Update.

* Fix lint.

* Fix.

* Fix to convert `shf_div64` to u64 when calling assign.

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

* Fix to convert `shf_mod64` to u64 when calling assign.

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

* Delete useless local variables `shf_div64` and `shf_mod64`.

* Fix to add `cb.require_equal` when contructing `a64s_lo_lt_p_lo`.

* Shorten `b64s[0]` constraint name.

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

* Shorten other `b64s[idx]` constraint names.

* Add more test cases.

* Fixed to use `rand_word()` in cases of `shr_gadget_simple` and merged
with `shr_gadget_rand_normal_shift`.

* Add more comments according to code review.

* minor tweak

Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
lispc pushed a commit that referenced this pull request Jun 1, 2023
* expose keccak circuit APIs

* expose more fields

* [chore] fix clippy
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.

None yet

4 participants