reconstruct memory when set EnableMemory to false in tracing #614

lightsing · 2022-07-11T04:33:16Z

This pull request implements issue described in #502.

It implements memory reconstruction when external tracer traced with EnableMemory: false.

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1202365709580411

lispc · 2022-07-11T04:34:28Z

@ed255 this PR can pass all the tests now. Would u like to have a look?

ed255

I've done an initial review round. Please take a look at my comments and questions :)

bus-mapping/src/evm/opcodes.rs

bus-mapping/src/evm/opcodes/sha3.rs

bus-mapping/src/evm/opcodes/returndatacopy.rs

bus-mapping/src/evm/opcodes/create.rs

eth-types/src/evm_types/opcode_ids.rs

bus-mapping/src/circuit_input_builder/input_state_ref.rs

bus-mapping/src/circuit_input_builder.rs

eth-types/src/lib.rs

bus-mapping/src/evm/opcodes/calldatacopy.rs

lispc · 2022-07-14T01:36:27Z

i am considering another approach. We modify every 'step.memory' read to 'call_context.memory' in this PR to avoid the RefCell.

So we will only debug_assert_eq next_step.memory== call_context.memory and will not put the reconstructed memory into next_step.memory

bus-mapping/src/evm/opcodes/sha3.rs

ed255 · 2022-07-25T13:32:39Z

i am considering another approach. We modify every 'step.memory' read to 'call_context.memory' in this PR to avoid the RefCell.

So we will only debug_assert_eq next_step.memory== call_context.memory and will not put the reconstructed memory into next_step.memory

@lightsing please ping me once the PR is ready for review again with the approach described by @lispc so that I take a look at it again.

bus-mapping/src/evm/opcodes.rs

bus-mapping/src/evm/opcodes/return.rs

lispc · 2022-07-26T04:56:05Z

bus-mapping/src/evm/opcodes/return.rs


-        state.handle_return(geth_step)?;
+        // can we use ref here?


life time problem?

lightsing · 2022-07-26T04:57:53Z

i am considering another approach. We modify every 'step.memory' read to 'call_context.memory' in this PR to avoid the RefCell.
So we will only debug_assert_eq next_step.memory== call_context.memory and will not put the reconstructed memory into next_step.memory

@lightsing please ping me once the PR is ready for review again with the approach described by @lispc so that I take a look at it again.

@ed255 ready for review

ed255 · 2022-07-26T09:57:17Z

@lightsing it seems that the tests have failed, can you please take a look? https://github.com/privacy-scaling-explorations/zkevm-circuits/runs/7516530148?check_suite_focus=true

lispc · 2022-07-28T22:55:46Z

@ed255 can you have a look at this？now finished i think

ed255

LGTM! I've left a minor comment, but otherwise looks good and I'm already approving this PR :)

bus-mapping/src/state_db.rs

…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>

* 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>

lispc marked this pull request as ready for review July 11, 2022 04:35

ed255 self-requested a review July 11, 2022 09:08

ed255 reviewed Jul 11, 2022

View reviewed changes

lispc reviewed Jul 12, 2022

View reviewed changes

bus-mapping/src/evm/opcodes/calldatacopy.rs Outdated Show resolved Hide resolved

lispc mentioned this pull request Jul 14, 2022

reconstruct memory when set EnableMemory to false in tracing scroll-tech/zkevm-circuits#164

Closed

15 tasks

lispc reviewed Jul 14, 2022

View reviewed changes

bus-mapping/src/evm/opcodes/sha3.rs Outdated Show resolved Hide resolved

lispc changed the title ~~reconstruct memory when set EnableMemory to false in tracing~~ WIP reconstruct memory when set EnableMemory to false in tracing Jul 18, 2022

impl memory reconstruction

6bef2a6

lightsing force-pushed the issues/502 branch from 533bf88 to 6bef2a6 Compare July 22, 2022 02:18

lightsing added 4 commits July 25, 2022 14:21

fix reconstruction order

9cae403

fix return copy length

4b9dc1e

revert change of OpCode trait

6cf7c64

fix reconstruction order

79fc546

Merge upstream/main into issues/502

25dd8cd

lightsing requested a review from miha-stopar as a code owner July 26, 2022 03:10

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-prover Issues related to the prover workspace member T-bench Type: benchmark improvements labels Jul 26, 2022

Merge remote-tracking branch 'upstream/main' into issues/502

42f6b1e

github-actions bot removed the crate-gadgets Issues related to the gadgets workspace member label Jul 26, 2022

lispc reviewed Jul 26, 2022

View reviewed changes

bus-mapping/src/evm/opcodes.rs Outdated Show resolved Hide resolved

lispc changed the title ~~WIP reconstruct memory when set EnableMemory to false in tracing~~ reconstruct memory when set EnableMemory to false in tracing Jul 26, 2022

lispc reviewed Jul 26, 2022

View reviewed changes

bus-mapping/src/evm/opcodes/return.rs Outdated Show resolved Hide resolved

lispc reviewed Jul 26, 2022

View reviewed changes

cleanup

4f6b590

cleanup x2

f39de6d

ed255 mentioned this pull request Jul 26, 2022

Official ethereum/tests StateTests execution #382

Closed

11 tasks

lispc mentioned this pull request Jul 26, 2022

(frozen) Scroll dev 0714 opt scroll-tech/zkevm-circuits#177

Closed

lispc and others added 7 commits July 27, 2022 10:52

tiny return data fix

b0f2428

fix execution tests

1feecef

Merge branch 'main' into issues/502

ecba1e4

fix some mem

b144e9c

cargo fmt

1aed24b

Merge branch 'main' into issues/502

306186c

Merge branch 'main' into issues/502

5bcdd7f

Merge branch 'main' into issues/502

0f1cf9d

ed255 approved these changes Jul 29, 2022

View reviewed changes

bus-mapping/src/state_db.rs Outdated Show resolved Hide resolved

clean codedb

960a0a6

github-actions bot removed the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jul 29, 2022

lispc merged commit a86aa1d into privacy-scaling-explorations:main Jul 29, 2022

lispc deleted the issues/502 branch July 29, 2022 10:53

lispc restored the issues/502 branch July 29, 2022 10:53

lispc mentioned this pull request Aug 1, 2022

SHA3 opcode #635

Merged

8 tasks

ed255 mentioned this pull request Aug 6, 2022

proposal: set EnableMemory to false when tracing #502

Closed

ed255 mentioned this pull request Oct 3, 2022

geth-utils: remove 10mb trace limit #817

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reconstruct memory when set EnableMemory to false in tracing #614

reconstruct memory when set EnableMemory to false in tracing #614

lightsing commented Jul 11, 2022 •

edited by icemelon

Loading

lispc commented Jul 11, 2022

ed255 left a comment

lispc commented Jul 14, 2022 •

edited

Loading

ed255 commented Jul 25, 2022

lispc Jul 26, 2022

lightsing Jul 26, 2022

lightsing commented Jul 26, 2022

ed255 commented Jul 26, 2022

lispc commented Jul 28, 2022

ed255 left a comment

reconstruct memory when set EnableMemory to false in tracing #614

reconstruct memory when set EnableMemory to false in tracing #614

Conversation

lightsing commented Jul 11, 2022 • edited by icemelon Loading

lispc commented Jul 11, 2022

ed255 left a comment

Choose a reason for hiding this comment

lispc commented Jul 14, 2022 • edited Loading

ed255 commented Jul 25, 2022

lispc Jul 26, 2022

Choose a reason for hiding this comment

lightsing Jul 26, 2022

Choose a reason for hiding this comment

lightsing commented Jul 26, 2022

ed255 commented Jul 26, 2022

lispc commented Jul 28, 2022

ed255 left a comment

Choose a reason for hiding this comment

lightsing commented Jul 11, 2022 •

edited by icemelon

Loading

lispc commented Jul 14, 2022 •

edited

Loading