-
Notifications
You must be signed in to change notification settings - Fork 857
Conversation
… MLOAD and SDIV Txs
Seems that introduction of build.rs causes build step to fail.
I assume the problem is associated with the git submodule pointing to OpenZeppelin ERC20.sol but i have yet not managed to figure out and fix it. Could we maybe introduce a worflow step that adds/init the submodule instead of doing that in run.sh? (if i locally directly execute run.sh without a prior cargo build step, tests proceed) |
I unassigned @leolara since Leo recently focused on the DSL effort. |
@ChihChengLiang @ed255 |
… declare_tests macro
…vacy-scaling-explorations/zkevm-circuits into integration-tests-benchmarks
Co-authored-by: Chih Cheng Liang <[email protected]>
Hi @ed255, do you still have time to review this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Everything looks really good and really clean! Also I found it really well commented an organized! I left a few nitpicks but they are minor, just some comments that popped up in my min while reviewing; feel free to read them and apply changes or not :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after Edu's and my comments are addressed.
Co-authored-by: Chih Cheng Liang <[email protected]>
Co-authored-by: Chih Cheng Liang <[email protected]>
Co-authored-by: Chih Cheng Liang <[email protected]>
@ChihChengLiang proposed removals of redundant string on BuildError variants plus enum adaptations and replacement of ok_or_else() with ok_or() according to clippy output addressed with commits: d80a1b4 & 3a0a27f |
…um_enum and num_enum_derive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
### Description We removed the warning message shown during the project build. ``` warning: the following packages contain code that will be rejected by a future version of Rust: traitobject v0.1.0 note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 2` ``` The message is caused by an unused dependency introduced during #1615 ### Issue Link None ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) ### Contents - Removed some unused dependencies - Removed some unused imports ### How Has This Been Tested? Local build on all targets
Description
Move SC compilation step to a build script. Rewrite deployment function based on SC rust bindings generated with
ethers_contract_abigen::Abigen and submit Txs for proof generation benchmarking.
Issue Link
#1557
Type of change
Contents
- Remove compilation step and rewrite fn deploy. New one uses the bindings generated at build step to deploy SCs and also instantiates SC instances to interact with.
- Create a dump TxTrace function [needed for testing future implementations of benchmark contracts - needs attribute allow(dead_code)]
- Submit Txs optimized for maximum calls of MLOAD and SDIV up to 300k gas
How Has This Been Tested?
https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/6234076847/job/16920593180>> Workflow build step is currently failing