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

Pi Circuit: Refactor word_rlc into word lo/hi #1383

Closed
1 task done
ed255 opened this issue May 3, 2023 · 4 comments · Fixed by #1345
Closed
1 task done

Pi Circuit: Refactor word_rlc into word lo/hi #1383

ed255 opened this issue May 3, 2023 · 4 comments · Fixed by #1345
Assignees
@hero78119
Milestone
word-hi-lo

Comments

@ed255
Copy link
Member

ed255 commented May 3, 2023 •
edited by hero78119
Loading

Specs: privacy-scaling-explorations/zkevm-specs#390

Blocked/Conflicted by:

Depends on:
@ed255 ed255 added this to the word-hi-lo milestone May 3, 2023
@ed255 ed255 mentioned this issue May 3, 2023
Closed
10 tasks
@adria0 adria0 assigned adria0 and unassigned adria0 May 17, 2023
@ChihChengLiang ChihChengLiang mentioned this issue May 18, 2023
Merged
1 task
@hero78119
Copy link
Member

For pi circuit, originally plan is to combine keccak refactor with word lo/hi. However I thinkto fix compile/uniitest on word-lo-hi branch it take some efforts to debug. So we better narrow down the error scope. My idea is to review/merge replace-rlc-by-keccak spec and circuit pr to main branch first, since it fit old design quite well, then next on word lo/hi which will make narrow down the scope of errors on word-lo-hi branch.

Below are plans

@ed255 and @ChihChengLiang what do you think about the plan :) ?

@ChihChengLiang
Copy link
Collaborator

Hi @hero78119,
Sorry, I missed your proposal before.
We've resolved the compilation issue on word-lo-hi branch with #1442 and #1446.
I think it makes sense to merge privacy-scaling-explorations/zkevm-specs#417 and #1345 before working on this issue(#1383).

@hero78119
Copy link
Member

Hi @hero78119, Sorry, I missed your proposal before. We've resolved the compilation issue on word-lo-hi branch with #1442 and #1446. I think it makes sense to merge privacy-scaling-explorations/zkevm-specs#417 and #1345 before working on this issue(#1383).

I combined pi circuit keccak + word lo hi together, to save other reviewer times. Besides unittest passed as expected :)

@ed255 ed255 linked a pull request Jun 16, 2023 that will close this issue
[word lo/hi] pi circuit replace rand/rlc by keccak hash #1345
Merged
1 task
hero78119 added a commit that referenced this issue Jun 30, 2023
@hero78119
[word lo/hi] pi circuit replace rand/rlc by keccak hash (#1345)
dd8e246 
### Description

replace rand/rlc by pure keccak hashing

### Issue Link
-
#1344
-
#1383

### Type of change

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

### Contents

- replace rpi rand/rlc logic by pure hashing 
- simplify public input into just 2 fields: digest[0:16] as `hi`, and
digest[16:32] as `lo`, while `digest = Keccak(<public data>)`
- adopt word-lo-hi

nitpick
- prefix table column annotation with table name for better debugging


### Rationale

[_design decisions and extended information_]

### How Has This Been Tested?

[_explanation_]
@ChihChengLiang
Copy link
Collaborator

closed by #1345

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

@hero78119 hero78119

Labels
None yet
Projects
zkEVM Community Edition
Status: ✅ Done
Milestone
word-hi-lo
Development

Successfully merging a pull request may close this issue.

[word lo/hi] pi circuit replace rand/rlc by keccak hash hero78119/zkevm-circuits
4 participants
@ChihChengLiang @hero78119 @adria0 @ed255