This repository has been archived by the owner on Jul 5, 2024. It is now read-only.
MPT: lo/hi + challengeAPI + preimage checks + performance improvements #1531
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # Cargo.lock # gadgets/src/util.rs # zkevm-circuits/Cargo.toml # zkevm-circuits/src/bytecode_circuit/dev.rs # zkevm-circuits/src/evm_circuit.rs # zkevm-circuits/src/evm_circuit/step.rs # zkevm-circuits/src/evm_circuit/util.rs # zkevm-circuits/src/keccak_circuit/keccak_packed_multi.rs # zkevm-circuits/src/lib.rs # zkevm-circuits/src/table.rs # zkevm-circuits/src/tx_circuit.rs # zkevm-circuits/src/util.rs # zkevm-circuits/src/witness/mpt.rs
…#1486) ### Description Add overall summary for cell utilization ### Issue Link N/A ### Type of change - [x] New feature (non-breaking change which adds functionality) ### Contents - sum of `available_cells` - sum of `unused_cells` - Utilization `#unused_cells`/`#available_cells` ### Example output #### Storage_1 ``` Main: +-----------------------------------+------------------------------+-------------------------+ | "storage_1" total_available_cells | "storage_1" total_used_cells | "storage_1" Utilization (%) | +-----------------------------------+------------------------------+-------------------------+ | 25480 | 6482 | 25.4 | +-----------------------------------+------------------------------+-------------------------+ Word-lo-hi +-----------------------------------+------------------------------+-----------------------------+ | "storage_1" total_available_cells | "storage_1" total_used_cells | "storage_1" Utilization (%) | +-----------------------------------+------------------------------+-----------------------------+ | 24080 | 7078 | 29.4 | +-----------------------------------+------------------------------+-----------------------------+ ``` #### Storage_2 ``` Main +-----------------------------------+------------------------------+-------------------------+ | "storage_2" total_available_cells | "storage_2" total_used_cells | "storage_2" Utilization | +-----------------------------------+------------------------------+-------------------------+ | 1456 | 467 | 32.1 | +-----------------------------------+------------------------------+-------------------------+ Word-lo-hi +-----------------------------------+------------------------------+-----------------------------+ | "storage_2" total_available_cells | "storage_2" total_used_cells | "storage_2" Utilization (%) | +-----------------------------------+------------------------------+-----------------------------+ | 1376 | 14 | 1.0 | +-----------------------------------+------------------------------+-----------------------------+ ``` #### Byte_lookup ``` Main +-------------------------------------+--------------------------------+---------------------------+ | "byte_lookup" total_available_cells | "byte_lookup" total_used_cells | "byte_lookup" Utilization | +-------------------------------------+--------------------------------+---------------------------+ | 8736 | 6786 | 77.7 | +-------------------------------------+--------------------------------+---------------------------+ Word-lo-hi +-------------------------------------+--------------------------------+-------------------------------+ | "byte_lookup" total_available_cells | "byte_lookup" total_used_cells | "byte_lookup" Utilization (%) | +-------------------------------------+--------------------------------+-------------------------------+ | 8256 | 6566 | 79.5 | +-------------------------------------+--------------------------------+-------------------------------+ ```
# Conflicts: # Cargo.lock # zkevm-circuits/src/state_circuit.rs # zkevm-circuits/src/table/keccak_table.rs # zkevm-circuits/src/table/mpt_table.rs # zkevm-circuits/src/witness/mpt.rs
### Description We remove the witness tx struct. We can wait for the lo-hi refactor ready then for this PR. ### Issue Link Part of privacy-scaling-explorations#1391 ### Type of change New feature (non-breaking change which adds functionality) ### Contents - Remove zkevm-circuits/src/witness/tx.rs entirely - Remove eth_block field from block - Acquire txID ASAP so we don't need a Optional<tx_id>
…s#1515) ### Description Fixed rws padding logic and padding error handling. ### Issue Link privacy-scaling-explorations#1507 ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) ### Contents - more documentation on padding logic - fix max_rws calculation - de-duplicated startop in end_block.tx by skip second one. - panic instead of confused error log in padding calculation function.
…xplorations#1514) ### Description Combines LookupU8 and LookupU16 under Lookup(Table). ### Issue Link privacy-scaling-explorations#1510 ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update - [x] Refactor <!-- ### Contents - [_item_] ### Rationale [_design decisions and extended information_] ### How Has This Been Tested? [_explanation_] <hr> ## How to fill a PR description Please give a concise description of your PR. The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request. MUST: Reference the issue to resolve ### Single responsability Is RECOMMENDED to create single responsibility commits, but not mandatory. Anyway, you MUST enumerate the changes in a unitary way, e.g. ``` This PR contains: - Cleanup of xxxx, yyyy - Changed xxxx to yyyy in order to bla bla - Added xxxx function to ... - Refactored .... ``` ### Design choices RECOMMENDED to: - What types of design choices did you face? - What decisions you have made? - Any valuable information that could help reviewers to think critically --> --------- Co-authored-by: Ming <[email protected]>
### Description circuit for invalid creation code error in create, markdown spec privacy-scaling-explorations/zkevm-specs#400 ### Issue Link issue privacy-scaling-explorations#1291 ### Type of change - [x] New feature (non-breaking change which adds functionality) ### Rationale Get first byte of code to store , check it is 0xef This PR contains: - buss mapping: at the return opcode in create, get the first byte . - add circuit & test - add tx deploy trace test . --------- Co-authored-by: Steven Gu <[email protected]>
privacy-scaling-explorations#1452) ### Description Change ethers-rs related packages from 0.17.0 to 2.0.7 ### Issue Link privacy-scaling-explorations#1451 ### Type of change - [x] New feature (non-breaking change which adds functionality) ### Rationale We need nothing more than simply upgrading the package.
…rivacy-scaling-explorations#1517) ### Description Remove evm_word challenge and optimize from 3->2 phase in dev phase. ### Issue Link privacy-scaling-explorations#1516 ### Type of change - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
…1500) ### Description Introduce the `debug_expression` method to the `EVMConstraintBuilder` which allows specifying an expression inside an ExecutionGadget to be evaluated and printed during assignment. This can be very useful for debugging. With this we can print the value of cells, but also of arbitrary expressions. For example, it allows printing the evaluation of all the expressions used in a lookup (previous to the RLC). ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents When calling `cb.debug_expression`, the expression is stored in the `EVMConstraintBuilder` along with a string. Later on, at assignment, whenever a step is assigned to an `ExecutionState` that had some debug expressions specified, those expressions will be evaluated and printed. #### Example usage In the `PushGadget::configure` method I include the following line: ```rust cb.debug_expression("push.code_hash", code_hash.expr()); ``` Then I run the a test that assigns this state (with `--nocapture` to see the stdout): ``` cargo test push_gadget_simple --release --all-features -- --nocapture ``` And I will get the following: ``` Debug expression "push.code_hash"=0x178027364c9ed08d00e38dbf197bbf65e45779c1d1b9dca1a229f7b7360892ce [offset=19, step=Op(PUSH1), expr=Advice { query_index: 591, column_index: 76, rotation: Rotation(0), phase: Phase(1) }] ``` --------- Co-authored-by: Chih Cheng Liang <[email protected]>
### Description - Removed #[allow(dead_code)] and `#![allow(unused_imports)]`. - Removed most of the dead code. Kept a few with the "_" initial name. - Reorganize dev modules and feature flags for "zkevm-circuits" crate. - ~~"test" flag is only used when test utils are shared within "zkevm-circuits"~~ "test" flag is gone. - "test-util" flag is used when utils are shared with testool crate. - "test-circuits" flag is reserved for test circuits that are shared with integration tests. ### Issue Link ### Type of change - [x] New feature (non-breaking change which adds functionality) ### Rationale These are the reason I keep a method: - The method is a sensible API and is tested. Although it is unused but it makes sense to keep it. The `remainder` method of the Constant Division Gadget is such an example. - Variants in Enums. We use the `match` macro for them so it is important we don't miss any variant. It makes sense to keep them even when we don't use them. - Work in progress and recent works. Cell manager and _error_depth are examples. I remove a method or a gadget for these reasons. - If it has been there for a long time but never used. - Default removing them to question us hard if the gadget really sparks joy. batched_is_zero and binary number gadgets are such cases. ### How Has This Been Tested? ``` cargo build --all-features cargo test // just for build, without running the full tests ```
…acy-scaling-explorations#1524) ### Description - rm deprecated, unreachable_code - doc some parts. ### Issue Link part of privacy-scaling-explorations#1494 ### Type of change - [ ] New feature (non-breaking change which adds functionality)
# Conflicts: # Cargo.lock # zkevm-circuits/src/state_circuit.rs # zkevm-circuits/src/table/mpt_table.rs # zkevm-circuits/src/witness/mpt.rs
github-actions
bot
added
crate-circuit-benchmarks
miha-stopar
approved these changes
Jul 17, 2023
| meta.lookup_any(lookup.description, |_meta| { | ||
| let table = self.get_dynamic_table_values(*lookup_name); | ||
| meta.lookup_any(lookup.description, |meta| { | ||
| // Fixed lookup is a direct lookup into the pre-difined fixed tables |
There was a problem hiding this comment.
Nitpick: pre-difined -> pre-defined
|
@Brechtpd which witness generation repo I should use to test it? I checked with https://github.com/privacy-scaling-explorations/mpt-witness-generator and https://github.com/CeciliaZ030/mpt-witness-generator but none of them works. Or maybe I'm doing something wrong. |
|
Oh it's the one linked above that's currently still a PR on the PSE repo: privacy-scaling-explorations/mpt-witness-generator#3. |
Thanks @Brechtpd ! |
adria0
merged commit 3c5fbfd
into
privacy-scaling-explorations:mpt-main-sync
7 checks passed
Brechtpd
added a commit
that referenced
this pull request
Aug 9, 2023
### Description Just merges the MPT branch with the master, without fixing any issues (and so breaks the code). The actual merge is done in #1531, the only purpose of this PR is to have a nicer diff for the other PR without breaking the code in the current `mpt2` branch. ### Issue Link [_link issue here_] ### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [X] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents - [_item_] ### Rationale [_design decisions and extended information_] ### How Has This Been Tested?
1 task
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is currently against to intermediate PR #1530 that just merges in the main, so the diff is cleaner.
Issue Link
[link issue here]
Type of change
Contents
Rationale
[design decisions and extended information]
How Has This Been Tested?
make test-allworks