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

SuperCircuit#622

Merged
ed255 merged 32 commits into
mainfrom
feature/super-circuit
Jul 28, 2022
Merged

SuperCircuit#622
ed255 merged 32 commits into
mainfrom
feature/super-circuit

Conversation

@ed255
Copy link
Copy Markdown
Contributor

@ed255 ed255 commented Jul 15, 2022

The SuperCircuit is a new circuit that will contain all the circuits of the zkEVM in order to achieve two things:

  1. Check the correct integration between circuits via the shared lookup tables
  2. Allow having a single circuit setup for which a proof can be generated that would be verified under a single aggregation circuit for the first milestone

The name "SuperCircuit" is open for discussion, other name proposals are welcome!

Adding the State Circuit into the SuperCircuit will be done after the work being done by scroll-tech to integrate the EVM circuit with the State circuit is merged (See main...scroll-tech:zkevm-circuits:scroll-dev-0607)

NOTE: The super circuit test is disabled from the CI because it requires more memory than is available in the github workers.

Summary of changes

  • general:

    • Addapt the circuits so that the shared tables can be assigned outside of
      the circuit's assign functions.
  • evm_circuit.rs

    • load table functions moved to table.rs
  • bytecode_circuit/bytecode_unroller.rs

    • Rename hash_length to code_length in Config
    • Rename r to randomness
    • Add is_enabled expression to the keccak lookup
    • Move load_keccaks to table.rs
    • Fix endianness in keccak hash output
    • Add function to obtain all the keccak inputs required by the SignVerifyChip
  • table.rs

    • Move table definitions from evm_circuit/table.rs and rw_table.rs to here
    • Move all shared table load functions to here
    • Add BytecodeTable to help reusing the table config with other
    • Rename hash to code_hash in BytecodeTable
    • Use RLC(reversed(input)) in keccak table assignment for input_rlc.
  • tx_circuit.rs

    • Add function to obtain all the keccak inputs required by the TxCircuit
    • Fix Gas encoding: remove RLC
    • Add missing field TxFieldTag::CallDataGasCost
  • tx_circuit/sign_verify.rs

    • Move load_keccak to table.rs
    • Add function to obtain all the keccak inputs required by the SignVerifyChip
    • Reverse the pk_hash used in the lookup so that the RLC is calculated as if
      it was a hash in an Ethereum Word.

The SuperCircuit contains the following circuits:

  • EVM Circuit
  • State Circuit
  • Tx Circuit
  • Bytecode Circuit
  • Copy Circuit
  • Keccak Circuit
  • MPT Circuit
  • PublicInputs Circuit

And the following shared tables, with the circuits that use them:

  • Copy Table
    • Copy Circuit
    • EVM Circuit
  • Rw Table
    • State Circuit
    • EVM Circuit
    • Copy Circuit
  • Tx Table
    • Tx Circuit
    • EVM Circuit
    • Copy Circuit
    • PublicInputs Circuit
  • Bytecode Table
    • Bytecode Circuit
    • EVM Circuit
    • Copy Circuit
  • Block Table
    • EVM Circuit
    • PublicInputs Circuit
  • MPT Table
    • MPT Circuit
    • State Circuit
  • Keccak Table
    • Keccak Circuit
    • EVM Circuit
    • Bytecode Circuit
    • Tx Circuit
    • MPT Circuit

Diagram of the Super Circuit (in green are the included components for this PR):
image

Resolve #600

