Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Circuit for ErrorInvalidOpcode state#257

Closed
silathdiir wants to merge 5 commits into
scroll-stablefrom
execution/error-invalid-opcode
Closed

Circuit for ErrorInvalidOpcode state#257
silathdiir wants to merge 5 commits into
scroll-stablefrom
execution/error-invalid-opcode

Conversation

@silathdiir
Copy link
Copy Markdown

@silathdiir silathdiir commented Jan 16, 2023

Spec PR scroll-tech/zkevm-specs#54

Summary

Add ErrorInvalidOpcodeGadget for invalid opcode error.
It verifies invalid bytes in any condition of op > 0x20 && op < 0x30, op > 0x48 && op < 0x50, op > 0xa4 && op < 0xf0, and any separate bytes in
SEPARATE_INVALID_OPCODES array.

SEPARATE_INVALID_OPCODES = [
    0x0c, 0x0d, 0x0e, 0x0f,
    0x1e, 0x1f,
    0x5c, 0x5d, 0x5e, 0x5f,
    0xf6, 0xf7, 0xf8, 0xf9,
    0xfb, 0xfc,
    0xfe,
];

@silathdiir silathdiir marked this pull request as draft January 16, 2023 01:27
@silathdiir silathdiir marked this pull request as ready for review January 16, 2023 02:29
@silathdiir silathdiir marked this pull request as draft January 16, 2023 02:30
@silathdiir silathdiir marked this pull request as ready for review January 16, 2023 02:41
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch from cd431ba to 9f6bf28 Compare January 16, 2023 02:44
@silathdiir silathdiir changed the base branch from main to scroll-stable January 16, 2023 02:44
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch from 9f6bf28 to 4b74a1b Compare January 16, 2023 02:48
@silathdiir silathdiir changed the base branch from scroll-stable to main January 16, 2023 02:48
@silathdiir silathdiir marked this pull request as draft January 16, 2023 02:49
@silathdiir silathdiir marked this pull request as ready for review January 16, 2023 02:49
@silathdiir silathdiir changed the title [WIP] Circuit for ErrorInvalidOpcode state Circuit for ErrorInvalidOpcode state Jan 16, 2023
@silathdiir silathdiir requested a review from DreamWuGit January 16, 2023 03:17
@silathdiir silathdiir force-pushed the execution/error-invalid-opcode branch 2 times, most recently from 80d1891 to 237fcbc Compare January 16, 2023 07:27
@silathdiir silathdiir changed the base branch from main to scroll-stable January 16, 2023 07:27
@silathdiir silathdiir marked this pull request as draft January 16, 2023 07:27
@silathdiir silathdiir marked this pull request as ready for review January 16, 2023 07:27

fn configure(cb: &mut ConstraintBuilder<F>) -> Self {
let invalid_opcode = cb.query_cell();
cb.opcode_lookup(invalid_opcode.expr(), 1.expr());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not sure if invalid code here, op code lookup with is_code can succeed ?

@silathdiir
Copy link
Copy Markdown
Author

After synchronizing with @DreamWuGit , make the fixes as below:

  1. Renamed field invalid_opcode to opcode in ErrorInvalidOpcodeGadget.
  2. Add variable op_range_20_30, op_range_48_50 and op_range_a4_f0 to make clear that condition.

@silathdiir silathdiir requested a review from DreamWuGit January 17, 2023 15:21
ChihChengLiang and others added 4 commits January 17, 2023 23:49
* add pr template

* apply feedback from @adria0

Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com>
* try to constrain rw_counter_end_of_reversion

* fix by minus one for rw_counter_end_of_reversion

* verify rwcounter end of reversion both for root and internal call

* make all errors with rw_counter_end_of_reversion check

* apply review suggestion

* fix typing
…circuit sizes (#1064)

* fix: Include Padding tag for `CopyDataType`

This includes a new tag `Padding` for `CopyDataType` enum which enables
applying conditions when we need to pad the number of copies to be fixed
(ie: avoiding variadic size circuits).

Co-authored-by: Eduard S<eduardsanou@posteo.net>

* fix: Include `max_copy_rows` in `CircuitsParams`

We need some way to set a fixed size for the ammount of rows of copy we
can have in our `CopyCircuit` so that we don't fall into variadic
circuits & also, we can set a hard cap for the padding etc..

Co-authored-by: Eduard S <eduardsanou@posteo.net>

* fix: Introduce padding in the copy_circuit assignation

This includes checks of `CopyDataType::Padding` within the constraints
of the CopyCircuit to enable padding up to
`CircuitsParams::max_copy_rows`.
Also adds assertions to guarantee that the `max_copy_rows` value is
never surpassed during witness assignation.

Finally, it also adds explanations for the `+2` final rows that the
circuit requires due to the `Rotation(2)` usage within it.

Co-authored-by: Eduard S <eduardsanou@posteo.net>

* fix: include variadic-permutation check in CopyCircuit tests

* chore: Remove next_halo2_tag tests from copy_circuit

* chore: Fix rebase conflicts`

* chore: Fix clippy lints

* chore: Apply Han's suggestions

Co-authored-by: Eduard S <eduardsanou@posteo.net>
@silathdiir
Copy link
Copy Markdown
Author

Thanks for dream's code review. Moved to upstream privacy-ethereum#1089

@silathdiir silathdiir closed this Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants