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

[word lo/hi] optimisation and improvements. #1487

Closed
6 of 10 tasks
hero78119 opened this issue Jun 20, 2023 · 2 comments · Fixed by #1427 or #1491
Closed
6 of 10 tasks

[word lo/hi] optimisation and improvements. #1487

hero78119 opened this issue Jun 20, 2023 · 2 comments · Fixed by #1427 or #1491
Assignees
@hero78119
Labels
T-feature Type: new features

Comments

@hero78119
Copy link
Member

hero78119 commented Jun 20, 2023 •
edited
Loading

Describe the feature you would like

Aimed as super issue to record some further optimisation/housekeeping tasks related to word-lo-hi refactor.

cc @ed255 @ChihChengLiang @adria0 @KimiWu123

Additional context

No response
@hero78119 hero78119 added the T-feature Type: new features label Jun 20, 2023
This was referenced Jun 20, 2023
Merged
Closed
Merged
@ChihChengLiang ChihChengLiang linked a pull request Jun 21, 2023 that will close this issue
[word lo/hi] support u16 range lookup #1427
Merged
4 tasks
hero78119 added a commit that referenced this issue Jun 21, 2023
@hero78119
[word lo/hi] add ci subcircuit unit test and fix address assignment (#…
cb6c17c 
…1488)

### Description

Add back 
- evm_circuit 
- keccak_circuit 
- exp_circuit 
- pi_circuit 
- state_circuit

unittest to ci

Other subcircuit `tx_circuit/copy_circuit/bytecode_circuit/root_circuit`
still failed for now, will be fixed in another pr

### Issue Link


[N/A](#1487)

### Type of change

- [x] New feature (non-breaking change which adds functionality)

### Contents

- Fix unittest assertion
[failed](https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/word-lo-hi/zkevm-circuits/src/util/int_decomposition.rs#L39-L43)
due to sanity check on address H160 retrieved from U256. We need to
clean up MSB 12 bytes to 0
- add subcircuit light tests on CI in whitelist based
@ChihChengLiang ChihChengLiang mentioned this issue Jun 22, 2023
Closed
5 tasks
@hero78119 hero78119 mentioned this issue Jun 22, 2023
Merged
1 task
@ChihChengLiang ChihChengLiang linked a pull request Jun 22, 2023 that will close this issue
[word lo/hi] cleanup evm_circuit workaround #1491
Merged
1 task
hero78119 added a commit that referenced this issue Jun 29, 2023
@hero78119
[word lo/hi] cleanup evm_circuit workaround (#1491)
e1350c8 
### Description

Clean up workaround introduced in
#1435

### Issue Link


#1487

### Type of change

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

### Contents

- it touch many files just under evm_circuit. Will not cause conflict
with other word-lo-hi pr
- all just cleanup/renaming without any logic change
@ChihChengLiang
Copy link
Collaborator

It looks like this task is done.
The only task on the list that is open is #1479. We should do it after merging lo-hi to main. @hero78119 Feel free to reopen if you have something to add.

@hero78119
Copy link
Member Author

One of them should be resolved before merge main: rename all foo_word to foo

For others can be lower priority and AFTER word-lo-hi merge back to main.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@hero78119 hero78119

Labels
T-feature Type: new features
Projects
zkEVM Community Edition
Status: ✅ Done
Milestone
No milestone
2 participants
@ChihChengLiang @hero78119