@ed255 ed255 marked this pull request as draft July 15, 2022 08:56
@github-actions github-actions Bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jul 15, 2022
@ed255 ed255 force-pushed the feature/super-circuit branch from b0601b3 to e738148 Compare July 15, 2022 09:42
@github-actions github-actions Bot added the crate-gadgets Issues related to the gadgets workspace member label Jul 15, 2022
@github-actions github-actions Bot added the CI Issues related to the Continuous Integration mechanisms of the repository. label Jul 18, 2022
@ed255 ed255 force-pushed the feature/super-circuit branch from 42d4029 to d20088a Compare July 18, 2022 15:53
@github-actions github-actions Bot removed the CI Issues related to the Continuous Integration mechanisms of the repository. label Jul 18, 2022
@ed255 ed255 marked this pull request as ready for review July 18, 2022 15:54
@ed255 ed255 requested a review from miha-stopar as a code owner July 18, 2022 15:54
@ed255 ed255 force-pushed the feature/super-circuit branch from 50a3e9b to cfcbd12 Compare July 20, 2022 14:42
@ed255 ed255 marked this pull request as draft July 20, 2022 23:06
The SuperCircuit is a new circuit that will contain all the circuits of the
zkEVM in order to achieve two things:
- Check the correct integration between circuits via the shared lookup tables
- Allow having a single circuit setup for which a proof can be generated that
would be verified under a single aggregation circuit for the first milestone

Summary of changes:
- general:
  - Addapt the circuits so that the shared tables can be assigned outside of
    the circuit's assign functions.

- evm_circuit.rs
  - load table functions moved to `table.rs`

- bytecode_circuit/bytecode_unroller.rs
  - Rename `hash_length` to `code_length` in `Config`
  - Rename `r` to `randomness`
  - Add `is_enabled` expression to the keccak lookup
  - Move `load_keccaks` to `table.rs`
  - Fix endianness in keccak hash output
  - Add function to obtain all the keccak inputs required by the `SignVerifyChip`

- table.rs
  - Move table definitions from `evm_circuit/table.rs` and `rw_table.rs` to here
  - Move all shared table load functions to here
  - Add BytecodeTable to help reusing the table config with other
  - Rename `hash` to `code_hash` in `BytecodeTable`
  - Use `RLC(reversed(input))` in keccak table assignment for `input_rlc`.

- tx_circuit.rs
  - Add function to obtain all the keccak inputs required by the `TxCircuit`
  - Fix `Gas` encoding: remove RLC
  - Add missing field `TxFieldTag::CallDataGasCost`

- tx_circuit/sign_verify.rs
  - Move `load_keccak` to `table.rs`
  - Add function to obtain all the keccak inputs required by the `SignVerifyChip`
  - Reverse the `pk_hash` used in the lookup so that the RLC is calculated as if
    it was a hash in an Ethereum Word.
@ed255 ed255 force-pushed the feature/super-circuit branch from 87f9359 to a23dfa1 Compare July 21, 2022 10:17
@ed255 ed255 marked this pull request as ready for review July 21, 2022 10:19
@ed255 ed255 changed the title [WIP] SuperCircuit SuperCircuit Jul 21, 2022
@ed255 ed255 requested review from han0110 and lispc and removed request for miha-stopar July 21, 2022 13:10
};

let prover = MockProver::<F>::run(k, &circuit, vec![]).unwrap();
let instance = vec![vec![randomness; (1 << k) - 7 + 1]];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 is the degree? (Maybe either add some comment or use cs.degree() will be better)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a variable and a constant that show what those values are in 7955480

@lispc
Copy link
Copy Markdown
Collaborator

lispc commented Jul 27, 2022

the general idea looks good to me.

Some questions:

(1) in scroll-dev-0714, i have already make rw table of evm circuit and state circuit identical. (implicitly, not a explicit "RwTable" struct). Will it be good to add it also into this branch? But i may submit no earlier than next Tuesday.
(2) this pr may bring a lot of conflict, eg: state circuit mpt pr. Any idea on the merging order?
(3) power_of_randomness_from_instance: (a) i am not sure whether it should be instance cols now, anyway finally it should be cs.challege(). (Another thought is that i find it be useful to allow a fixed Fr to be fake "randomness" for debugging, better for reproducibility rather than real Fr::rand(). In my own branch, I use power of 0x1000 as randomness [Expr; 31] in daily development. )

Copy link
Copy Markdown
Contributor

