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

Ecrecover soundness #1597

Conversation

roynalnaruto
Copy link
Collaborator

Description

[PR description]

Issue Link

[link issue here]

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Contents

  • [item]

Rationale

[design decisions and extended information]

How Has This Been Tested?

[explanation]


How to fill a PR description

Please give a concise description of your PR.

The target readers could be future developers, reviewers, and auditors. By reading your description, they should easily understand the changes proposed in this pull request.

MUST: Reference the issue to resolve

Single responsability

Is RECOMMENDED to create single responsibility commits, but not mandatory.

Anyway, you MUST enumerate the changes in a unitary way, e.g.

This PR contains:
- Cleanup of xxxx, yyyy
- Changed xxxx to yyyy in order to bla bla
- Added xxxx function to ...
- Refactored ....

Design choices

RECOMMENDED to:

  • What types of design choices did you face?
  • What decisions you have made?
  • Any valuable information that could help reviewers to think critically

silathdiir and others added 30 commits April 26, 2023 15:57
* Fix to handle successful run with Uint64 overflow in multiple opcodes.

* Fix lint.

* Fix failed cases.

* Fix failed tests.

* Try to trigger CI for network failure.

* Revert "Try to trigger CI for network failure."

This reverts commit c17ba9d.

* Fix lint.

* Delete `debug_assert!(VALID_BYTES < 32)` in `assign` and `valid_value` function (only leave it in `construct`).

* Delete redundant constraint `JUMPI condition must be 0 if destination is Uint64 overflow` in `jumpi`.

* Rename `within_range` to `not_overflow`.

* Replace `checked_add` and `unwrap` with just adding.

* Update some comments to retry CI.

---------

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

* Add `ChainConfig` to `TraceConfig` in external-tracer, and merge it in geth-utils.

* Add feature `shanghai` to bus-mapping and zkevm-circuits.

* Add debug outputs.

---------

Co-authored-by: Steven Gu <asongala@163.com>
* add copy circuit test

* fix rows

* fix rows

* add copy_checks

* fix copy circuit

* fix copy rows

* fix copy rows

* fix copy rows

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
* induce withdraw root reading in busmapping

* induce withdraw root reading in busmapping

* induce withdraw_root in witness block and block building

* update withdraw_root op's txid

* induce withdraw_root into end_block gadget

* integrating into supercircuit

* fmt

* clippy

* * use phase 2 copy cell
* clippy and fmt

* remove message_queue from precompiled to predeployed

* add checking for withdraw_root

* fmt

* clippy

* assign withdraw root value to pi circuit data

* update withdraw_trie_root to real word one

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
* feat: sealer api

* lint
* print rw usage in bus mapping

* more accurate ecmul (#302)
* Update `CreateGadget` to limit maximum init code size, and add init code word gas.

* Update for oog-create errors.
* Implement l1-fee in `bus-mapping`.

* Implement l1-fee `zkevm-circuits`.

* Delete feature guard (of `scroll`) for tx-fee constraint.

* Fix to assign 3 committed values with `region.word_rlc` (for RW lookups).

* Fix to calculate `tx_l1_fee` with converting word to value by `from_bytes(word.cells[..N_BYTES_U64])`.

* Add case `test_tx_l1_fee_with_right_values` and `test_tx_l1_fee_with_wrong_values` to test if `tx_l1_fee` calculation is correct.

* Add `tx_data_gas_cost` to calculate L1 fee, and sanity check for geth L1 fee.

* Fix failed cases in tx circuit.

* Fix failed PI circuit test case.

* Fix to divide `TX_L1_FEE_PRECISION` (10e9) when calculating L1 fee of transaction.

* Add a error log if `tx_fee != tx_l1_fee + tx_l2_fee`.

* Fix to return `tx_l1_fee.expr()` in `tx_l1_fee` function.

* pass supercircuit

* fmt

* overflow issue

* typo

---------

Co-authored-by: Ho Vei <noelwei@gmail.com>
* Update `testool/tests` to latest `develop` branch.

* Add `Shanghai` network.

* Set to `Ref::Any` if missing fields of `expected["indexes"]`.

* Skip test files failed to parse from json or yaml.

* Sync fixes (init code gas in begin TX and some test cases) from upstream PR-1424 (running all test cases with `shanghai` feature).

* Fix wrong stack operations of `PUSH0`.

* Fix `testool` dependency issue.

* Update `max_copy_rows` from `55000` to `65472` in `testool`.

* Fix to network `Shanghai` if shanghai feature is enabled.

* Comment out copy-circuit-test.

* Return as invalid opcodes for `TLOAD` and `TSTORE`.

* Revert "Comment out copy-circuit-test."

This reverts commit 48859e1.

* Revert "Update `max_copy_rows` from `55000` to `65472` in `testool`."

This reverts commit afcdbbb.
* expose keccak circuit APIs

* expose more fields

* [chore] fix clippy
* fix some audit issues

* fix create rw

* fix create rw

* TOB-SCROLL-5: additional assertions (#524)

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

---------

Co-authored-by: naure <naure@users.noreply.github.com>
Co-authored-by: Aurélien Nicolas <info@nau.re>
* bus-mapping impl

* circuits impl

* temp fix: TODO (move to precompile table)

* caller -> precompile -> (callee, caller)

* clippy fix

* Fix circuit `copy_table_lookup` for precompile and callee memory as bus-mapping. (#516)

* Add a simple test case.

* Revert "Add a simple test case."

This reverts commit 5efc67a.

* Fix precompile step state transition (#517)

* Fix step state transition for precompile.

* Add comments.

* Combine state step transition to previous condition of precompile.

* Delete `IdentityGadget` and replace with `BasePrecompileGadget`.

* In bus-mapping, fix to call `handle_return` with precompile exec step (not call exec step). And in zkevm-circuit, use `RestoreContextGadget` back to caller in `BasePrecompileGadget` (as STOP and RETURN).

* Restore `IdentityGadget` (and add `RestoreContextGadget` for restoring to caller context).

* Fix (part of) gas cost calculation, and move failed Precompile test case to `ErrorPrecompileFailedGadget`.

* Fix `exec_step.gas_cost = gas_cost + contract_gas_cost` for Call, and `precompile_step.gas_cost = contract_gas_cost` for Precompile.

* copy data type for each precompile

* equality for input and output rlc (identity)

* fix: read memory RW only for precompile

* fix: copy circuit degree (bin num chip needs 1 more bit)

* Fix to check precompile length for copy in bus-mapping, and check `cd_length > 0` in zkevm-circuits. (#526)

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Co-authored-by: Steven <asongala@163.com>
)

* wip

* wip 22

* wip 3

* wip 4

* wip 5

* wip 6

* wip 7

* wip 8

* wip 9

* wip 10

* wip 11

* wip 12

* wip 13

* wip 14

* wip 15

* wip 16

* wip 17

* wip 18

* wip 19

* add macros

* addressing some comments

* rewrite constraint (part 1)

* reduce condition degree

* rewrite constraint (part 2)

* rewrite constraint (part 3)

* rewrite constraint (part 4)

* rewrite constraint (part 5)

* add docs

* docs

* data table assignments

* add witness gen part 1

* add more fields for supporting pre-eip155 tx

* add witness gen part 2

* add witness gen part 3

* finish gen_sm_witness

* pass eip1559 rlp witness test

* refactor

* add pre-eip155 test case

* add unit test in rlp_circuit_fsm

* pass pre-eip155 tx unit test in rlp circuit

* fix constraint errors part 1

* fix witness gen bug

* update unit test

* fix constraint errors (part 2)

* add eip155 tx test

* make witness gen more precise

* update data table checks

* fix clippy errors

* fix clippy

* fix clippy

* fmt

* update sm checks

* reduce degree to 9

* add eip1559 test and add constraints on DecodeTagStart -> End

* finish padding

* move away RLP internal tables from table.rs

* remove old rlp circuit

* re-organize tests in rlp_circuit_fsm into its own dir

* clean

* clean

* disable tx circuit

* fmt

* clippy

* fix typo

* fix a bug in keccak circuit's min_num_rows_block

* move q_enable to RlpTable

* finish rom of l1_msg_hash

* add sm init checks

* remove old RlpTable

* fix typos and refine comments

* bug-fix: use max_length to select b to accumulate bytes' value

* update tx_data_gas_cost calc rule

* clean

* ignore tx_l1_fee unit tests in evm_circuit

* refactor tx_circuit

* add empty row

* turn on the tx_circuit

* fix clippy errors

* fmt

* fix

* assign padding_tx in rlp circuit

* clean

* pass pre-eip155 tx test

* clippy and add constraints on sig.v

* ignore null signature

* add ChainID

* dep: use scroll tech's fork of ethers-rs

* refactor

* add l1 msg tx test in rlp circuit

* pass l1 msg unit test in tx circuit

* finish

* update Cargo.lock

* fix

* refactor: move is_zero to util/

* add doc

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
* Fix `BLOCKHASH` to return `keccak(chain_id || block_height)` of the last 256 block hashes.

* Replace with a custom go-ethereum in `geth-utils` when `scroll` feature is enabled.

* Update to use `scroll-tech/gobuild` in geth-utils.
* Set empty keccak codehash

* Handle non-scroll feature flag

* Fix rebase

* clippy

* lint

* Next rws for keccak code hash

---------

Co-authored-by: Mason Liang <mason@scroll.io>
Co-authored-by: Zhuo Zhang <mycinbrin@gmail.com>
…account (#556)

* fix storage issue for accessing non-existed account

* clippy
…act (#544)

* feat: refactor pi circuit to latest (wip)

* do not include parent_hash in block table

* pi circuit tests reorg

* move constants to params

* use copy constraints to enforce pi fields' validity

* add rpi_padding related constraints

* update dummy_tx to be pre-eip155 tx

* finish rpi padding constraints

* update keccak inputs for pi circuit

* update (chain_id, hi, lo) instance into bytes

* fmt

* chain_id is u64 now

* copy chain_id from pi to tx table
* xx

* fix: skip first 12 bytes for eth address

* chore: fmt

* feat: add sig table to evm circuit

* impl SubCircuit trait for TxCircuitTester

* fill r s v in test_util sig

* lint

* lint

* clean code

* chore: refactoring witness assignment

* [fix] soundness bug in ctr int (#543)

* [fix] soundness bug in ctr int

* [fix] clippy

* move assign function to make git diff easier

* Feat: assert v is odd (#546)

* [fix] soundness bug in ctr int

* [fix] clippy

* [feat] implement v_is_odd check

* [fix] extract v from affine coord

* [chore] cargo clippy; fmt

---------

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

* address some comments

* fix instance col

* fix v permutation

* sync up changes with halo2-lib; remove lifetimers (#554)

* [fix] sync up api changes with halo2-lib: removing lifetimes

* [minor] update cargo lock

* [chore] clippy; update cargo lock

* Update sig_circuit.rs

---------

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

* lint

* add lookup to sig table in tx circuit

* add lookup to sig table in tx_circuit

* remove get_num_rows_required

* Update tx_circuit.rs

* fix sig circuit assignment

* fix: sign_data of padding tx

* fmt

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: zhenfei <zhenfei.zhang@hotmail.com>
Co-authored-by: kunxian xia <xiakunxian130@gmail.com>
@github-actions github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member crate-external-tracer Issues related to the external-tracer workspace member CI Issues related to the Continuous Integration mechanisms of the repository. crate-mock Issues related to the mock workspace member T-bench Type: benchmark improvements crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-prover Issues related to the prover workspace member crate-integration-tests Issues related to the integration-tests workspace member crate-eth-types Issues related to the eth-types workspace member crate-geth-utils Issues related to the geth-utils workspace member crate-gadgets Issues related to the gadgets workspace member labels Sep 11, 2023
roynalnaruto and others added 11 commits September 11, 2023 12:08
* fixed part of tx table must have is_calldata = false

* fix typo
…w_usage` to witness block (#933)

* Add analysis function `get_num_rows_required_no_padding` and `print_rw_usage` to witness Block.

* Add a comment.

* Rename `get_num_rows_required_no_padding` to `print_evm_circuit_row_usage`, and delete returned num row.
* fix

* update ecdsa parameters

* upgrade halo2-lib

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
@roynalnaruto
Copy link
Collaborator Author

Closing as the PR was opened by mistake

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI Issues related to the Continuous Integration mechanisms of the repository. crate-bus-mapping Issues related to the bus-mapping workspace member crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-eth-types Issues related to the eth-types workspace member crate-external-tracer Issues related to the external-tracer workspace member crate-gadgets Issues related to the gadgets workspace member crate-geth-utils Issues related to the geth-utils workspace member crate-integration-tests Issues related to the integration-tests workspace member crate-mock Issues related to the mock workspace member crate-prover Issues related to the prover workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.