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

GasGadget for GAS opcode #301

Merged
merged 9 commits into from
Feb 1, 2022
Merged

GasGadget for GAS opcode #301

merged 9 commits into from
Feb 1, 2022

Conversation

scroll-dev
Copy link
Collaborator

Implement the GasGadget for opcode GAS (5A). Specs at privacy-scaling-explorations/zkevm-specs#104.

Other changes:

  1. Fix constant gas cost for OpcodeId::EXP

@github-actions github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member T-opcode Type: opcode-related and focused PR/Issue labels Jan 22, 2022
@icemelon icemelon mentioned this pull request Jan 22, 2022
Closed
@ed255 ed255 self-requested a review January 24, 2022 09:58
@CPerezz CPerezz self-requested a review January 25, 2022 15:53
@CPerezz CPerezz mentioned this pull request Jan 26, 2022
Merged
CPerezz
CPerezz reviewed Jan 26, 2022
View reviewed changes
Copy link
Contributor

@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 once privacy-scaling-explorations/zkevm-specs#104 has been merged.

Also, some small nits.

Great work!!

ed255
ed255 approved these changes Jan 26, 2022
View reviewed changes
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.

LGTM! (Modulo adding a line documenting N_BYTES_GAS like Carlos asked)

This was referenced Jan 27, 2022
Closed
Merged
@roynalnaruto
Copy link
Collaborator

Not sure why the rustfmt check fails. I'm using the same toolchain, and cargo fmt --all -- --check gives me no warnings/errors.

@ed255
Copy link
Member

ed255 commented Jan 31, 2022

Not sure why the rustfmt check fails. I'm using the same toolchain, and cargo fmt --all -- --check gives me no warnings/errors.

There's a file in circuit-benchmarks/ that is generated on build time. If you run cargo fmt when this file is not available, it will not complete successfully because the file is not found and the process aborts, skipping some files. Can you try this?:

cargo check --all-features # required to generate `circuit-benchmarks/src/bench_params.rs`
cargo fmt --all

@ed255 ed255 merged commit 8d5e2af into privacy-scaling-explorations:main Feb 1, 2022
@ed255
Copy link
Member

ed255 commented Feb 1, 2022

Thanks for the contribution!

@icemelon icemelon deleted the feat/gas-gadget branch February 3, 2022 18:44
zemse pushed a commit to zemse/zkevm-circuits that referenced this pull request Mar 22, 2023
@silathdiir
Handle all Precompile errors to unimplemented (privacy-scaling-expl…
0269001 
…orations#301)

* Handle Precompile OOG error to unimplemented.

* Fix to handle all precompile failed.

* Delete `PRECOMPILE_ECRECOVER`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@roynalnaruto roynalnaruto roynalnaruto left review comments

@CPerezz CPerezz CPerezz left review comments

@ed255 ed255 ed255 approved these changes

Assignees
No one assigned
Labels
crate-bus-mapping Issues related to the bus-mapping workspace member T-opcode Type: opcode-related and focused PR/Issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@scroll-dev @roynalnaruto @ed255 @CPerezz