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

[proof chunk] refactor evm_circuit/state_circuit to support proof chunk #1641

Merged

Conversation

hero78119
Copy link
Member

@hero78119 hero78119 commented Oct 3, 2023

Description

[PR description]

Issue Link

#1601

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

Highlights

  • separate rw_table in evm_circuit/state_circuit and each have its own permutationchip. fingerprints checking will be deferred to root circuit.
  • disable rwtable first row check (by disable selector offset=0 in state_circuit, other offset toggle on as usual), since it will be copy constraints via public input.
  • keep Rw::Start and introduce Rw::Padding to support post-padding in address-sorted rwtable in last_chunk.
  • instance (new public input) column are design into two, one for prev_chunk context, another for current_chunk context to be used for next chunk. So in root circuit commitment of instance column can be used in snark-verifier without need to unwrap it.

Update on 18 Oct 2023: beginchunk/endchunk virtual step, inner_rw_counter, chunkctx

  • add Begin/EndChunk virtual step. BeginChunk is not mandatory in first chunk first step. while EndChunk is not mandatory in last chunk last step.
  • add inner_rw_counter which is local rw_counter within each chunk. This is used to count valid rw_row and assure Rw::Padding are append consecutively in end_block.rs logic => EndChunk should apply similar check later on
  • introduce chunkctx->{chunk_index, total_chunks} to tell first chunk (chunk_index==0) and last chunk (chunk_index + 1 == total_chunks) during witness generation/constraints assignment
  • add chunkctx_table to able to lookup chunk context (chunk_index, next_chunk_index, total_chunk, initial_rwc, end_rwc..etc) in exec step to allow various conditional check based on current chunk context

How Has This Been Tested?

[explanation]

More context on Rw::Padding (Not cover in current PR, will be handled in later multiple chunk PR to make scope smaller)

In new logic, Rw::Start will be insert in first chunk offset 0, while other holes are filled by Rw::Padding in last chunk(s). The address-sorted rwtable layout will be

address-sorted rwtable
## first chunk
[
      Rw::start,  // offset 0, Rw::Start is only in first chunk, and only in offset 0, constrainted by public input
      ....(normal Rw), 
       ...(Rw::Padding),  // padding if there are only one chunk
]

## end chunk (if there are > 1 chunk)
[
      ....(normal Rw),  // offset 0 are carry over from previous chunk, other are address sorted
       ...(Rw::Padding) // padding in end chunk
]

For chronologically rwtable, since there is no in-row constraints to check each Rw operation, so theoretically Rw::Padding rows can be filled in any offset. However, we also need to assure there is no malicious insertion other than Rw::Padding. To do that, we will rewrite this logic in later PR https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/main/zkevm-circuits/src/evm_circuit/execution/end_block.rs#L86-L90 to lookup consecutive Rw::Padding at chronologically sorted table, at the END of EACH chunk.

A tricks: first Rw::Padding rw_counter need to be set as last (globally) valid row rw_counter + 1. This is to make sure in both chronologically rw_table or address-sorted rw_table it's always append in the end of rw_table.

chronologically rwtable, ascending sorted by `rw_counter`.
## first chunk
[
      Rw::start,  // offset 0, Rw::Start is only in first chunk, constrainted by public input
      ...(normal Rw),
      ...(Rw::Padding),   // first Rw::Padding rw_counter need to be set as last (globally) valid row rw_counter + 1, last means from last !!chunk!!. It assures Rw::Padding always append at the end of each chunk
]

## end chunk (if there are > 1 chunk)
[
      ....(normal Rw),  // offset 0 are carry over from previous chunk, other are rw_counter sorted
       ...(Rw::Padding) // padding, also rw_counter sorted
]

@hero78119 hero78119 marked this pull request as draft October 3, 2023 15:30
@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Oct 3, 2023
@hero78119 hero78119 changed the title [Draft] permutation circuit to support permutation fingerprint on rw_table [Draft] permutationchip to support permutation fingerprint on rw_table Oct 4, 2023
@github-actions github-actions bot added the crate-gadgets Issues related to the gadgets workspace member label Oct 4, 2023
@hero78119 hero78119 changed the base branch from main to proof-chunk October 4, 2023 10:09
@github-actions github-actions bot added T-bench Type: benchmark improvements crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member labels Oct 4, 2023
@hero78119 hero78119 marked this pull request as ready for review October 4, 2023 17:16
@github-actions github-actions bot added the crate-bus-mapping Issues related to the bus-mapping workspace member label Oct 5, 2023
@hero78119 hero78119 changed the title [Draft] permutationchip to support permutation fingerprint on rw_table [proof chunk] [fixing unittest] refactor evm_circuit/state_circuit to support proof chunk later on Oct 5, 2023
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.

