Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds increment-and-check map-to-curve gadgets (from the referenced paper) for short Weierstrass curves, with both emulated and native implementations, plus tests/benchmarks and a dependency bump to support required crypto primitives.
Changes:
- Introduces a generic emulated
maptocurvepackage supporting X-increment and Y-increment for BN254, secp256k1, and P-256 (incl. Cardano solver path). - Adds native Y-increment gadgets for Grumpkin (over BN254) and BLS12-377 (over BW6-761), including hint plumbing and basic tests/benchmarks.
- Updates
go.mod/go.sum(notablygnark-crypto) to pull in required functionality.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| std/algebra/native/maptocurve_grumpkin/maptocurve.go | Native Grumpkin Y-increment gadget (constraints: curve equation + k range). |
| std/algebra/native/maptocurve_grumpkin/hints.go | Hint to search k ∈ [0,256) and compute cube root witness. |
| std/algebra/native/maptocurve_grumpkin/maptocurve_test.go | Satisfiability + benchmark harness for the native gadget. |
| std/algebra/native/maptocurve_grumpkin/doc.go | Package documentation / compilation curve notes. |
| std/algebra/native/maptocurve_bls12377/maptocurve.go | Native BLS12-377 Y-increment gadget (over BW6-761 scalar field). |
| std/algebra/native/maptocurve_bls12377/hints.go | Hint to search k ∈ [0,256) and compute cube root witness. |
| std/algebra/native/maptocurve_bls12377/maptocurve_test.go | Satisfiability + benchmark harness for the native gadget. |
| std/algebra/native/maptocurve_bls12377/doc.go | Package documentation / compilation curve notes. |
| std/algebra/emulated/maptocurve/maptocurve.go | Generic emulated Mapper implementing X-increment and Y-increment gadgets. |
| std/algebra/emulated/maptocurve/hints.go | Emulated hints for BN254, secp256k1, and P-256 (x/y increment). |
| std/algebra/emulated/maptocurve/maptocurve_test.go | Satisfiability tests + benchmarks for emulated gadgets. |
| std/algebra/emulated/maptocurve/doc.go | Package-level documentation describing both methods and tradeoffs. |
| go.mod | Bumps deps (incl. gnark-crypto) needed for curve operations/solvers. |
| go.sum | Corresponding checksum updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 4248b17. Configure here.
| const ( | ||
| T = 256 // increment window size (8 bits) | ||
| B = 1 // curve coefficient b for BLS12-377: y² = x³ + 1 | ||
| S = 46 // 2-adicity v₂(q-1) for BLS12-377 Fp |
There was a problem hiding this comment.
Exported constant S defined but never used
Low Severity
The exported constant S = 46 is defined but never referenced anywhere in the package or in external callers. The y-increment method deliberately avoids the inverse-exclusion witness (which uses 2-adicity), so S serves no purpose here. It adds confusion since a reader might wonder whether the 2-adicity check was accidentally omitted.
Reviewed by Cursor Bugbot for commit 4248b17. Configure here.


Description
Add increment-and-check map-to-curve gadgets for short Weierstrass curves, implementing the constructions from https://eprint.iacr.org/2026/590.pdf.
Two methods are provided:
N.B. Currentlygo.modpoints to the commit Consensys/gnark-crypto@2c33f2d. We need to change that once that PR get merged.Packages
std/algebra/emulated/maptocurve/Generic emulated map-to-curve for any supported curve;
std/algebra/native/maptocurve_grumpkin/Native y-increment for Grumpkin (y² = x³ − 17), compiled over BN254.
std/algebra/native/maptocurve_bls12377/Native y-increment for BLS12-377 (y² = x³ + 1), compiled over BW6-761.
Constraint counts
Type of change
How has this been tested?
All tests verify circuit satisfiability via
test.CheckCircuit(both Groth16 and PLONK backends):TestXIncrementEmulatedBN254— x-increment on BN254TestXIncrementEmulatedSecp256k1— x-increment on secp256k1TestXIncrementEmulatedP256— x-increment on P-256TestYIncrementEmulatedBN254— y-increment on BN254TestYIncrementEmulatedSecp256k1— y-increment on secp256k1TestYIncrementEmulatedP256— y-increment on P-256 (Cardano solver)TestYIncrement(maptocurve_grumpkin) — native y-increment on GrumpkinTestYIncrement(maptocurve_bls12377) — native y-increment on BLS12-377How has this been benchmarked?
Checklist:
golangci-lintdoes not output errors locallyNote
Medium Risk
Adds new constraint-system gadgets and hint logic for mapping messages to curve points; correctness relies on hint implementations and new cube-root functionality across multiple curves/fields.
Overview
Adds new
std/algebra/emulated/maptocurvegadgets implementing x-increment and y-increment map-to-curve relations for short Weierstrass curves, backed by new solver hints that searchk∈[0,256)and validate the curve equation (with an extra 2^S-root witness for x-increment).Introduces native y-increment gadgets for
GrumpkinandBLS12-377, with associated hints, tests, and constraint-count benchmarks.Updates the generated
tinyfieldcode by addingElement.Cbrt/Element.Cube, switching internal parallel execution tognark-crypto/parallel.Execute, modernizing loop idioms, and bumps dependencies (notablygnark-cryptoandx/sync).Reviewed by Cursor Bugbot for commit 4248b17. Bugbot is set up for automated code reviews on this repo. Configure here.