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

fix dummy error handling #979

Merged
lispc merged 4 commits into
privacy-ethereum:mainfrom
scroll-tech:fix_dummy_error
Dec 15, 2022
Merged

fix dummy error handling #979
lispc merged 4 commits into
privacy-ethereum:mainfrom
scroll-tech:fix_dummy_error

Conversation

@DreamWuGit

@DreamWuGit DreamWuGit commented Dec 12, 2022

Copy link
Copy Markdown
Collaborator

since many of error states are also in dummy way both buss mapping and circuit side , recent commit make some error states go to fn_gen_error_state_associated_ops and use wrong dummy gens_ops. fix this by first checking fn_gen_error_state_associated_ops cover it , if not, use normal dummy way to handle it which contains the error info in exec step.

@github-actions github-actions Bot added the crate-bus-mapping Issues related to the bus-mapping workspace member label Dec 12, 2022
@lispc lispc marked this pull request as ready for review December 13, 2022 02:25

@han0110 han0110 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice fix, so now all unimplemented exception states will have handle_return called.

Also IMO I think the logic here (line 306-324) could be simplified to:

        // For exceptions that have been implemented
        if let Some(fn_gen_error_ops) = fn_gen_error_state_associated_ops(&exec_error) {
            return fn_gen_error_ops(state, geth_steps);
        } else {
            // For exceptions that already enter next call context, but fail immediately
            // (e.g. Depth, InsufficientBalance), we still need to parse the call.
            if geth_step.op.is_call_or_create() && !exec_step.oog_or_stack_error() {
                let call = state.parse_call(geth_step)?;
                state.push_call(call);
            // For exceptions that fail to enter next call context, we need
            // to restore call context of current caller
            } else {
                state.gen_restore_context_ops(&mut exec_step, geth_steps)?;
            }
            state.handle_return(geth_step)?;
            return Ok(vec![exec_step]);
        }

I just tested locally and it works, but not sure if there is any edge case we haven't covered (I guess there are many...), so it'd be nice if you could help to check if it works and perhaps refactor in this way, since I found I need to understand what exec_step.oog_or_stack_error() && !geth_step.op.is_call_or_create() means every time I review tihs part.

@DreamWuGit

Copy link
Copy Markdown
Collaborator Author

Nice fix, so now all unimplemented exception states will have handle_return called.

Also IMO I think the logic here (line 306-324) could be simplified to:

should be correct, I will test it .

@DreamWuGit

Copy link
Copy Markdown
Collaborator Author

Nice fix, so now all unimplemented exception states will have handle_return called.
Also IMO I think the logic here (line 306-324) could be simplified to:

should be correct, I will test it .

that's OK , updated in 7c78298. thanks!.

@han0110

han0110 commented Dec 15, 2022

Copy link
Copy Markdown
Contributor

that's OK , updated in 7c78298. thanks!.

Thanks a lot!

