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

[Keccak] Replace running sum gates in rho with generic gate #576

Merged
merged 1 commit into from
Jun 20, 2022
Merged

[Keccak] Replace running sum gates in rho with generic gate #576

merged 1 commit into from
Jun 20, 2022

Conversation

ChihChengLiang
Copy link
Collaborator

In the current rho design, rho has 4 gates. 2 are used for the lookup, and 2 are for the running sum of input slices and output slices. Slices must be aligned in the same rows to make both the lookup and running sum work.

In this PR, we replace the 2 running sum gates with the generic gate. Generic gate copies cells and performs running sum for slices. This approach frees us 2 gates, 2 fixed columns, and 2 advice columns, but comes with the overhead of more rows.

This PR also unlocks #569, it was blocked that q_special is used in both running sum and lookup.

@github-actions github-actions bot added the crate-keccak Issues related to the keccak workspace member label Jun 15, 2022
CPerezz
CPerezz approved these changes Jun 20, 2022
View reviewed changes
Copy link
Contributor

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

LGTM!!

Nice simplification of Rho with the GenericConfig! Looks much better.

keccak256/src/permutation/rho_checks.rs
Comment on lines -128 to +129
input_pob: Column<Fixed>,
input_acc: Column<Advice>,
output_coef: Column<Advice>,
output_pob: Column<Fixed>,
output_acc: Column<Advice>,
pub overflow_detector: Column<Advice>,
_marker: PhantomData<F>,
generic: GenericConfig<F>,
Copy link
Contributor

Choose a reason for hiding this comment

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

That is really really nice! Makes things a lot easier!

Will rebase this into the final keccak PR ASAP!

