Repository navigation
Conversation
xrvdg
marked this pull request as draft
September 28, 2026 10:02
xrvdg
changed the title
…BitTable with OwnedBitTable
…-fine butterflies
…lumns view and let transpose take fewer than 128 columns
xrvdg
marked this pull request as ready for review
October 5, 2026 12:06
xrvdg
commented
Oct 5, 2026
… WordColumns view and let transpose take fewer than 128 columns
… images and the transposed table
…y BitMatrix, moving the t >= 7 gate back to Shape
… BitZParams Production never transposes a table with fewer than 128 columns: the CLI builds t = 7, s = m - 7 shapes and the reference split has s >= 8. Go back to origin/main's 128x128 block kernel and drop the small-dim1 machinery (bit_transpose_blocks, rotate_bit_index, rotation_swaps and StepIter::preloaded) together with its tests. That version lives in 336e809. The bit_transpose doc comment points there and notes that it extends to dim2 < 128 if needed. BitZParams::new now rejects s < 7 with ParamsError::ColumnCountTooNarrow rather than letting the reduction panic in the transpose. It is not a protocol bound but a limit of the kernel, and restoring that version lifts it.
xrvdg
changed the title
…d let GrandProductCircuit::new take the table BitTable::as_matrix wraps the table's words as an [columns][rows] BitMatrix without moving a bit, and transposing goes only through BitMatrix::transpose. To borrow the words, BitMatrix now holds a Cow<'a, [Word]>; transpose always returns an owned BitMatrix<'static>. The crate-private transposed helper is gone, and BitMatrix computes no ilog2. GrandProductCircuit::new takes the BitTable and does as_matrix().transpose() itself, so the prover and the verifier test no longer transpose before building the circuit. BitMatrix::bit was only read by tests, so it moves into the matrix test module as the oracle there; the as_matrix test reads columns through bits() instead.
zkfriendly
requested changes
Oct 8, 2026
zkfriendly
left a comment
zkfriendly
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, the PR looks clean and correct. Left some minor comments!
BitZParams::table and VirtualParams::table cast the prover's &[F128] to &[Word] in place. On a big-endian target that cast swaps the two u64 halves of every word, so BitTable::bit, column and the transpose would read row r ^ 64 for row r, without any error. spongefish already refuses big-endian targets, but that is a dependency's policy; a crate-level compile_error! puts the requirement where the cast is and covers gkr, which depends on common.
zkfriendly
approved these changes
Oct 19, 2026
zkfriendly
left a comment
zkfriendly
left a comment
Collaborator
There was a problem hiding this comment.
Thanks, looks good. We can merge after conflicts are resolved!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preparation for the lookup table optimisation for GKR, which builds lookups from short bit strings during tree construction and the sumcheck rounds. That needs bit layouts that no protocol Shape describes.
The witness is now read as u128 words, with StepIter yielding them S bits at a time. BitTable stays the shaped view of the witness, and its transpose returns a geometry-only BitMatrix that GrandProductCircuit::new builds the leaves from.
Tables with fewer than 128 columns are now rejected by BitZParams instead of taking the prover's bit-at-a-time fallback; production shapes never have them. Commit 336e809 has a more general transpose that also takes rows shorter than a word, which might be useful later but isn't needed now.
BitTable has been split into BitTable (protocol specific) and BitMatrix (bit operations) to not have the extra restrictions when building the lookup table.
common now refuses to build for big-endian targets. BitZParams::table reads the prover's F128s in place as u128 words, and on big-endian that cast swaps the two halves of each word.