I've done a very quick overview and left some comments! I didn't find anything strange :) Note that I didn't check the fingerprint chip

bus-mapping/src/exec_trace.rs Show resolved Hide resolved
zkevm-circuits/src/witness/block.rs Show resolved Hide resolved
zkevm-circuits/src/super_circuit.rs Show resolved Hide resolved
// annotate columns
rw_table.annotate_columns(meta);
mpt_table.annotate_columns(meta);
u8_table.annotate_columns(meta);
u10_table.annotate_columns(meta);
u16_table.annotate_columns(meta);

<RwTable as LookupTable<F>>::columns(&rw_table)
.iter()
.for_each(|c| meta.enable_equality(*c));
Copy link
Member

Choose a reason for hiding this comment

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

From the enable_equality I imagine that you're doing the fingerprint in another region and passing the cells via copy constraints. I think the fingerprint could be implemented as a gadget applied inline at every row of the state circuit, to skip the copy constraints. But this is an optimization, the current solution should work for now!

Copy link
Member Author

@hero78119 hero78119 Oct 19, 2023

Choose a reason for hiding this comment

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

You're right! Previously Rwtable continuity is carried over via a separate copy constraints, because the PermutationChip is meant to only cover fingerprints related logic. After your reminding, I think rwtable continuity can be realized via fingerprints as well, because row of rwtable vs fingerprint are one to one mapping, so we can leverage PermutationChip to do everything.

I have done the change to simplify design in new commit de55034 so we can save copy contraints effort on rw_table itself.

zkevm-circuits/src/evm_circuit.rs Outdated Show resolved Hide resolved
bus-mapping/src/circuit_input_builder/input_state_ref.rs Outdated Show resolved Hide resolved
gadgets/src/permutation.rs Outdated Show resolved Hide resolved
gadgets/src/permutation.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

LGTM!

zkevm-circuits/src/witness/rw.rs Show resolved Hide resolved
zkevm-circuits/src/state_circuit.rs Show resolved Hide resolved
@hero78119 hero78119 force-pushed the sm/proof-chunk branch 4 times, most recently from bc54696 to dfe0fe3 Compare October 23, 2023 15:04
@@ -8,7 +8,7 @@ use halo2_proofs::{
use std::collections::HashMap;

// Step dimension
pub(crate) const STEP_WIDTH: usize = 128;
Copy link
Member Author

@hero78119 hero78119 Oct 23, 2023

Choose a reason for hiding this comment

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

add extra columns 128 -> 131 to make EndBlock height remain 1

@@ -64,7 +64,7 @@ pub const FIXED_TABLE_LOOKUPS: usize = 8;
pub const TX_TABLE_LOOKUPS: usize = 4;

/// Rw Table lookups done in EVMCircuit
pub const RW_TABLE_LOOKUPS: usize = 8;
Copy link
Member Author

Choose a reason for hiding this comment

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

increase 5 columns for rw_lookup to make sure EndChunk height to be 1, same as EndBlock

EndChunk consume more rw_lookup than EndBlock is because EndChunk write StepState fields into rw_table

Copy link
Contributor

@davidnevadoc davidnevadoc left a comment

Choose a reason for hiding this comment

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

First pass of the review. Still have some parts to go through ( common_gadget and Chunkctx_table ), the rest LGTM.

zkevm-circuits/src/evm_circuit.rs Outdated Show resolved Hide resolved
@hero78119
Copy link
Member Author

will merge it first and left impromemt to future PR

@hero78119 hero78119 merged commit f5db508 into privacy-scaling-explorations:proof-chunk Oct 30, 2023
13 checks passed
hero78119 added a commit that referenced this pull request Nov 13, 2023
### Description

Depends on #1641 with extra commit: adding fingerprint equality check on
chronological/by address rw_table
Fingerprint check gate will be enable in last chunk last row

### instance columns, top down order match instance array order

chunk ctx
- [current chunk index, total chunk, initial rwc] // equal with
chunk_{i-1}
- [next chunk index, total chunk, next rwc] equal with chunk_{i+1}

pi circuit
- [pi digest lo, pi digest hi] // same across all chunks

state circuit
- [prev permutation fingerprint] // equal with chunk_{i-1}
- [next permutation fingerprint] // equal with chunk_{i+1}
- [alpha, gamma] // same across all chunks

evm circuit
- [prev permutation fingerprint] // equal with chunk_{i-1}
- [next permutation fingerprint] // equal with chunk_{i+1}
- [alpha, gamma] // same across all chunks

### 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
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.

[proof-chunk] super circuit implement new permutation gadget for evm rwtable lookup
5 participants