@han0110 han0110 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great! With structured tables It makes things much easier to follow! Only left some small questions and suggestions.

Comment thread zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs
Comment thread zkevm-circuits/src/bytecode_circuit/bytecode_unroller.rs Outdated
Comment thread zkevm-circuits/src/evm_circuit.rs Outdated
Comment thread zkevm-circuits/src/super_circuit.rs
Comment thread zkevm-circuits/src/table.rs
Comment thread zkevm-circuits/src/table.rs Outdated
Comment thread zkevm-circuits/src/table.rs Outdated
@ed255
Copy link
Copy Markdown
Contributor Author

ed255 commented Jul 27, 2022

Some questions:

(1) in scroll-dev-0714, i have already make rw table of evm circuit and state circuit identical. (implicitly, not a explicit "RwTable" struct). Will it be good to add it also into this branch? But i may submit no earlier than next Tuesday.

I intentionally left the State circuit out of this PR because I knew you were working on the EVM - State Circuit integration in parallel. I would prefer to leave this PR as is, and in the future, once your work is also merged we can extend the Super Circuit.

(2) this pr may bring a lot of conflict, eg: state circuit mpt pr. Any idea on the merging order?

I think the conflict that may be introduced by that PR will be manageable (The current state of the SuperCircuit doesn't include the MPT Table nor the State Circuit). About merging order, I would like to give this PR priority because it touches several circuits so the earlier it is merged, the soon we can build on top of those changes.

I do have some doubts about how to approach the lookups between circuits: state circuit mpt uses RLC for lookups (so that the lookup is a single expression), but in the SuperCircuit the lookups are kept without RLC. See this comment #621 (review)

(3) power_of_randomness_from_instance: (a) i am not sure whether it should be instance cols now, anyway finally it should be cs.challege(). (Another thought is that i find it be useful to allow a fixed Fr to be fake "randomness" for debugging, better for reproducibility rather than real Fr::rand(). In my own branch, I use power of 0x1000 as randomness [Expr; 31] in daily development. )

Ah those are interesting points. So the idea of passing the randomness via instance was originally used in the EVM circuit, as a temporary solution to allow testing while we don't have the challenge API available. I completely agree that using a different random value on each test run is not desirable: it makes debugging harder and the tests are not 100% reproducible. I would prefer handling that outside of this PR. I'll open an issue for that: #641

@lispc
Copy link
Copy Markdown
Collaborator

lispc commented Jul 27, 2022

ok. for the rlc, i can discuss it later ( we currently use AggregationCircuit to verify multi small circuits, and check some advice column commitments are equal between small circuits, so using rlc can reduce num of cols, reducing the cost of AggregationCircuit. I may adjust the "RLC"-ness of shared table between circuits configurable, or at least, merge friendly later)

@ed255 ed255 force-pushed the feature/super-circuit branch from 875745b to 68350c4 Compare July 28, 2022 15:16
@ed255 ed255 merged commit 9e4701e into main Jul 28, 2022
lispc added a commit that referenced this pull request Aug 28, 2023
…S auditors (#572)

* fix finding 3 (#575)

* Fix zellic finding 4 (#576)

* fix finding 3 (#575)

* fix finding 4

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* add range check on diffs (#586)

* Fix finding 10 (#578)

* fix finding 3 (#575)

* fix finding 10

* Fix finding 13 (#579)

* fix finding 3 (#575)

* fix finding 13

* Fix zellic finding 14 (#580)

* fix finding 3 (#575)

* fix finding 14

* Fix zellic finding 5 (#584)

* fix finding 3 (#575)

* fix finding 5

* refine comments

* fmt

* Fix finding 17 (#602)

* add q_last

* fix

* add more diff range check

* fix finding 7 (#625)

* tx_id = 1 when sm starts

* Fix finding 11 : use length for rlc in rlp table (#719)

* fix: use tag_bytes_rlc and tag_length to copy tag's bytes around

* fix lookup input for Len & RLC & GasCost fields in tx circuit

* refactor

* fix

* refactor

* fix col phase issue

* refactor bytes_rlc type

* Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612)

* lookup chain_id to RLP table

* fix finding 22 (#614)

* fix finding 21 (#613)

* fix finding 23 (#618)

* fix finding 26 (#622)

* fix finding 28 (#624)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix finding 29 (#623)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* enforce is_final is true at the last row and fix RLC related vul (#735)

* Fix finding 30  (#733)

* enforce all txs in a block are included in the tx table

* clippy

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* Fix Zellic / Kalos finding25 (#619)

* fix finding 25

* add comment

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix conflicts

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* use q_first instead

* fmt

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
lispc added a commit that referenced this pull request Aug 28, 2023
* add row counting interface for keccak

* add class level capacity calculator for keccak

* remove f capacity from core

* remove capacity calculator in aggregator util

* remove unnecessary imports

* replace max keccak round in core

* replace reference for max keccak

* remove unnecessary keccak imports and constants

* remove max keccak constant

* remove constants in hash cell parsing

* remove constant column sanity check

* add state column usage log

* adjust input bytes column

* add long column padding

* correct fmt

* fix fmt

* minor fixes

* fix

* Fix: allow skipping of L1Msg tx part 2 (calculate num_all_txs in tx circuit) (#778)

* calculate num_l1_msgs and num_l2_txs in tx circuit

* fix

* fmt and clippy

* fix: non-last tx requires next is calldata

* add NumAllTxs in block table and copy it from pi to block table

* add lookup for NumAllTxs in tx circuit

* clippy

* add block num diff check to avoid two real block have same num

* clippy

* address comments

* Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALOS auditors (#572)

* fix finding 3 (#575)

* Fix zellic finding 4 (#576)

* fix finding 3 (#575)

* fix finding 4

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* add range check on diffs (#586)

* Fix finding 10 (#578)

* fix finding 3 (#575)

* fix finding 10

* Fix finding 13 (#579)

* fix finding 3 (#575)

* fix finding 13

* Fix zellic finding 14 (#580)

* fix finding 3 (#575)

* fix finding 14

* Fix zellic finding 5 (#584)

* fix finding 3 (#575)

* fix finding 5

* refine comments

* fmt

* Fix finding 17 (#602)

* add q_last

* fix

* add more diff range check

* fix finding 7 (#625)

* tx_id = 1 when sm starts

* Fix finding 11 : use length for rlc in rlp table (#719)

* fix: use tag_bytes_rlc and tag_length to copy tag's bytes around

* fix lookup input for Len & RLC & GasCost fields in tx circuit

* refactor

* fix

* refactor

* fix col phase issue

* refactor bytes_rlc type

* Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612)

* lookup chain_id to RLP table

* fix finding 22 (#614)

* fix finding 21 (#613)

* fix finding 23 (#618)

* fix finding 26 (#622)

* fix finding 28 (#624)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix finding 29 (#623)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* enforce is_final is true at the last row and fix RLC related vul (#735)

* Fix finding 30  (#733)

* enforce all txs in a block are included in the tx table

* clippy

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* Fix Zellic / Kalos finding25 (#619)

* fix finding 25

* add comment

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix conflicts

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* use q_first instead

* fmt

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* add pi comments

* rename preimage col idx

* add keccak rows check

* rename input bytes col finder fn

* modify keccak row env constaint

* modify keccak row env constaint

* add named constant setup vars

* modify keccak row check

* clippy advised

* add comments on chunk hash

* fmt

* avoid constant lookup table

* avoid repetitive computation of input_bytes_col_idx

---------

Co-authored-by: Zhuo Zhang <mycinbrin@gmail.com>
Co-authored-by: xkx <xiakunxian130@gmail.com>
Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

crate-bus-mapping Issues related to the bus-mapping workspace member crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-gadgets Issues related to the gadgets workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define a circuit that contains all circuits

3 participants