Repository navigation
feat: Some extension field utilities - #901
Conversation
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
ivokub
left a comment
There was a problem hiding this comment.
Please see comments:
- lets not start panicking when we didn't before (it is not programming error, but input error, it is not worth a panic). I think here we can also fix the silent parsing error for extension.SetString (i.e. it fails silently right now)
- in code generation, lets try to avoid
f.Name == "koalabear"and instead define properties in the configuration we base the configuration on. Then all the configuration is kept in a single place.
ivokub
left a comment
There was a problem hiding this comment.
And I think the code generation we have right now is a bit messy, we have some "IsKoalaBear" in the templates as well, but we really shouldn't. It should be about the properties "SupportsAVX512()" etc which is then computed based on the width etc.
But lets keep it for another PR to clean up, I think there is an open issue for that already.
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
|
Fair comments. All addressed. Instead of a boolean for polynomial implementations did a slice of which extensions to generate polynomials for, which defaults to empty. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 454ab67. Configure here.
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
ivokub
left a comment
There was a problem hiding this comment.
Thanks for the changes. Yes, you have addressed my comments, but added Poseidon2 matrix getters, this is extends the scope of this PR significantly (and I didn't check it for now, I think @ThomasPiellard or @yelhousni would be better to review the correctness of the matrices).
Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
…t least one ext Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
Revert "build: go generate" and "feat: export external and internal matrices". They are unrelated to the extension work of this PR and continue in feat/poseidon2-matrices. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Arya Tabaie <arya.pourtabatabaie@gmail.com>
ivokub
left a comment
There was a problem hiding this comment.
Thanks for addressing all the comments! Perfect now!

This PR adds convenient I/O methods to extension fields to make GKR code generation in gnark cleaner and more uniform.
Note
Medium Risk
Touches low-level field encoding and arithmetic used by FFT/GKR paths;
Vectorremains aliased but new APIs must stay consistent withfr.Elementcanonical rules.Overview
Adds canonical serialization and integer/base-field helpers on extension types E2/E4/E6 across babybear, goldilocks, and koalabear:
BytesE*,Marshal/SetBytesCanonical,SetInt64/SetUint64/SetBigInt/BigInt, and embedded-base ops (SetElement,AddElement,SubElement,SubFromElement), with round-trip and non-canonical rejection tests.Renames the E4 slice type to
VectorE4(keepingVectoras a deprecated alias) and wires serialization/FFT call sites toBytesE4andVectorE4.VectorE6gainsSetRandom/MustSetRandom.Introduces koalabear
polynomial(denseMultiLin,Polynomial, pooling) andextensions/polynomialfor E6 multilinear/polynomial utilities (MultiLinE6,FoldFromBase,EvaluateBase,PolynomialE6,PoolE6).Reviewed by Cursor Bugbot for commit 5798e08. Bugbot is set up for automated code reviews on this repo. Configure here.