Conversation
|
Review pass from 2026-05-07. Findings:
Verification run locally:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 913fcc2. Configure here.
The `1<<32-1` literal in MultiExpGLVWide's nbPoints guard overflows the 32-bit int on GOARCH=386, breaking the CI `go test -json` step that runs on ./ecc/bn254/... under 32-bit. The build failure emits a JSON event with empty Package, which makes gotestfmt v2.5.0 panic with "BUG: Empty package name encountered" and fails the test job. Cast through uint64 and compare against math.MaxUint32 so the constant compiles cleanly under both 32- and 64-bit int. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ivokub
left a comment
There was a problem hiding this comment.
Sending first round of review. This is mostly about the code generation part.
I think for some implementation having hand-written files is fine, but in that case they should explicilty not have the "DO NOT EDIT" header, this breaks the automatic deletion logic. And I think we can still even now have more code-generation coverage and avoid switching in the generator function on concrete curves (and rather use attribute-base generation logic). Please see the inline comments.
I will do the curve implementation and multiset hash in another round. But it would be easier if we would have more code-generated parts (for the ECC even), so that I know everything matches the templates. What was the block change why files in ecc/kb8 had to be hand-written, not generated? Imo seems quite similar to other curves?

Description
This PR adds support for the Octobear curve over the degree-8 extension of KoalaBear field and introduces a native multiset hash built on top of it, including a post-quantum vector extension. The target use case is circuit-friendly multiset hashing for computations defined over the KoalaBear field, in particular zkVM memory arguments that will later be verified inside SNARK circuits.
Concretely, this PR:
ecc.OCTOBEARcurve and theecc/octobearpackagefield/koalabearfor the base field and adds sharedE8support infield/koalabear/extensionsoctobearcurve arithmetic, scalar field, generator config, and testsecc/octobear/multiset-hash, implementing three flavors of multiset hash onoctobear:y-increment map (uint16 messages)N=23coordinates,T=128,M=2^18, uint32 messages)N=23coordinates,T=256, width-16 sponge, uint64 messages)octobearwhile keeping the rest of the repository greenThe one-point construction maps
uint16messages by searchingy = 256*m + kfork < 256, solving the resulting depressed cubic overFp^8, and accumulating mapped points onoctobear. The final implementation uses recursive Cardano with binary Lucas sequences at every extension level.The vector construction keeps
N=23independently domain-separated ECMSH accumulators. After Shor's algorithm computes discrete logarithms, a collision in the vector digest reduces to a bounded modular linear relation, i.e. a SIS-shaped problem with modulusr ≈ 2^248and dimensionN=23(23·248 = 5704bits, matching the Linea KoalaBear LtHash baseline). The linear separator is cheap but leaves a "probably" structurally regular post-Shor matrix; the Poseidon2 separator pays a sponge cost but yields a less structured matrix and is the preferred concrete derivation. Both variants preserve inverse-freeness (y_i < p/2) by construction.Type of change
How has this been tested?
The following checks pass locally:
go test ./field/koalabear/extensions ./ecc/octobear/...Additional validation performed during development:
Map(uint16)validation over all65536inputs of the one-point varianty_i < p/2) checked on sampled messages for both vector variantsHash(A ∪ B) == Hash(A) + Hash(B)componentwise) for both vector variantsHow has this been benchmarked?
Benchmarks were run locally on a MacBook Pro class machine (
darwin/arm64, Apple M5, 32GB RAM) with:Current results:
BenchmarkMap-10:25440 ns/opBenchmarkAccumulatorInsert-10:6896592 ns/op(256 inserts of the one-point variant)BenchmarkHash256-10:6802114 ns/op(one-point hash over 256 messages)BenchmarkMapLinear-10:512971 ns/op(linear vector map for one message → 23 points)BenchmarkMapPoseidon2-10:570382 ns/op(Poseidon2 vector map for one message → 23 points)BenchmarkHashLinear256-10:157928583 ns/op(linear vector hash over 256 messages)BenchmarkHashPoseidon2_256-10:160487518 ns/op(Poseidon2 vector hash over 256 messages)Checklist:
golangci-lintdoes not output errors locallyNote
Medium Risk
Medium risk due to adding a large amount of new generated cryptographic field arithmetic (including arch-specific assembly) and extending the global
ecc.ID/field mappings, which could affect downstream curve selection and serialization if misused.Overview
Adds a new curve identifier
OCTOBEARtoecc.ID, wiring it intoIDFromString,String(), andScalarField/BaseFieldlookups, and updates the top-leveleccpackage docs accordingly.Introduces a new
ecc/octobearsubtree: anfppackage that aliases the existingfield/koalabearbase field, plus a full generated scalar-field implementation infr(Element/Vector APIs, exp/sqrt/cbrt helpers, tests/benchmarks, and amd64/arm64 assembly + purego fallbacks).Updates
.golangci.ymlto ignoreoctobearinmisspell.Reviewed by Cursor Bugbot for commit 8d7eba4. Bugbot is set up for automated code reviews on this repo. Configure here.