This repository was archived by the owner on Apr 18, 2025. It is now read-only.
add prev bytes and refactor copy event #569
Merged
Conversation
This file contains hidden or 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
naure
reviewed
Jun 28, 2023
naure
reviewed
Jun 29, 2023
naure
reviewed
Jun 29, 2023
| #[derive(Clone, Debug)] | ||
| pub struct CopyBytes { | ||
| /// Represents the list of (bytes, is_code, mask) copied during this copy event | ||
| pub bytes: Vec<(u8, bool, bool)>, |
There was a problem hiding this comment.
Since there is already a value in each copy step, that is a good place for value_prev too. In the same tuple, like this:
(value, value_prev, is_code, mask)
And there is only read_steps and write_steps.
Then this CopyBytes structure is simpler or not needed. And all the changes below with the tuples (copy_steps, prev_bytes) can be avoided.
There was a problem hiding this comment.
already added in value_prev in CopyStep
There was a problem hiding this comment.
Correct, I meant that this same structure could be instead of (u8, bool, bool)
naure
reviewed
Jun 29, 2023
| /// Represents the list of (bytes, is_code, mask) read to copy during this copy event | ||
| pub aux_bytes: Option<CopyEventSteps>, | ||
| /// Represents the list of bytes related during this copy event | ||
| pub copy_bytes: CopyBytes, |
There was a problem hiding this comment.
Here we could use the structure CopyStep instead of (u8, bool, bool)
There was a problem hiding this comment.
prev_byte doesn't care about is_code, is_mask . use CopyStep may import more unused fields.
There was a problem hiding this comment.
I meant to add only value_prev to the existing tuple, next to value. is_mask and is_code are part of a step, and value and value_prev also part of the same step.
naure
reviewed
Jun 29, 2023
| @@ -320,6 +320,8 @@ impl_expr!(CopyDataType, u64::from); | |||
| pub struct CopyStep { | |||
| /// Byte value copied in this step. | |||
| pub value: u8, | |||
| /// Byte value before this step. | |||
| pub prev_value: Option<u8>, | |||
There was a problem hiding this comment.
This is the most logical approach: everywhere there is a value, there is a value_prev too.
There was a problem hiding this comment.
We could say that in a reader step, value_prev = value. That would simplify code without the Option wrapper, and always handling reader and writer the same way.
There was a problem hiding this comment.
currently only write step has value_prev, if do the same for reader step, calculating additinal value_read_prev_rlc making no sense since it is equal to value_word_rlc. wdyk?
There was a problem hiding this comment.
Sure then the reader case is different as an optimisation.
naure
reviewed
Jun 29, 2023
| @@ -1708,11 +1737,14 @@ impl<'a> CircuitInputStateRef<'a> { | |||
| // memory word writes to destination word | |||
| for chunk in code_slot_bytes.chunks(32) { | |||
There was a problem hiding this comment.
To get the value_prev into the copy_steps, we could merge these two loops like this:
for (indexes, chunk) in code_slot_bytes.enumerate().chunks(32) {
let prev_bytes = …memory write…
…
for ((idx, value), value_prev) in indexes.zip(chunk).zip(prev_bytes) {
let address = dst_begin_slot + idx;
…There was a problem hiding this comment.
It’s nice because the data is processed once and it’s clear that the two actions are connected: write memory AND generate copy events for the same data.
DreamWuGit
added a commit
that referenced
this pull request
Jul 4, 2023
* 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 (#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 (#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 (#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 (#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>
lispc
added a commit
that referenced
this pull request
Jul 17, 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. (#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 (#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 (#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 (#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 (#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 (#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 (#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). (#571) * Fix to calculate copy length as the minimum value (as bus-mapping). (#573) * remove Memory related codes (#574) * remove Memory related * update inline doc * remove memory constraints * fix inline doc * remove outdated test * Memory opt update (#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 (#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 (#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 (#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 (#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 (#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 (#604) Co-authored-by: naure <naure@users.noreply.github.com> * fix return_revert * copy_simplify_addr: always increment the address (#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 (#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 (#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 (#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 (#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 (#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 (#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 (#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. (#632) * memory_opt: value column is in the first phase * memory_opt_check_align: require aligned memory addresses in State (#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 (#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 (#636) Co-authored-by: Aurélien Nicolas <info@nau.re> * memory_opt: organize variables and comments * Copy Gadgets (#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 (#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 (#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. (#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>
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
memory rw table enable value_prev field, which cause copy circuit lookup fails. need to add prev bytes to rlc value_word_prev .
Issue Link
[link issue here]
Type of change
Contents