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

[keccak] Alternative implementations #634

Merged
merged 32 commits into from
Aug 24, 2022

Conversation

Brechtpd
Copy link
Collaborator

@Brechtpd Brechtpd commented Jul 25, 2022

Description: https://hackmd.io/NaTuIvmaQCybaOYgd-DG1Q?view

Contains two (~three) different approaches to do keccak:

  • bit implementation: does keccak on the bit level
  • packed implementation: does keccak by packing multiple bits together and uses lookups to do the transformations. The multi variant can use multiple rows/internal round to reduce the number of columns, but because of that it is a little bit less efficient than the non multi variant (~5%) . This variant is similar to the current approach followed, but the implementation is quite a bit different.

The current code should be good enough to compare performance between the different implementations.

@github-actions github-actions bot added 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 labels Jul 25, 2022
@CPerezz CPerezz self-requested a review August 10, 2022 07:26
Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

Review for the approach 1: Bit solution

So far it looks nice an simple. I would say a couple of things:

  • The naming used is a bit poor and does not help to understand what's what (b bits, c bits etc...).
  • The generation of the witness could be more automatic if we build methods as the KeccakRow generators for example as associated fns of KeccakCircuit.
  • There are some commented code parts on Theta gate lookups generation which I'm not sure they should stay or are just missed things.

Also, as mentioned. We need to take into account that #659 will imply that the RLC is not longer a witness we provide to the circuit and rather just a value we "compute and can pass over to other circuits" via lookups.

Aside from all that. Awesome work with this @Brechtpd I'm super impressed! Will continue with the packed and multi_packed reviews.
And from these, we should just pick the most performant and make some estimations to see if we should try to optimize more before going for the full impl with padding + table etc..

zkevm-circuits/src/keccak_circuit/keccak_bit.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/keccak_bit.rs Outdated Show resolved Hide resolved
Comment on lines 276 to 281
// q_end
cb.require_equal(
"q_end needs to be enabled on the first row",
meta.query_advice(q_end, Rotation::cur()),
1.expr(),
);
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use a Fixed or Constant for q_end?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess q_end is a bad name. The goal is for it to be used to mark the end of one keccak hash so a new keccak hash can be started. So where this will be set to 1 will change depending on the inputs.

|| Ok(F::from(round == 24)),
)?;
// q_end
region.assign_advice(
Copy link
Member

Choose a reason for hiding this comment

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

As said above. Why can't this be also fixed?

zkevm-circuits/src/keccak_circuit/keccak_bit.rs Outdated Show resolved Hide resolved
@AronisAt79
Copy link
Contributor

AronisAt79 commented Aug 11, 2022

zkevm-circuits PR#634 keccak bench results

1. packed_multi_keccak_bench

Cloud instance specs

Architecture:                    x86_64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
Address sizes:                   46 bits physical, 48 bits virtual
CPU(s):                          96
On-line CPU(s) list:             0-95
Thread(s) per core:              2
Core(s) per socket:              24
Socket(s):                       2
NUMA node(s):                    2
Vendor ID:                       GenuineIntel
CPU family:                      6
Model:                           85
Model name:                      Intel(R) Xeon(R) Platinum 8275CL CPU @ 3.00GHz
Stepping:                        7
CPU MHz:                         2999.998
BogoMIPS:                        5999.99

RAM: 192 Gb
Result

test packed_multi_keccak::tests::bench_packed_multi_keccak_circuit_prover has been running for over 60 seconds
End:     Packed Keccak Multi Proof generation with 19 rows .........................390.956s
Start:   Keccak Proof verification
End:     Keccak Proof verification .................................................41.143ms
test packed_multi_keccak::tests::bench_packed_multi_keccak_circuit_prover ... ok

max RAM at ~ 35Gb

2. packed_keccak_bench

Cloud instance specs

Architecture:                    x86_64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
Address sizes:                   46 bits physical, 48 bits virtual
CPU(s):                          96
On-line CPU(s) list:             0-95
Thread(s) per core:              2
Core(s) per socket:              24
Socket(s):                       2
NUMA node(s):                    2
Vendor ID:                       GenuineIntel
CPU family:                      6
Model:                           85
Model name:                      Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
Stepping:                        7
CPU MHz:                         2499.998
BogoMIPS:                        4999.99


RAM: 388 Gb
Result

test packed_keccak::tests::bench_packed_keccak_circuit_prover has been running for over 60 seconds
End:     Packed Keccak Proof generation with 19 rows ...............................2170.731s
Start:   Keccak Proof verification
End:     Keccak Proof verification .................................................97.567ms
test packed_keccak::tests::bench_packed_keccak_circuit_prover ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 6 filtered out; finished in 2190.35s


max RAM at ~ 190Gb

3. bit_keccak_bench

Cloud instance specs

Architecture:                    x86_64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
Address sizes:                   46 bits physical, 48 bits virtual
CPU(s):                          96
On-line CPU(s) list:             0-95
Thread(s) per core:              2
Core(s) per socket:              24
Socket(s):                       2
NUMA node(s):                    2
Vendor ID:                       GenuineIntel
CPU family:                      6
Model:                           85
Model name:                      Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
Stepping:                        7
CPU MHz:                         2499.998
BogoMIPS:                        4999.99


RAM: 388 Gb
Result

test bit_keccak::tests::bench_bit_keccak_circuit_prover has been running for over 60 seconds
End:     Bit Keccak Proof generation with 19 rows ..................................1819.173s
Start:   Keccak Proof verification
End:     Keccak Proof verification .................................................175.432ms
test bit_keccak::tests::bench_bit_keccak_circuit_prover ... ok

max RAM at ~ 255Gb

PR634-packed_keccak_bench.log
PR634-packed_multi_keccak_bench.log
PR634-bit_keccak_bench.log

@AronisAt79
Copy link
Contributor

@CPerezz @ChihChengLiang bench results in previous comment ^^^^

@Brechtpd
Copy link
Collaborator Author

Brechtpd commented Aug 11, 2022

Awesome, thanks a lot for this @AronisAt79! Results are quite a bit differently than I expected.

Some extra context for these benches:

1. packed_multi_keccak_bench

Uses 5 rows/internal round (assuming default settings were used, the main feature of this implementation is that the number of rows/internal rounds can be chosen) and so hashes 136 bytes/125 rows. In the bench 2**19/125 = 4194 keccak hashes are done in 391s giving 10.72 keccak_f/s. This circuit in this configuration uses ~200 columns.

2. packed_keccak_bench

Uses 1 row/internal round and so hashes 136 bytes/25 rows. In the bench 2**19/25 = 20971 keccak hashes are done in 2170s giving 9.66 keccak_f/s. This circuit uses ~800 columns. Surprises me that it's a bit slower than the packed_multi version... EDIT: Oh I see the packed_multi one was run on a CPU that was 3GHz vs 2.5GHz for this one, that makes sense then.

3. bit_keccak_bench

Uses 1 row/internal round and so also hashes 136 bytes/25 rows. In the bench 2**19/25 = 20971 keccak hashes are done in 1819s giving 11.53 keccak_f/s. This circuit uses ~2000 columns. This result is much closer to the packed implementations than I expected and I expected this one to be quite a bit faster. (EDIT: I can do 4 hashes/s for a circuit with 2**14 rows on my 10 year 4-core cpu, so seems like something on the prover doesn't scale as it should with many more cores (and better cores). I would expect this circuit to do something like 20 keccak hashes/s on the bench pc).

Pending and future prover optimizations should improve all of these quite a bit still.

All columns given as an estimate because the actual number of columns depends on the circuit height. The circuits automatically builds themselves to use the biggest lookup tables possible so that the number of lookups (and so also the number of columns required for those lookups) is reduced as much as possible.

@CPerezz
Copy link
Member

CPerezz commented Aug 12, 2022

@Brechtpd the only issue I see with the packed approaches is the big amount of RAM required. Considering specially that the bit approach performs better (at least now) than the others consuming less resources.

I'd also edit your prev message to say that they're not hashes/s but keccak_f's/s or permutations/s.
As the hash itself does not have a magnitude since the input can have any size.

Aside from that, as we commented in the past, we will check with the authors of Caulk whether it can help us to bring down these lookup costs and how could we integrate it within halo2.

@Brechtpd
Copy link
Collaborator Author

The implementations are now complete, but I will spend a bit more time cleaning up the code some more (and incorporating the feedback above).

@Brechtpd Brechtpd marked this pull request as ready for review August 19, 2022 23:02
@CPerezz
Copy link
Member

CPerezz commented Aug 23, 2022

Hey @Brechtpd is it clean now so that we can go for a merge here?
What do you think??

@Brechtpd
Copy link
Collaborator Author

Can be merged whenever you want for me. I will still do some changes to make things better, but like discussed that can be done in a follow up PR.

Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

I added some comments. @CPerezz Feel free to merge if we want to avoid conflict for halo2 upgrade. I still want to do more reviews, but I can add comments post-merge.

zkevm-circuits/src/keccak_circuit/util.rs Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/util.rs Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/util.rs Outdated Show resolved Hide resolved
}

/// Pack bits into a word
pub fn pack(bits: &[u8]) -> Word {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be EVM word(256 bits) or Keccak word(64 bits)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think everywhere I talk about words they are keccak words (because keccak doesn't directly have anything to do with EVM words I think it should be clear from context, or at least I hope so).

Copy link
Collaborator

Choose a reason for hiding this comment

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

The type Word here is pub type Word = U256; from zkevm-circuits/eth_types. Can we define a type Word = u64 and replace U256 with it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Something >u64 is needed here because this will pack all 64 sparse bits into a single field element. With each sparse bit taking up 3 bits, the full packed word needs to be stored in at least 64*3 = 192 bits. Though I'm not sure anymore why I don't simply do all the math on a Field value directly, will see if it's easy to switch to that instead.

zkevm-circuits/src/keccak_circuit/util.rs Show resolved Hide resolved
@CPerezz CPerezz self-requested a review August 24, 2022 08:34
Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

LGTM with the current status.

I would merge this to prevent massive conflicts once the halo2 update PR is merged here.
I encourage anyway to keep the work of cleanup. And probably we should consider to start with the entire circuit as I attempted in #542

@CPerezz CPerezz merged commit d16ac9b into privacy-scaling-explorations:main Aug 24, 2022
CPerezz added a commit that referenced this pull request Aug 31, 2022
As decided on
#634
we're going to use the bit-approach merged there. And so, most of the
circuits here are no longer required/ will be added back when needed.
CPerezz added a commit that referenced this pull request Sep 1, 2022
* change: Update halo2 tag version in all Cargo.toml

* change: Use `Value` instead of `AssignedCell`.

* remove: Strip keccak circuits from the repo

As decided on
#634
we're going to use the bit-approach merged there. And so, most of the
circuits here are no longer required/ will be added back when needed.

* tmp

* fix: RegionCtx new API adoption without &mut holding

* change: Use KZG, SHPLONK and Bn256 instead of generics

The first attempt to update the `prover` crate was to basically use full
generics everywhere.

The issue is that we ended up hitting trait safety and the complexity
started to increase significantly.
Hence, since we are always using the same commitment scheme and curve,
it's easier to switch to concrete types.

* remove: Delete prover crate as was migrated to zkevm-chain

* remove: Remove legacy keccak bench file

* remove: Remove `group` dep from all crates Cargo.toml

* update: Port circuit-benchmarks to latest halo2 version

* update: Port integration-tests to newest halo2 release

* Fix errors in zkevm-circuits tests

* fix: Errors introduced in `main` branch merge

* fix: Load range_chip tables in SignVerigy and Tx Configs

* Fix Synthesis and lookup table errors

* fix: Apply suggestions from @han0110 review

Co-authored-by: Han <tinghan0110@gmail.com>

* Fix Signature errors introduced on Compressed treatment

* fix: Fix tests and address review comments

Co-authored-by: Han <tinghan0110@gmail.com>
@ed255 ed255 mentioned this pull request Nov 10, 2022
5 tasks
jonathanpwang pushed a commit to jonathanpwang/zkevm-circuits that referenced this pull request Dec 29, 2022
* WIP keccak permutation

* Better theta lookup approach

* WIP improvements

* Better bit based implementation

* WIP packed keccak

* Packed absorb

* Multi stage theta (first part)

* Fixes

* Added better performing mode at certain circuit sizes

* Generalized splitting

* small improvements

* fixes

* small improvements

* Added multi-row per round version

* cleanup code

* Add padding on the witness side

* Preparations for padding + rlc squeeze in bit implementation

* Feedback + packed squeeze

* multi packed squeeze

* Finish keccak implementations (added padding/data rlc/length)

* cleanup code

* More accurate benchmark results by filling the full circuit

* more cleanup

* more cleanup

* more cleanup

* cleanup

* cleanup

* even more cleanup + feedback

* Fix docs error
jonathanpwang pushed a commit to jonathanpwang/zkevm-circuits that referenced this pull request Dec 29, 2022
* change: Update halo2 tag version in all Cargo.toml

* change: Use `Value` instead of `AssignedCell`.

* remove: Strip keccak circuits from the repo

As decided on
privacy-scaling-explorations#634
we're going to use the bit-approach merged there. And so, most of the
circuits here are no longer required/ will be added back when needed.

* tmp

* fix: RegionCtx new API adoption without &mut holding

* change: Use KZG, SHPLONK and Bn256 instead of generics

The first attempt to update the `prover` crate was to basically use full
generics everywhere.

The issue is that we ended up hitting trait safety and the complexity
started to increase significantly.
Hence, since we are always using the same commitment scheme and curve,
it's easier to switch to concrete types.

* remove: Delete prover crate as was migrated to zkevm-chain

* remove: Remove legacy keccak bench file

* remove: Remove `group` dep from all crates Cargo.toml

* update: Port circuit-benchmarks to latest halo2 version

* update: Port integration-tests to newest halo2 release

* Fix errors in zkevm-circuits tests

* fix: Errors introduced in `main` branch merge

* fix: Load range_chip tables in SignVerigy and Tx Configs

* Fix Synthesis and lookup table errors

* fix: Apply suggestions from @han0110 review

Co-authored-by: Han <tinghan0110@gmail.com>

* Fix Signature errors introduced on Compressed treatment

* fix: Fix tests and address review comments

Co-authored-by: Han <tinghan0110@gmail.com>
jonathanpwang pushed a commit to axiom-crypto/zkevm-circuits that referenced this pull request Aug 1, 2023
* add MemoryWordAddress gadget and todos

* update buss mapping MemoryOp with word type value

* update mload/mstore with memory word

* fix mload buss mapping test

* add evm circuit with new memory word type and fix mstore circuit test pass

* fix mload&mstor8 circuit

* udpate error invalid creation code circuit

* update calldataload buss mapping

* fix calldataload buss map test

* update calldataload circuit&test pass

* update log* sha3 calldatacopy buss mapping tests

* add state circuit constraint for memory word tag

* copy circuit buss mapping add mask field into copy event

* copy circuit add value_wrod_rlc

* add word_index into copy table and align calldata to word

* fix word_index lt gate issue and move some columns

* fix memor word lookup & rw counter

* calldatacopy fix evm circuit root test

* fix calldatacopy evm circuit internal test, memory offset overflow fail

* fix codecopy bussmapping and copy circuit test pass

* update codecopy evm circuit

* update exrcodecopy and gen_copy_steps_for_log

* try log with byte lookup and refactor

* update log* with log byte to word and disable rows[0].addr + 1 == rows[2].addr

* update sha3 copy circuit & evm circuit

* Fix test case `test_precompiled_call` in CALL OP. (privacy-scaling-explorations#485)

* fix log with non zero start slot and comment multi buss mapping tests

* add create/return test into copy circuit

* update return non root non create to generate diff r/w bytes

* reading with writing word one by one

* [feat] returndatacopy implemented (privacy-scaling-explorations#506)

* init impl of returndatacopy

* add aux_bytes to CopyEvent

* add read_steps and write_steps and rlc_acc_read and rlc_acc_write

* add assert

* using real dst word value

* rlc_acc_read/write only counts when not padding

* fix overflow

* fix typo

* add read_word

* fix rwc

* write after read one by one

* add test

* fix constraint

* fix typo

* update test

* fix lookup

* fix rw_increase

* update return gadget

* fix root create in return tests

* fix returndatacopy, add calldatacopy non root

* fix trace test with last_callee_memory

* fix calldatacopy non root

* update create pass for non persistent etc.

* fix calldatacopy test

* add constrain rlc_acc_read == rlc_acc_write

* cargo fmt

---------

Co-authored-by: Dream Wu <wwuwwei@126.com>

* some fix

* enable and remove state circuit tods

* recover test for MemoryWordOp

* recover test for TxLogOp

* mask is boolean

* word_index [0--32] increase by 1

* fix copy circuit degree increase issue

* update constraint for read=write vaule for non memory to memory

* enable addr change of copy circuit and rename is_word_continue

* Apply suggestions from code review

Co-authored-by: naure <naure@users.noreply.github.com>

* fix not enough row

* memory_opt_opcodes: constraints for MLOAD/MSTORE/MSTORE8/CALLDATALOAD (privacy-scaling-explorations#533)

* memory_opt_opcodes: constraints for MLOAD/MSTORE/MSTORE8

* memory_opt_opcodes: refactor all memory alignment logic into a gadget

* memory_opt_opcodes: connect calldata and memory values

* memory_opt_opcodes: switch to BE-order to match current copy circuit

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* fix codecopy & log test failures

* fix return/create test failures

* fix returndatacopy& insufficient balance in create

* fix calldatacopy and some clippy

* add constraint for rw counter (privacy-scaling-explorations#532)

* fix tx_log table issue

* constraint rw_counter and rw_inc_left

* constraint rw_counter

* simplify constraint

* add column is_mem_to_mem to reduce the degree

* replace other usage of memory to memory tag query

* fix value_acc

* fix merge

* remove is_mem_to_mem

* fix build

* apply review

* fix merge

* restore addr contraint

* Fix/callop memory (privacy-scaling-explorations#555)

* add gen_copy_steps for callop precompile

* calc rlc from memory word

* add rws count

* fix lookup

* change to memory to memory

* [memory_opt] fix failed precompile test (privacy-scaling-explorations#547)

* Replace `call_id` with `caller_id` for copy steps of input bytes.

* Set `max_copy_rows` to 1100 in precompile test.

* update tests

---------

Co-authored-by: Steven Gu <asongala@163.com>

* fix degree issue

* using real length in copy table lookup (privacy-scaling-explorations#565)

* add real_length

* add constraint

* fmt

* remove bytes_length_word

* fix comment

* fix clippy

* replace print to trace

* do not need to suppress theses

* fix import

* fix other crates

* fix incomplete comment

* Add condition for input, output and return bytes for precompile (as bus-mapping). (privacy-scaling-explorations#571)

* Fix to calculate copy length as the minimum value (as bus-mapping). (privacy-scaling-explorations#573)

* remove Memory related codes (privacy-scaling-explorations#574)

* remove Memory related

* update inline doc

* remove memory constraints

* fix inline doc

* remove outdated test

* Memory opt update (privacy-scaling-explorations#541)

* memory_opt_opcodes: constraints for MLOAD/MSTORE/MSTORE8

* memory_opt_opcodes: refactor all memory alignment logic into a gadget

* memory_opt_opcodes: connect calldata and memory values

* memory_opt_opcodes: switch to BE-order to match current copy circuit

* memory_opt_update: add memory value_prev to the RW API

* memory_opt_update: track value_prev for memory writes

* memory_opt_update: check value_prev of memory ops

* memory_opt_update: common lookup with mstore8

* memory_opt_update: add column word_prev in copy circuit

* memory_opt_update: find value_prev in memory_write_word

* memory_opt_update: move common logic into read_memory_word

* memory_opt_update: fix tests

* memory_opt_update: move logic to a common read_memory_caller function

* memory_opt_update: fix, simplify, optimize RETURN copy

* memory_opt_update: refactor calldatacopy

* memory_opt_update: calldatacopy with value prev

* memory_opt_update: revert new_read name

* memory_opt_update: returndatacopy with value prev

* memory_opt_update: common function align_range with correct edge cases

* memory_opt_update: optimize codecopy

* memory_opt_update: optimize log copy

* memory_opt_update: fix LOG off-by-one word count

* memory_opt_update: prepare callop write_memory_words

* memory_opt_update: remove unnecessary stack_index

* memory_opt_update: read actual log data

* fix memory_word_value to memory_word_pair in callop

* memory_opt_update: fix indexes in circuit_row

* memory_opt_update: take rwc from bus-mapping instead of outdated calculations

* memory_opt_update: remove redundant RWC calculations

* memory_opt_update: remove redundant RWC calculations

* memory_opt_update: remove redundant RWC calculations

* memory_opt_update: remove redundant RWC calculations

* memory_opt_update: track copy RWs for check_rw_lookups

* memory_opt_update: fix calldatacopy source length

* add prev bytes and refactor copy event  (privacy-scaling-explorations#569)

* refactor copy event bytes

* update to use CopyBytes sturcture

* make calldatacopy with pre_bytes & remove bytes_read_prev

* set prev_write_bytes in copy table and pass calldatacopy root

* update code copy with pre bytes

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* fix returndatacopy and state test (privacy-scaling-explorations#582)

* fix_returndatacopy and state test

* update state_circuit_simple_2 test

* use new_write

* memory_opt_update: fix copy_circuit_invalid_tx_log with a more flexible test

* memory_opt_update: remove outdated constraint on log

* Revert "memory_opt_update: in log, use extend_for_range"

This reverts commit fb5ae3e.

* fix state circuit: value_prev column is initial_value for first access for (ext)codecopy (privacy-scaling-explorations#583)

* fix state circuit: value_prev column is initial_value for first access

* remove unused comment

* fix_statetest_extcodecopy: simplify copy_start

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt_update: in log, use extend_for_range

* memory_opt_circuit: constrain is_tx_log

* memory_opt_update: fix check_rw_lookup again

* fix precompile prev bytes (privacy-scaling-explorations#590)

* fix prev_bytes

* fix memory word count

---------

Co-authored-by: DreamWuGit <wwuwwei@126.com>

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: Dream Wu <wwuwwei@126.com>
Co-authored-by: Akase Cho <lightsing@users.noreply.github.com>

* refactor memory_write_word_prev_bytes to memory_write_word

* fix clippy

* misc updates

* fix constraint

* fix merge

* fix fmt & remove unused comment

* codecopy buss mapping memory word test

* update sha3 test and mark get_addr_shift_slot deprecated

* better to use usize in addresses

* refactor to use Memory::align_range

* fix clippy

* use Memory::align_range for return_revert

* remove unnecessary cast

* add a cfg

* rename MemoryWord back to Memory

* rename MemoryWord back to Memory

* disable memory tracing by default

* fix some typos

* use &memory_updated

* include copy_rwc_inc in rw_offset

* refactor helper get_copy_bytes

* not a soundness problem anymore

* apply review

* refactor write_chunks&write_chunk_for_copy_step

* Memory opt refactor (privacy-scaling-explorations#603)

* initial refactor

* fix code_copy

* fix

* fix

* fix

* remove debug print

* remove debug print

* fix return/create

* fix compile

* add memory_range

* use word_count

* cargo fmt && clippy

* add missing docs

* remove gen_copy_steps

* fmt

* refactor for all copy bytes using rlc_acc equality (privacy-scaling-explorations#604)

Co-authored-by: naure <naure@users.noreply.github.com>

* fix return_revert

* copy_simplify_addr: always increment the address (privacy-scaling-explorations#609)

* copy_simplify_addr: always increment the address

* copy_simplify_addr: derive addr_slot at once at the end

* copy_simplify_addr: control by front mask + return value prev

* copy_simplify_addr: replace addr_slot by an expression

* copy_simplify_addr: adjust limits for CREATE tests

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* fix begin_tx copy event

* fix clippy

* fix scroll feature test failure

* Copy: fix conditions (privacy-scaling-explorations#616)

* memory_opt_counters: replace incorrect non_pad_non_mask with is_continue

* memory_opt_counters: constraints on front_mask

* memory_opt_continue: remove redundant conditation

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: DreamWuGit <wwuwwei@126.com>

* memory_opt_word_end: replace broken LtChip with IsEqualChip (privacy-scaling-explorations#617)

* memory_opt_counters: replace incorrect non_pad_non_mask with is_continue

* memory_opt_counters: constraints on front_mask

* memory_opt_word_end: replace broken LtChip with IsEqualChip

* memory_opt_continue: remove redundant conditation

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: DreamWuGit <wwuwwei@126.com>

* memory_opt_pad: replace LtChip with a simple column (privacy-scaling-explorations#620)

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt: Ensure that the word operation completes.

* memory_opt_rlc: simplify rlc_acc check (privacy-scaling-explorations#621)

*replace LtChip with a simple column

* memory_opt_rlc: simplify rlc_acc check

* memory_opt_rlc: accumulate RLC or copy based on mask

* memory_opt_rlc: simplify RLC with a single column

* memory_opt_rlc: use value_acc for read/write equality

* memory_opt_rlc: enforce RLC initial value

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* word_rlc: introduce value_prev in copy circuit (privacy-scaling-explorations#626)

* word_rlc: introduce value_prev in copy circuit

* memory_opt_word_rlc: verify word RLCs

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt_length: constrain length and mask (privacy-scaling-explorations#628)

* memory_opt_length: track both source and destination lengths. Simplify constraints

* memory_opt_length: constrain the shape of the mask

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt_rwc: fix and simplify RW counter logic (privacy-scaling-explorations#631)

* memory_opt_rwc: fix and simplify RW counter logic

* memory_opt_rwc: simplify CopyEvent rw functions

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt: Prevent an event from spilling into the disabled rows.

* Delete `memory.clone` in invalid-creation-code error. (privacy-scaling-explorations#632)

* memory_opt: value column is in the first phase

* memory_opt_check_align: require aligned memory addresses in State (privacy-scaling-explorations#634)

* memory_opt_check_align: require aligned memory addresses in State

* memory_opt_check_align: tests

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* Copy: remove LtChip (privacy-scaling-explorations#635)

* memory_opt_rm_ltchip: replace LessThan with IsEqual

* memory_opt_rm_ltchip: support other rotations in IsEqualChip

* memory_opt_rm_ltchip: constrain is_pad from is_src_end_next

* memory_opt_rm_ltchip: remove the old chip

* memory_opt_rm_ltchip: add doc and assertion

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt_readability: give a name to Rotations (privacy-scaling-explorations#636)

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt: organize variables and comments

* Copy Gadgets (privacy-scaling-explorations#637)

* memory_opt_gadgets: move code into a function

* memory_opt_gadgets: move code into constrain_word_rlc()

* memory_opt_gadgets: move code into constrain_value_rlc()

* memory_opt_gadgets: move code into constrain_event_rlc_acc()

* memory_opt_gadgets: constrain_bytes_left

* memory_opt_gadgets: doc

* memory_opt_gadgets: move code into constrain_rw_counter. rustfmt works again!

* memory_opt_gadgets: constrain_forward_parameters and constrain_address

* memory_opt_gadgets: move code to constrain_mask()

* memory_opt_gadgets: move code to constrain_is_pad()

* memory_opt_gadgets: move code to constrain_non_pad_non_mask()

* memory_opt_gadgets: move code to constrain_first_last

* memory_opt_gadgets: constrain_masked_value and constrain_must_terminate

* memory_opt_gadgets: reorg is_pad and is_last

* memory_opt_gadgets: reorder gadgets to match the circuit

* memory_opt_gadgets: move code into constrain_tag. Done!

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt: remove unnecessary is_event column

* memory_opt: remove duplicate code in CopyTable::assignments (privacy-scaling-explorations#638)

Co-authored-by: Aurélien Nicolas <info@nau.re>

* memory_opt: hide details of extend_at_least

* Avoid whole memory clone when generating copy steps (privacy-scaling-explorations#629)

* Avoid whole memory clone for CODECOPY and EXTCODECOPY.

* Update

* Fix to extend memory as range(begin_slot, full_length), and update `src_addr_end`.

* Delete unused function `write_memory_words` in callop.

* Delete memory clone in callop.

* Fix lint.

* Delete memory clone in calldatacopy.

* Update returndatacopy, and extract to a common function.

* Fix lint.

* Update bytecode copy.

* Small fix.

* Fix failed testool cases for memory-copy. (privacy-scaling-explorations#642)

---------

Co-authored-by: Steven <asongala@163.com>
Co-authored-by: Akase Cho <lightsing@users.noreply.github.com>
Co-authored-by: lightsing <light.tsing@gmail.com>
Co-authored-by: naure <naure@users.noreply.github.com>
Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
omega000133 added a commit to omega000133/zkevm-circuits that referenced this pull request May 17, 2024
* change: Update halo2 tag version in all Cargo.toml

* change: Use `Value` instead of `AssignedCell`.

* remove: Strip keccak circuits from the repo

As decided on
privacy-scaling-explorations/zkevm-circuits#634
we're going to use the bit-approach merged there. And so, most of the
circuits here are no longer required/ will be added back when needed.

* tmp

* fix: RegionCtx new API adoption without &mut holding

* change: Use KZG, SHPLONK and Bn256 instead of generics

The first attempt to update the `prover` crate was to basically use full
generics everywhere.

The issue is that we ended up hitting trait safety and the complexity
started to increase significantly.
Hence, since we are always using the same commitment scheme and curve,
it's easier to switch to concrete types.

* remove: Delete prover crate as was migrated to zkevm-chain

* remove: Remove legacy keccak bench file

* remove: Remove `group` dep from all crates Cargo.toml

* update: Port circuit-benchmarks to latest halo2 version

* update: Port integration-tests to newest halo2 release

* Fix errors in zkevm-circuits tests

* fix: Errors introduced in `main` branch merge

* fix: Load range_chip tables in SignVerigy and Tx Configs

* Fix Synthesis and lookup table errors

* fix: Apply suggestions from @han0110 review

Co-authored-by: Han <tinghan0110@gmail.com>

* Fix Signature errors introduced on Compressed treatment

* fix: Fix tests and address review comments

Co-authored-by: Han <tinghan0110@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants