Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: add another bytecode circuit to improve super circuit capacity #1368

Merged
merged 108 commits into from
Sep 6, 2024

Conversation

DreamWuGit
Copy link
Member

@DreamWuGit DreamWuGit commented Jul 18, 2024

Description

details of this PR targets to implement

  • evm circuit
    • add one more bytecode table (name bytecode_table_1)
    • modify SameContextGadget&CommonErrorGadget to do conditionally lookups base new field is_first_bytecode_table
    • implement algorithm find_two_closest_subset to dynamic split bytecodes into two lists
    • update opcodes gadget since SameContextGadget::assign_exec_step changes to include block, call.
    • modify dynamic copy table lookup with new field is_first_bytecode_table
  • copy circuit
    • copy table: add new flag column indicates which bytecode table is used.
    • do conditionally lookup bytecode table along with new flag column: is_first_bytecode_table
    • constrain is_first_bytecode_table: bool type & not changed in one copy event .
    • modify assign to pass bytecode_map to get which bytecode circuit is belong to.
  • super circuit
    • add new bytecode circuit and assign correct part of bytecode sequences.
    • update ccc rows in helpermin_num_rows_block_subcircuits by BytecodeCircuit::min_num_rows_block
    • add tests for multi bytecodes instances so that can be group into two sub circuit.
  • misc
    • add rust feature dual_bytecode to enable/disable two bytecode circuit side by side functionality.
    • testing
      • CI tests include new feature
      • mock prover integration tests
      • test tool
    • WIP: after testing then refactor or optimize some codes (opcode_lookup etc.)

@DreamWuGit DreamWuGit marked this pull request as draft July 18, 2024 01:28
@lispc
Copy link

lispc commented Jul 19, 2024

change assign_exec_step input to also include block and call. Inside SameContextGadget, alloc the cell and do assignment.

@DreamWuGit
Copy link
Member Author

change assign_exec_step input to also include block and call. Inside SameContextGadget, alloc the cell and do assignment.

after included block and call , exec_step no need to change as got enough information already.

@lispc lispc self-requested a review July 23, 2024 08:25
@z2trillion
Copy link
Member

I think it is much cleaner to add a in_first_bytecode_table boolean field to Bytecode. With that, we won't need to pass around bytecode_map anymore. I have a prototype PR here: #1402, to make this more clear.

@DreamWuGit
Copy link
Member Author

I think it is much cleaner to add a in_first_bytecode_table boolean field to Bytecode. With that, we won't need to pass around bytecode_map anymore. I have a prototype PR here: #1402, to make this more clear.

looking at it today.

@lispc lispc changed the title bytecode circuit refactor opt: add another bytecode circuit to improve super circuit capacity Aug 29, 2024
@roynalnaruto roynalnaruto self-requested a review August 29, 2024 10:34
@DreamWuGit
Copy link
Member Author

add comment for stable sort helper explicitly (5e840fa)

Copy link

@roynalnaruto roynalnaruto left a comment

Choose a reason for hiding this comment

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

Preliminary check done. I have some suggestions, some nit-picks and some opinions. Let me know what your thoughts are

zkevm-circuits/Cargo.toml Outdated Show resolved Hide resolved
zkevm-circuits/Cargo.toml Outdated Show resolved Hide resolved
zkevm-circuits/src/witness/block.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/witness/block.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/witness/block.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/evm_circuit/execution/codecopy.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/evm_circuit.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/copy_circuit.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/copy_circuit.rs Outdated Show resolved Hide resolved
Copy link

@roynalnaruto roynalnaruto left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@roynalnaruto roynalnaruto merged commit 6e6ee0e into develop Sep 6, 2024
17 checks passed
@roynalnaruto roynalnaruto deleted the bytecode_refactor branch September 6, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants