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

sync main branch to word-lo-hi branch #1470

Merged
merged 27 commits into from
Jun 13, 2023
Merged

sync main branch to word-lo-hi branch #1470

merged 27 commits into from
Jun 13, 2023

Conversation

hero78119
Copy link
Member

@hero78119 hero78119 commented Jun 12, 2023 •
edited
Loading

Description

merge main to word-lo-hi branch

Issue Link

N/A

File conflicts list, other file merge from main successfully
Conflict file path:
        deleted: keccak256/src/gate_helpers.rs
        both modified:   zkevm-circuits/src/copy_circuit.rs
        both modified:   zkevm-circuits/src/evm_circuit/execution.rs
        both modified:   zkevm-circuits/src/evm_circuit/execution/balance.rs
        both modified:   zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
        both modified:   zkevm-circuits/src/evm_circuit/execution/end_tx.rs
        both modified:   zkevm-circuits/src/evm_circuit/execution/logs.rs
        both modified:   zkevm-circuits/src/evm_circuit/step.rs
        both modified:   zkevm-circuits/src/evm_circuit/util.rs
        both modified:   zkevm-circuits/src/evm_circuit/util/common_gadget.rs
        both modified:   zkevm-circuits/src/state_circuit.rs
        both modified:   zkevm-circuits/src/state_circuit/constraint_builder.rs
        both modified:   zkevm-circuits/src/table.rs
        both modified:   zkevm-circuits/src/witness/rw.rs

After fixing conflict, file change list

        modified:   zkevm-circuits/src/evm_circuit/execution/create.rs
        modified:   zkevm-circuits/src/evm_circuit/util/constraint_builder.rs
        modified:   zkevm-circuits/src/table/block_table.rs
        modified:   zkevm-circuits/src/table/bytecode_table.rs
        modified:   zkevm-circuits/src/table/copy_table.rs
        modified:   zkevm-circuits/src/table/keccak_table.rs
        modified:   zkevm-circuits/src/table/mpt_table.rs
        modified:   zkevm-circuits/src/table/rw_table.rs
        modified:   zkevm-circuits/src/table/tx_table.rs
        modified:   zkevm-circuits/src/util/word.rs

ChihChengLiang and others added 27 commits May 10, 2023 15:11
@ChihChengLiang
Simplify bytecode circuit assignment (#1397)
fbbe1bd 
### Description

We simplify the assignment logic in the bytecode circuit by reducing the
number of function arguments. The reduction is achieved by

1. Construct IsZero Chips early and make them a part of the circuit
struct so that they don't stay with the function arguments.
2. Create a new row struct that carries all the relevant data.

### Issue Link

This would be part of #1391. 

### Type of change

- [x] 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

### Rationale

- I avoid using `BytecodeRow` in the `bytecode_unroller` since they are
part of the future simplification plan.


### How Has This Been Tested?

Sent to the CI and pray.
@adria0
fix(testool): fix coinbase, solves #1160 (#1400)
a807754 
### Description

When running `testool --oneliner`, set the coinbase to `MOCK_COINBASE`
and top up with 1 wei to register it into the state db.

### Issue Link

#1160 

### Type of change

- [x] 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

Co-authored-by: adria0 <adria0@nowhere>
@ed255
Use halo2 circuit-params feature (#1399)
14bd7c9 
Refactor SuperCircuit and PiCircuit to use circuit parameters instead of
associated constants.

Resolve
#1393

### Type of change

- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected)

### Contents

Using the `circuit-params` feature recently introduced in our fork of
halo2 allows simplifying the code for circuits that require runtime
parameters at configuration time. It removes the annoying associated
const of the circuit types, and also allows having a single circuit type
(no need for an extra circuit test type).
@ChihChengLiang @ed255
Replace Witness.rs's ExecStep with CIP's (#1403)
0e1d230 
### Description

- We replace ExecStep struct in witness.rs with the one in the Circuit
Input Builder.
- We add some convenient methods in CIP's ExecStep so that we don't
create extra struct fields.
- Add `get_rws` method to `witness::Block` to streamline the logic of
getting Read-Write records.

### Issue Link

Part of #1391

### Type of change

- [x] 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

### Rationale

- Notes for reviewers, one place that might be error-prone would be the
rw_indices. After the cleanup in this PR, we can replace the manual
counting of rw indices with something like
`evm_circuit::utils::StepRws`.

---------

Co-authored-by: Eduard S. <eduardsanou@posteo.net>
@ChihChengLiang
Set keccak logs to debug level (#1405)
5dba7f7 
### Description

Keccak circuit logs messages that make CI logs hard to reason.

We can set it from "INFO" level to "DEBUG" level to suppress the
messages in the CI output but still keep it for develoepers.



https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/4955284960/jobs/8864549606#step:13:50

<details><summary>Details</summary>
<p>

```
test mock_prover::serial_test_exp_circuit_multiple_transfers_0 ... ok
[2023-05-12T05:00:22Z INFO  integration_tests::integration_test_circuits] test Keccak circuit, block: #1 - Transfer 0
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post absorb:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: 2
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 7
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post padding:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: 1
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 10
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post theta:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: 14
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 38
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post rho/pi:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: 53
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 191
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post chi:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: [52](https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/4955284960/jobs/8864549606#step:13:53)
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 195
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] - Post squeeze:
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Lookups: 1
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Columns: 198
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Degree: 4
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Minimum rows: [61](https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/4955284960/jobs/8864549606#step:13:62)
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Total Lookups: 123
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] Total Columns: 198
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] num unused cells: 229
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size absorb: 4
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size theta: 2
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size theta c: 2
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size theta t: 3
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size rho/pi: 3
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] part_size chi base: 3
[2023-05-12T05:00:22Z INFO  zkevm_circuits::keccak_circuit] uniform part sizes: [2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2]
```

</p>
</details> 


### Issue Link


### Type of change

- [x] 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
@ChihChengLiang
Move stats to bin (#1407)
ead0b3d 
### Description

We have 4 utility commands that show us statistics of different
circuits. Currently, these features were compiled with the tests,
causing dependency and feature flags management troubles. The commands
also cause confusion with the ignored test flags.

We can enjoy many benefits if we keep these commands in binary
executables.

- We can compile them on demand. We remove compilation overheads for the
main components and tests.
- Stats commands are confined in the `bin` directory, so we don't have
confusing testing code.

### Issue Link

It was #1246. After exploring, I found room for improving the status
quo.

### Type of change

- [x] 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

- Move the underlying logics of `make stats_state_circuit`, `make
stats_evm_circuit`, `make stats_copy_circuit`, and `make
evm_exec_steps_occupancy` to `bin` directory.
- Add a feature flag, `stats` for these specific tools.
- Remove `DisplayTable` and replace all its use with `cli-table`'s
feature. This has the benefit that the line of codes in the codebase
reflects more on the core zkevm functionality. After we used
`cli-table`, the table also rendered differently, see images below.
- Move `cli-table` from `dev-dependency` to `dependency`, but as an
optional dependency.
- Remove dead dependency `subtle` and `criterion`

After:
<img width="508" alt="Screenshot 2023-05-14 at 3 26 43 PM"
src="https://github.com/privacy-scaling-explorations/zkevm-circuits/assets/3391420/a4d32ce1-3559-4057-852e-b9b358f18553">
Before:
<img width="471" alt="Screenshot 2023-05-14 at 3 27 53 PM"
src="https://github.com/privacy-scaling-explorations/zkevm-circuits/assets/3391420/a073763d-71e1-4c15-876b-7e4252970875">


### Rationale

- Some undesirable results were introduced.
- We left the Instrument struct unchanged. The `Instrument` is buried
deep in the ExecutionConfig. We intend to deal with it in future PRs to
keep the diff of this one slim.
- We changed some visibilities of structs and functions in evm circuits
from `pub(crate)` to `pub`

### How Has This Been Tested?

Manual run `make stats_state_circuit`, `make stats_evm_circuit`, `make
stats_copy_circuit`, and `make evm_exec_steps_occupancy` and see if they
work.
@ChihChengLiang
Rm dead code except of zkevm circuits (#1408)
ef13927 
### Description

We remove `#![allow(dead_code)]` everywhere in the codebases other than
`zkevm-circuits`.

This PR should be merged after #1407 

### Issue Link

No issue yet

### Type of change

- [x] 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

- Removed `#![allow(dead_code)]` everywhere other than `zkevm-circuits`.
- Make test utility functions like `Block::get_test_degree` and
`Sha3CodeGen` public functions so that they don't cause `cargo build -p
zkevm-circuits` to fail. This might come with the cost of extra
compilation time.
- Refactor `gen_sha3_code` into `Sha3CodeGen` so that we don't have to
import `MemoryKind` to use it.
@han0110 @ChihChengLiang
Add rustdocflags config for macos in Makefile (#1412)
255a9bc 
### Description

Add `rustdocflags` override in `Makefile` when it's macos to make doc
test work.

Note that `config.toml` doesn't support to specify `rustdocflags` under
`[target.'cfg(target_os = "macos")']` (see
rust-lang/cargo#11323) so we have to use command
line override to add it manually.

### Issue Link

Resolves
#1410

### Type of change

- [x] 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

### How Has This Been Tested?

On macos, run `make test_doc`.

Co-authored-by: Chih Cheng Liang <chihchengliang@gmail.com>
@ChihChengLiang
Retype Gas limit and nonce to u64 (#1409)
d21952d 
### Description

- Gas limit from Word to u64
- Nonce from Word to u64

### Issue Link

Fix #1377

### Type of change

- [x] 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

- change geth_type Account's nonce and gas_limit to eth_core::U64. We
can not use simple u64 because we need to serialize nonce and gas_limit
to hex value with 0x prefix for Geth RPC to use.
- simple u64 is used in rest of the places.

### Rationale
@SuccinctPaul @ChihChengLiang
Split table into files (#1402)
438bac5 
### Description
Split table into files.

* prior
This is one the new pr of #1061

---------

Co-authored-by: Chih Cheng Liang <chihchengliang@gmail.com>
@ChihChengLiang
Remove RwTableTag (#1406)
69cb8a9 
### Description

`RwTableTag` can be fully replaced by `Target`.

### Issue Link

Part of #1391 

### Type of change

- [x] 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

We add EnumIter, Hash, Expr to Target to let it serve different needs.

Note to reviewers: the `impl From<Target> for usize` is required by
BinaryNumberConfig.
@ed255
Fix infinite loop (#1421)
70d7da4 
### Description

Fix an infinite loop from a `From` implementation calling itself.

### Issue Link

Resolve
#1420
Related
#1417

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

### Contents

@ChihChengLiang found that this issue was introduced in
#1406
and by debugging on the debug build I found the infinite loop (which
caused the stack to overflow which terminated the program with a
segfault). The segfault is not observed in the release build, I guess
the compiler optimizes the recursive call into an infinite loop, so the
stack is never exhausted.
@silathdiir
Bug: fix to truncate offset from Word to Uint64 when length is zero f…
43c67c9 
…or some opcodes (#1415)

### Description

Reference go-ethereum function
[calcMemSize64WithUint](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/common.go#L38),
ignore to check Uint64 overflow for memory offset if length is zero.

it is also called by
[calcMemSize64](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/common.go#LL31C9-L31C30).
And both are used for opcodes in
[memory_table.go](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/memory_table.go#L20)
as `memorySize` in
[jump_table.go](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/jump_table.go#L387).

For Call opcodes, in offset and size are truncated to Uint64 as
[opCall](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/instructions.go#LL672C60-L672C60).

### Issue Link

Related issue
#1301

Original local PR scroll-tech#393

### Type of change

- [X] Bug fix (non-breaking change which fixes an issue)

### How Has This Been Tested?

1. Fix `testool` case `randomStatetest85_d0_g0_v0`.
2. Add test cases of overflow offset and zero length for related
opcodes.
@ed255
Fix EXTCODECOPY with empty account (#1429)
626f4b9 
### Description

We treat empty accounts by storing their code_hash in the RwTable as 0.
EXTCODECOPY was obtaining the bytecode length by querying the bytecode
table with code_hash=0 on existing accounts, but that entry should be
invalid (there's no bytecode with code_hash=0). Skip the bytecode table
length lookup when code_hash=0.

I've also reintroduced the `Block::debug_print_txs_steps_rw_ops`
function, updated to use the new `Block::get_rws` API. This function is
not used in the code, but it's very convenient to call it when
debugging.

### Issue Link

Resolve
#1190

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
@ed255 @roynalnaruto @KimiWu123
[CREATE Part A (updated)] Modification to Copy Circuit (#1419)
dbe67c8 
### Description

NOTE: This is an updated version of
#1356

This is Part A of a 3 part pull request to add support for
`CREATE`/`CREATE2` opcodes.

Part A:
#1356
Part B:
#1357
Part C:
#1358

### Issue Link

#1130 

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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
 
As part of the bigger additions needed for the `CREATE`/`CREATE2`
opcodes' gadget, this PR adds support for the copy circuit to "always"
have a value accumulator field `value_acc`.

### Rationale

We need a value accumulator (of the random linear combination) in order
to get the `RLC(bytes)` for the bytes copied from `Memory` to `Bytecode`
(specifically the init code). This RLC is later used to do a lookup to
the Keccak table to check the value of `keccak256(init_code)`.

### How Has This Been Tested?

The existing tests for copy circuit pass for the updated constraints on
the copy circuit.

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: KimiWu <leonhartwu@gmail.com>
@KimiWu123 @roynalnaruto
Feat/CREATE Part B - Error cases that will be handled within opcode g…
5e1d35e 
…adgets (#1425)

### Description

**NOTE: This is an updated version of
#1357

This PR is actually based on top of #1419

### Issue Link

#1130 

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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
Error types for insufficient balance and nonce overflow. These are
supporting changes required for the CREATE/CREATE2 opcodes' gadget.

### Rationale
The above errors will be handled within the CREATE/CREATE2 opcodes'
gadget. In case of insufficient balance, it is also handled in the
CallOp gadget for call related opcodes.

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
@ChihChengLiang
Remove release drafter (#1436)
96d2e3c 
### Description

The release drafter was used to create a list of changes in the release
note. As Github now provides this functionality by native, there's no
need to maintain this CI script.

### Issue Link

[_link issue here_]

### Type of change

- [x] 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
@ChihChengLiang
Remove keccak256 crate and migrate keccak plain to eth-types (#1440)
73b2aeb 
### Description

- We remove the keccak256 crate.
- We migrate the keccak256 plain implementation to the eth-types crate.

As we migrate keccak circuit to the zkevm-crate, most of the functions
left in keccak256 crate are unused except of the plain keccak hash
implementation.

Most of the crates in the codebase depend on keccak256 crate because
they use the plain keccak hash. The plain keccak hash is only 300 lines
big. It would be beneficial to keep a plain implementation for people to
audit the optimized keccak circuit.

### Type of change

Bug fix (non-breaking change which fixes an issue)
@rrtoledo @adria0
Replace Gas, GasCost and ProgramCounter by u64. (#1448)
092ab1f 
### Description
This is a very small MRs removing some overhead. It replaces the
types/structures for Gas, GasCost and ProgramCounter by u64.

### Issue Link
Issue #1404 

### Type of change

- [x] 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

### Rationale
Keeping the existing code but removing the structures.

### How Has This Been Tested?
No tests were added.

---------

Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com>
@ed255
Add PiCircuit to integration tests (#1447)
7715ef2 
### Description

This is a very small PR. While investigating
#1439
I noticed we don't have an integration test for the Public Inputs
Circuit, so I just added it.

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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
@silathdiir @lispc
feat: add shanghai feature and make CI pass with Shanghai geth trac…
de9f845 
…es (#1424)

### Description

1. Add Shanghai related fields to chain config in geth-utils.

2. EIP-3651 (Warm COINBASE): add a new access-list write for coinbase to
Begin TX.

3. Part of EIP-3860 (Limit and meter initcode): only add gas cost of
init code to Begin TX (missing gas cost changes in Create and OOG
Create).

### Issue Link

Issue
#1362

Local related PRs:
scroll-tech#497
scroll-tech#500
scroll-tech#507

Reference previous PR:
#1361

### Type of change

- [x] Breaking change (fix or feature that would cause existing
functionality to not work as expected)

### How Has This Been Tested?

Unit test cases of CI could pass with Shanghai geth traces.

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
@naure @han0110
Fix RLC is_lt_128 (#1449)
7b77f9c 
### Description

In the RLC gadget, the indicator `is_lt_128` must be equivalent to
`value < 128`.

However, currently only the `true` case is handled. This PR handles both
`true` and `false` cases.

### Issue Link

Scroll internal.

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

---------

Co-authored-by: Aurélien Nicolas <info@nau.re>
Co-authored-by: Han <tinghan0110@gmail.com>
@enricobottazzi
fix: modify assign function of LtChip to take Value<F> input instea…
06cce9f 
…d of `F` (#1456)

### Description

The PR modifies the assign function of the LtChip to take `Value<F>`
input instead of `F`. According to this discussion [Halo2 -
Discord](https://discord.com/channels/995022112811135148/1113747239534346250/1113854493646397621)
with @therealyingtong and @ed255, it emerged how the current design of
the chip makes its usage prone to bugs.

For example, this is how it is currently used in summa-solvency =>
https://github.com/summa-dev/summa-solvency/blob/a7d0c1f51c7e146bac2686c432c945ff6f3f063f/src/chips/merkle_sum_tree.rs#L338-L354

In order to extract `F` from an assigned cell, I need to create an if
conditional that performs the `assign` function only if a witness value
is Some. This is a problem as the assignment function won't be called
during keygen.

### Type of change

- [x] 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

### How Has This Been Tested?

`cargo test less_than`
Change ExecError related functions to be associated-fns (#1450)
9620822 
### Description

This PR attempts to associate the function `get_step_reported_error` to
`ExecError`.

### Issue Link

Resolves #1181 

### Type of change

- [x ] 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

### Rationale
We wanted to move the function `get_step_reported_error` to an `imp
ExecError`, I also moved the part related to `OogError` to its own
associated function `From<OpCodeId> for OogError`. I changed an if/else
block into a match expecting to handle more errors for CREATE/2.

Personally I would update the function `get_step_reported_error` to take
as input an `GethExecStep` and in that case directly implement it as a
`From<GethExecStep> for ExecError` but the return type should be changed
to `Option<ExecError>` since the step may not have an error (step.error
is optional).

### How Has This Been Tested?
No tests were added as no new functionalities were added.
@KimiWu123 @roynalnaruto
Feat/CREATE Part C - bus-mapping and gadget implementation (#1430)
9ab662b 
### Description

NOTE: This is an updated version of
#1358

This PR is actually based on top of
#1425

### Issue Link

#1130 

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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

Bus mapping implementation for CREATE/CREATE2
EVM circuit's gadget for CREATE/CREATE2

### How Has This Been Tested?
Tests can be found here:
https://github.com/scroll-tech/zkevm-circuits/blob/2d2bfc6ccf179ade1a8d063f9586b93e5283a557/zkevm-circuits/src/evm_circuit/execution/create.rs#L678

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Re-enable end_tx tests that depended on SSTORE (#1459)
224e98f 
### Description

Enable tests to check if gas refund was implemented correctly (c.f. [EIP
3529](https://eips.ethereum.org/EIPS/eip-3529))

### Issue Link

Issue #1111 

### Type of change

- [x] 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

- Uncommenting and updating tests

### Rationale

Using the commented values and common values for account creation.

### How Has This Been Tested?
@hero78119
merge main
ce0d997
@hero78119 hero78119 changed the title Feat/word lo hi merge main sync main branch to word-lo-hi branch Jun 12, 2023
ed255
ed255 approved these changes Jun 13, 2023
View reviewed changes
Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! Although this PR has many diff lines, most of them are from commits already in main. This is how I reviewed this PR:

# With all branches up to date

# Create a local branch where I apply the merge of main, comiting all the conflicts without manual resolution
git checkout word-lo-hi
git checkout -b feat/word-lo-hi-merge-main-edu
git merge main
git add .
git commit -m "Merge with conflicts"

# Checkout the PR branch and diff it against my local branch.  This diff is much more smaller, and represents the contents that really need to be reviewed.
git checkout origin/feat/word-lo-hi-merge-main
git diff feat/word-lo-hi-merge-main-edu

ChihChengLiang
ChihChengLiang approved these changes Jun 13, 2023
View reviewed changes
Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

LGTM. I followed Edu's helpful guide and found the diffs all make sense.

@hero78119 hero78119 merged commit fab8e2e into word-lo-hi Jun 13, 2023
@hero78119 hero78119 deleted the feat/word-lo-hi-merge-main branch June 13, 2023 14:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ChihChengLiang ChihChengLiang ChihChengLiang approved these changes

@ed255 ed255 ed255 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.