@ChihChengLiang ChihChengLiang changed the base branch from replace-gates-with-generics to main June 20, 2022 09:34
@ChihChengLiang ChihChengLiang force-pushed the replace-rho-running-sum-with-generics branch from 5364e23 to 85314dc Compare June 20, 2022 09:35
@ChihChengLiang ChihChengLiang marked this pull request as ready for review June 20, 2022 09:36
@ChihChengLiang ChihChengLiang changed the base branch from main to replace-gates-with-generics June 20, 2022 10:45
@ChihChengLiang ChihChengLiang force-pushed the replace-rho-running-sum-with-generics branch 2 times, most recently from 6b74886 to 8db428a Compare June 20, 2022 10:58
@ChihChengLiang ChihChengLiang force-pushed the replace-gates-with-generics branch from 7e32f91 to 1144738 Compare June 20, 2022 10:58
@ChihChengLiang ChihChengLiang changed the base branch from replace-gates-with-generics to main June 20, 2022 15:02
@ChihChengLiang ChihChengLiang force-pushed the replace-rho-running-sum-with-generics branch from 8db428a to 5f5ec3e Compare June 20, 2022 21:34
@ChihChengLiang ChihChengLiang merged commit bcb56ae into main Jun 20, 2022
@CPerezz CPerezz deleted the replace-rho-running-sum-with-generics branch November 30, 2022 11:59
lispc added a commit that referenced this pull request Aug 28, 2023
@kunxian-xia @roynalnaruto @lispc
Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALO…
2e42287 
…S auditors (#572)

* fix finding 3 (#575)

* Fix zellic finding 4 (#576)

* fix finding 3 (#575)

* fix finding 4

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* add range check on diffs (#586)

* Fix finding 10 (#578)

* fix finding 3 (#575)

* fix finding 10

* Fix finding 13 (#579)

* fix finding 3 (#575)

* fix finding 13

* Fix zellic finding 14 (#580)

* fix finding 3 (#575)

* fix finding 14

* Fix zellic finding 5 (#584)

* fix finding 3 (#575)

* fix finding 5

* refine comments

* fmt

* Fix finding 17 (#602)

* add q_last

* fix

* add more diff range check

* fix finding 7 (#625)

* tx_id = 1 when sm starts

* Fix finding 11 : use length for rlc in rlp table (#719)

* fix: use tag_bytes_rlc and tag_length to copy tag's bytes around

* fix lookup input for Len & RLC & GasCost fields in tx circuit

* refactor

* fix

* refactor

* fix col phase issue

* refactor bytes_rlc type

* Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612)

* lookup chain_id to RLP table

* fix finding 22 (#614)

* fix finding 21 (#613)

* fix finding 23 (#618)

* fix finding 26 (#622)

* fix finding 28 (#624)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix finding 29 (#623)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* enforce is_final is true at the last row and fix RLC related vul (#735)

* Fix finding 30  (#733)

* enforce all txs in a block are included in the tx table

* clippy

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* Fix Zellic / Kalos finding25 (#619)

* fix finding 25

* add comment

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix conflicts

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* use q_first instead

* fmt

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
lispc added a commit that referenced this pull request Aug 28, 2023
@darth-cy @lispc
@kunxian-xia @roynalnaruto
refactor: remove keccak constants from aggregator (#752)
1d190ba 
* add row counting interface for keccak

* add class level capacity calculator for keccak

* remove f capacity from core

* remove capacity calculator in aggregator util

* remove unnecessary imports

* replace max keccak round in core

* replace reference for max keccak

* remove unnecessary keccak imports and constants

* remove max keccak constant

* remove constants in hash cell parsing

* remove constant column sanity check

* add state column usage log

* adjust input bytes column

* add long column padding

* correct fmt

* fix fmt

* minor fixes

* fix

* Fix: allow skipping of L1Msg tx part 2 (calculate num_all_txs in tx circuit) (#778)

* calculate num_l1_msgs and num_l2_txs in tx circuit

* fix

* fmt and clippy

* fix: non-last tx requires next is calldata

* add NumAllTxs in block table and copy it from pi to block table

* add lookup for NumAllTxs in tx circuit

* clippy

* add block num diff check to avoid two real block have same num

* clippy

* address comments

* Fix the bugs in RLP/Tx/PI circuit which are reported by Zellic & KALOS auditors (#572)

* fix finding 3 (#575)

* Fix zellic finding 4 (#576)

* fix finding 3 (#575)

* fix finding 4

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* add range check on diffs (#586)

* Fix finding 10 (#578)

* fix finding 3 (#575)

* fix finding 10

* Fix finding 13 (#579)

* fix finding 3 (#575)

* fix finding 13

* Fix zellic finding 14 (#580)

* fix finding 3 (#575)

* fix finding 14

* Fix zellic finding 5 (#584)

* fix finding 3 (#575)

* fix finding 5

* refine comments

* fmt

* Fix finding 17 (#602)

* add q_last

* fix

* add more diff range check

* fix finding 7 (#625)

* tx_id = 1 when sm starts

* Fix finding 11 : use length for rlc in rlp table (#719)

* fix: use tag_bytes_rlc and tag_length to copy tag's bytes around

* fix lookup input for Len & RLC & GasCost fields in tx circuit

* refactor

* fix

* refactor

* fix col phase issue

* refactor bytes_rlc type

* Fix the bugs in Tx & PI circuits reported by Zellic & KALOS auditors (#612)

* lookup chain_id to RLP table

* fix finding 22 (#614)

* fix finding 21 (#613)

* fix finding 23 (#618)

* fix finding 26 (#622)

* fix finding 28 (#624)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix finding 29 (#623)

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* enforce is_final is true at the last row and fix RLC related vul (#735)

* Fix finding 30  (#733)

* enforce all txs in a block are included in the tx table

* clippy

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* Fix Zellic / Kalos finding25 (#619)

* fix finding 25

* add comment

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>

* fix conflicts

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* use q_first instead

* fmt

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>

* add pi comments

* rename preimage col idx

* add keccak rows check

* rename input bytes col finder fn

* modify keccak row env constaint

* modify keccak row env constaint

* add named constant setup vars

* modify keccak row check

* clippy advised

* add comments on chunk hash

* fmt

* avoid constant lookup table

* avoid repetitive computation of input_bytes_col_idx

---------

Co-authored-by: Zhuo Zhang <mycinbrin@gmail.com>
Co-authored-by: xkx <xiakunxian130@gmail.com>
Co-authored-by: Rohit Narurkar <rohit.narurkar@protonmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@CPerezz CPerezz CPerezz approved these changes

Assignees
No one assigned
Labels
crate-keccak Issues related to the keccak workspace member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ChihChengLiang @CPerezz