@lispc lispc merged commit 20b4db0 into privacy-ethereum:main Dec 15, 2022
@lispc lispc deleted the fix_dummy_error branch December 15, 2022 07:58
pinkiebell added a commit to pinkiebell/zkevm-circuits that referenced this pull request Dec 15, 2022
commit cc1e0d6
Author: z2trillion <z2trillion@users.noreply.github.com>
Date:   Thu Dec 15 04:02:49 2022 -0500

    Add TODO (privacy-ethereum#961)

    Co-authored-by: Mason Liang <mason@scroll.io>
    Co-authored-by: Carlos Pérez <37264926+CPerezz@users.noreply.github.com>
    Co-authored-by: Chih Cheng Liang <chihchengliang@gmail.com>

commit 20b4db0
Author: DreamWuGit <wwuwwei@126.com>
Date:   Thu Dec 15 15:58:43 2022 +0800

    fix dummy error handling  (privacy-ethereum#979)

    * fix dummy error

    * minor refacotr

commit 5710e30
Author: DreamWuGit <wwuwwei@126.com>
Date:   Thu Dec 15 10:15:24 2022 +0800

    fix balance test (privacy-ethereum#989)

commit 4840a5a
Author: Zhang Zhuo <mycinbrin@gmail.com>
Date:   Wed Dec 14 21:57:38 2022 +0800

    fix(evm): get_test_degree (privacy-ethereum#983)

    * fix(evm): get_test_degree

    * fmt

    * change bytecode table row estimation

commit c78c86d
Author: smtmfft <99081233+smtmfft@users.noreply.github.com>
Date:   Mon Dec 12 16:05:08 2022 +0800

    Math gadgets refactory (privacy-ethereum#920)

    * refactory is_zero

    * add a few gadget files

    * fix import warn

    * Split gadget into independent files

    * Fix lints

    * Add bye size gadget test case

    * Split tests into single function

    * Fix comments from brecht

    * update unit test organization for better self explain

    * fix review comments

    * fix review comments

    * fix review comments

    * remove useless tests

    * Update zkevm-circuits/src/evm_circuit/util/math_gadget/min_max.rs

    Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

    * Update zkevm-circuits/src/evm_circuit/util/math_gadget/addwords.rs

    Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

    * improve: mod dependency

    * keep fixing review comments

    * refine tests

    * Fix test framework issue

    Signed-off-by: smtmfft <smtm@taiko.xyz>

    * use macro to make better readability

    * remove dup marco def

    * use try_test macro

    * code refine

    * update test case

    * fix max carry test case

    * fix review comments

    * refine annotations

    * fix review comments

    * fix review comments

    Signed-off-by: smtmfft <smtm@taiko.xyz>
    Co-authored-by: smtmfft <smtm@taiko.xzy>
    Co-authored-by: john xu <john@taiko.xyz>
    Co-authored-by: Brecht Devos <Brechtp.Devos@gmail.com>

commit ee1cfbd
Author: zengzengzenghuy <85567868+zengzengzenghuy@users.noreply.github.com>
Date:   Fri Dec 9 19:09:51 2022 +0800

    fix: typo in memory.rs (privacy-ethereum#962)

    * fix: typo in memory.rs

    * fix: typo in memory.rs

commit f444466
Author: Leo Lara <leolara@users.noreply.github.com>
Date:   Thu Dec 8 19:01:59 2022 +0700

    Integration tests to use actual prover (privacy-ethereum#878)

    Closes privacy-ethereum#789

commit a7f87b3
Author: Carlos Pérez <37264926+CPerezz@users.noreply.github.com>
Date:   Wed Dec 7 16:34:59 2022 +0100

    CopyCircuit Challenge API integration (privacy-ethereum#965)

    * change: Port CopyCircuit to Challenge and Value APIs

    The CopyCircuit was not using the Value API for witness assignation.
    Therefore, the Challenge API has been integrated in both `configure` and
    `synthesize` steps and the `Value<F>` is the default operand.

    * change: Port Tx table to Challenge API

    Now the `TxTable` uses the `Challenges` in order to assign all the
    values on it's columns.

    * update: EVM and Super Circuits CopyTable calls

    Now that the CopyTable and CopyCircuit use the `Value` and `Challenges`
    APIs, the calls from EvmCircuit and SuperCircuit have changed and needed
    to be updated.

    * update: Cargo.lock

    * fix: Remove randomness instance from benchmarks

    * fix: Add `value` and `id` columns as SecondPhase

commit 56642ad
Author: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date:   Wed Dec 7 02:38:23 2022 +0100

    bus-mapping: fix return data offset and length bookkeeping (privacy-ethereum#951)

    * bus-mapping: fix return data offset and length bookkeeping

    This resolves a problem with the returndatacopy opcode because
    of zero return_data.[offset, length]

    * fix conflict

    Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

commit 5def16d
Author: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date:   Mon Dec 5 18:07:37 2022 +0100

    fix: keccak/plain: incremental update() broken (privacy-ethereum#964)

    * fix: keccak/plain: incremental update() broken

    Fixes: privacy-ethereum#922

    * always pad
noel2004 pushed a commit to noel2004/zkevm-circuits that referenced this pull request Nov 29, 2023
* fix tob-3-10 and tob-3-11

* Fix lint.

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Co-authored-by: Steven Gu <asongala@163.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants