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

test-keccak-normalize-table: unit tests for the content of lookup tables #1263

Merged
merged 3 commits into from
Mar 2, 2023
Merged

test-keccak-normalize-table: unit tests for the content of lookup tables #1263

merged 3 commits into from
Mar 2, 2023

Conversation

naure
Copy link
Contributor

@naure naure commented Feb 27, 2023

Description

This PR verifies the properties of the lookup tables used in Keccak.

A port of scroll-tech#326

Issue Link

This is a good regression test before #1241

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

  • A test circuit that populates the fixed lookup tables using the functions under test.
  • Check that the table contain valid values, and nothing else.
  • Minor refactoring for testability. Make pure functions that calculate parameters, and isolate the dependency on environment variables.

How Has This Been Tested?

cargo test -p zkevm-circuits --features test --release -- keccak

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Feb 27, 2023
@ChihChengLiang ChihChengLiang self-requested a review March 1, 2023 07:00
ChihChengLiang
ChihChengLiang approved these changes Mar 1, 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! The tests improved the readability and security of the implementations.
I added some comments.

zkevm-circuits/src/keccak_circuit/util.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/util.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/util.rs Outdated
}

// Implementation of the above without environment dependency.
pub fn get_num_bits_per_lookup_impl(range: usize, log_height: usize) -> usize {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub fn get_num_bits_per_lookup_impl(range: usize, log_height: usize) -> usize {
pub(crate) fn get_num_bits_per_lookup_impl(range: usize, log_height: usize) -> usize {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

These (crate) were added after the downstream PR. They do nothing since the module is private but I’ll add it for consistency with the surroundings.

zkevm-circuits/src/keccak_circuit/util.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/util.rs Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/table.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/keccak_circuit/table.rs Outdated Show resolved Hide resolved
@naure
Copy link
Contributor Author

naure commented Mar 1, 2023

Thanks! I will add the clarifications.

@naure
Copy link
Contributor Author

naure commented Mar 1, 2023

Included suggestions. Thanks for the comprehensive review!

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Mar 2, 2023
Merged via the queue into privacy-scaling-explorations:main with commit a3ffd60 Mar 2, 2023
cameron1024 pushed a commit to cameron1024/zkevm-circuits that referenced this pull request Jun 30, 2024
@roynalnaruto @darth-cy
@noel2004 @lispc
Epic: DA-Compression (privacy-scaling-explorations#1213)
2e38fb8 
* feat: copy existing witgen logic

* Separation of `BlobDataConfig` and `BatchDataConfig` (privacy-scaling-explorations#1214)

* blob is the encoded form of batch

* fix: bytes_rlc in batch data config is constrained, unit tests OK

* Decoder "input region" (privacy-scaling-explorations#1217)

* feat: general gates, tag gates, FHD and FCS

* feat: block processing, block header, literals header

* fix: degree reduction

* feat: add block_idx to decoder's block config

* fix: block idx is the index of the block (not the byte index within block)

* feat: decode sequences section header

* fix: constraints for padded rows

* ROM Tables (LLC, MLC, MOC) and Bitstring Accumulation Table (privacy-scaling-explorations#1222)

* feat: add ROM tables for fse code to value (sequence section)

* fix: handle probability=0 case (fse)

* todo: prob=-1 case not sure how to handle

* feat: bitstring accumulation table

* print digest of compressed bytes (privacy-scaling-explorations#1226)

* fix: tests should read batches in sorted order (batch000 before batch001 and so on)

* Add bitstream decoder config (privacy-scaling-explorations#1230)

* feat: bitstream decoder config in decoder config

* add num_sequences to block region

* Bitstream Decoding and FSE Table Recovery (privacy-scaling-explorations#1234)

* feat: initial constraints for fse table recovery and bitstream decoding

* feat: add check for table_kind (LLT, MOT, MLT) in tag=fseCode

* feat: add fse table, related constraints/lookups

* fix: degree reduction

* chore: some comments

* chore: renaming

* chore: first row of bitstring table, renaming

* chore: degree assertion

* Multiple Fixes and Improvements (privacy-scaling-explorations#1239)

* fix: tag_idx == tag_len check on prev row

* impr: literals header table to use block_idx

* impr: fse table also uses (block_idx, table_kind) identifier

* fix: handle is_nil and is_nb0 separately in bitstrream decoder

* fix: add q_first=0 to fse table lookup

* Sequence Decoding and Execution (privacy-scaling-explorations#1241)

* seq decoder conf | tag=seqData | roms | more todos

* feat: sequences data section | more todos

* fix: fse table lookup and update-state

* feat: checks for nb at init-state

* feat: fse and fse sorted states table | support for prob=-1 case

* witgen (fse::reconstruct) with prob=-1 case covered

* FSE `Predefined_Mode` support for sequences section (privacy-scaling-explorations#1246)

* initial work

* tests: fse table reconstruction for default distributions

* tests: predefined MLT and MOT added

* feat: support predefined mode (compr mode) in decoder config

* `FixedTable` instead of multiple ROM tables (privacy-scaling-explorations#1247)

* fix: reduce 2 columns for rom fse order

* feat: add the fixed table module

* chore(refactor): remove rom tables and use fixed table lookup

* fix: pass fixed table by reference

* Self-review and missing constraints (misc fixes/refactoring) (privacy-scaling-explorations#1248)

* literals header table (block_idx) and bitstring table (byte_idx)

* wip: end of block (start of new block)

* wip: more constraints for last rows of fsecode and seqdata

* update: last row of tag=FseCode

* fix: need a single byte to compare byte value

* chore: add a todo

* fix: review changes

* fix: tag=FseCode last row constraint

* chore: enable while adding SeqExecTable

* `tag=FseCode` should take into account "variable bit-packing" (privacy-scaling-explorations#1251)

* fix: account for variable bit-packing in fse code section

* chore: range starts from 1 (ignore 0)

* block fields in tag=BlockHeader

* DA Compression Witness Generation and Assignments (privacy-scaling-explorations#1232)

* Remove deprecate tests

* Remove zstd encoding of literals

* Remove raw/rle blocks

* Remove block raw/rle scenarios

* Remove lstream tag

* Remove raw/rle bytes processing

* Literals witness row assignment

* Remove huffman code component from witness

* Add witness rows to sequence section

* Add debug flags

* Add sequence instruction table witness

* Finish recovering original input. sequence decoding done

* Add witness rows for sequence FSE tables

* Add witness rows for sequence header

* Remove debug flags

* fmt

* Add debug flags

* Add debug flags

* Add debug flags

* Add unit tester

* Update Fse construction

* Correct fse construction

* Correct tag length

* Temporarily recover const

* Assign literals header rows

* Assign columns

* Resolve merge compile errors

* Modify tag config assignment

* Correct block config from witgen

* Assign additional seq bitstream fields

* Correct tags

* configure gadgtes

* Add more data fields onto fse

* Correct fse assignment

* fmt

* Remove debug flags

* fmt

* Fix witness assignment error

* fmt

* Increase fse fixed capacity

* Correct bitstream column assignmnet

* Correct assignment for bitstring table

* fmt

* Correct comment

* Remove FseSymbol

* Rename FSE section

* Recover tests

* Correct block header assignment

* Correct fse decoder assignment

* fmt

* Remove constants

* Add debug flags

* Correct offset increment

* Assign padding

* Isolate gates

* Remove gates

* Remove gates

* Remove gates

* Remove gates

* Adjust gates

* Recover gates

* fix: q_enable fixed column to avoid active gates on unusable rows

* wip dbg: non-padded rows except first row

* fix: tag transition has been fixed

* fix(witgen): tag is_change=true

* wip(dbg:gate): continue same tag

* fix: compilation (u8 table load) and fhd gate works

* fix: fcs OK || unusable_rows=14

* test: fcs is OK

* fix(gate): tag=BlockHeader OK

* test(lookup): tag=BlockHeader Block_Size OK

* test(gate): processing block content OK

* fix(gate+lookup): literals header and literals header table

* fix(gate): tag=RawBytes OK

* fix(gate): tag=SequencesHeader (header decoder fixed order of bits)

* fix(fse): sorted state table (partially)

* wip(fse sorted states): gates active on unusable rows?

* fix(fse): fse table and sorted table OK

* witgen: bitstring table OK

* Correct skipped bitstream rows

* Recover gadgets

* Recover gadgets

* Recover gadgets

* Recover gates

* Recover gates

* Recover component

* Recover constraint

* Recover constraints

* Recover gates

* Recover gates

* Recover constraints

* Recover constraints

* Recover constraints

* Recover constraints

* fix(lookup): var-bit-packing and other rows of tag=FseCode

* restore gate

* fix(lookup): bitstream table lookup (start + end)

* Correct sequence data init

* Correct fse in sequence data decoding

* Recover gates

* Recover constraints

* fix(lookup): interleaved order lookup OK

* fix(gate): tag=Null OK

* Correct states, symbols, values in bitstream decoding witness

* Adjust gates

* Adjust fixed table

* Remove debug flag

* fix(fse_decoder): is_mlt and is_mot expr computation

* fse table lookup OK

* Correct seq_idx

* Recover constraints

* Recover gate

* fix: constraints updated

* chore: degree-overflow fix

* Remove is_next_null condition

* Add next_nb

* Correct tail end bit read idx

* fix: update various cases of nil

* Correct nil row index

* Correct bitstream decoding idx

* `SequenceInstructionTable` and `SequenceExecutionConfig` (privacy-scaling-explorations#1259)

* rebase to upstream

Signed-off-by: noelwei <fan@scroll.io>

* wip of addrtable

Signed-off-by: noelwei <fan@scroll.io>

* all gates

Signed-off-by: noelwei <fan@scroll.io>

* change the lookup purpose

Signed-off-by: noelwei <fan@scroll.io>

* fix according to reviews

* purge unused seqvaluetable

Signed-off-by: noelwei <fan@scroll.io>

* change zero testing to corresponding gadget
complete assignment

Signed-off-by: noelwei <fan@scroll.io>

* trivial fixing

Signed-off-by: noelwei <fan@scroll.io>

* purge the duplicated works

Signed-off-by: noelwei <fan@scroll.io>

* unit test (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* seq exec circuit (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* seq exec circuit (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* seq exec circuit (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* output region: gates and lookups (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* pass unittest for seqinst table

Signed-off-by: noelwei <fan@scroll.io>

* seq exec: complete the seq num lookup

Signed-off-by: noelwei <fan@scroll.io>

* add seq exec info in witgen

Signed-off-by: noelwei <fan@scroll.io>

* assign and unit tests (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* refactor for better assignment

Signed-off-by: noelwei <fan@scroll.io>

* assignments and unit tests (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* induce debug utilities in AddressRow

Signed-off-by: noelwei <fan@scroll.io>

* pass first unit test (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* chore: integrate seq-inst-table and seq-exec-config into decoder-config

* refactor to low degree

Signed-off-by: noelwei <fan@scroll.io>

* more unittest for seq exec (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* pass unit tests

Signed-off-by: noelwei <fan@scroll.io>

* add assign entry

Signed-off-by: noelwei <fan@scroll.io>

* update some witgens, pass decoder's unit test

Signed-off-by: noelwei <fan@scroll.io>

* integrate seq exec into decoder (WIP)

Signed-off-by: noelwei <fan@scroll.io>

* temporary disable 3 lookups and unit test pass for the rest

Signed-off-by: noelwei <fan@scroll.io>

* all of the unit test passed

Signed-off-by: noelwei <fan@scroll.io>

* trivial updates: head condition in seq exec and exported cells

Signed-off-by: noelwei <fan@scroll.io>

* clear the warnings

Signed-off-by: noelwei <fan@scroll.io>

* chore: fmt

---------

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>

* chore: most clippy issues/warnings resolved

* wip(dbg): decode encoded batch data

* fix: literals header's size format bits

* DA-Compression (Missing Pieces for single encoded batch) (privacy-scaling-explorations#1263)

* Fix repeated match byte slice:

* Add batch witgen test

* Adjust test

* fix: handle predefined table appropriately in table reconstruction

* Skip predefined fse tables:

* Add copy constraints and assigned exports

* wip dbg: expected vs got

* fix: rlc_acc and rlc, handle is_reverse approprately

* fix decoder rlc (privacy-scaling-explorations#1266)

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>

* fix: constraints/lookups OK for batch0000

* fix: literal header assignment fix, batch127 failure

* Add tag transition fix for fse

* fix: fse table handling of prob=-1 and state_idx/sym_count_acc

* fix: fse -> seq data order

* fix: handle first real symbol in prob=-1 table cases

---------

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>
Co-authored-by: Ho <noel.wei@gmail.com>

* commit from @noel2004: f181535

* Dbg (large block) (privacy-scaling-explorations#1270)

* data and test

* wip dbg: offset > currently decoded (overflow subtraction)

* chore: bitwise op table generic over op and range of lhs/rhs

* wip dbg: more blobs (witgen fail)

* fix: repeat offset usage (ll ==0) and dont include skipped states

* Account for nil row in fse

* Fix fse nil row byte skip

* Correct is_update_state for nil row in sequence data

* fix (fse state increasing, but may not be in u8) and fmt

* chore: remove println

---------

Co-authored-by: Ray Gao <qg2153@columbia.edu>

* Refactor/Cleanup Witgen code (privacy-scaling-explorations#1271)

* Remove debug flags

* Remove aux data

* fmt and add comments

* Recover test

* Remove import

* Recover test

* Recover test

* Assign padding

* Remove unused variables

* Remove initial assignment

* Add comments

* fmt

* fmt

* fmt

* fmt

* Organize comments:

* tag_value unused | refactor max_len

* more refactor and clippy

* fix: handle multi-block witgen

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>

* chore: refactor zstd config into init fn

* fix(tag=sequences_data): columns can change iff conditions are met

* Support multi-block (privacy-scaling-explorations#1273)

* add test

* detour zstd dep

Signed-off-by: noelwei <fan@scroll.io>

* tbt: test for multi-blob

* update zstd dep

Signed-off-by: noelwei <fan@scroll.io>

* chore: remove unused patch

* update zstd (scroll repo)

Signed-off-by: noelwei <fan@scroll.io>

* Correct multi-block back reference target bytes (privacy-scaling-explorations#1275)

Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>

* minor fix in assignment (block_info and seq_info)

* dbg: fix some issues

* dbg: fix bitstring table lookup for byte_idx delta

* fix: repeated offsets are carried forward into next block

* wip test: large-multi-block

* update zstd, set windowlog

Signed-off-by: noelwei <fan@scroll.io>

* fix: is_init continue if nil=1

* refactor bitstring table to separate out bitstrings of different len

---------

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: noelwei <fan@scroll.io>
Co-authored-by: Ray Gao <qg2153@columbia.edu>

* fix: handle padding in blob-data and batch-data configs (integration tests OK)

---------

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>
Co-authored-by: Ho <noel.wei@gmail.com>
Co-authored-by: noelwei <fan@scroll.io>

* fix: compile and test OK, fmt, clippy

* fix: dont completely randomise txdata (mock data)

---------

Signed-off-by: noelwei <fan@scroll.io>
Co-authored-by: Ray Gao <qg2153@columbia.edu>
Co-authored-by: Ho <noel.wei@gmail.com>
Co-authored-by: noelwei <fan@scroll.io>
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ChihChengLiang ChihChengLiang ChihChengLiang approved these changes

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

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