Feat: Subgroup membership testing on Bandersnatch#708
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds subgroup membership checks for twisted Edwards curves (Bandersnatch and Jubjub) and hooks them into generated code, along with corresponding tests and benchmarks.
- Introduces
IsInSubGroupmethod implementations in the edwards generator template and in the concretebandersnatchandtwistededwardspackages. - Extends
CurveParamswith constants (x1, x2, t0, t1, b) needed for the subgroup tests. - Adds
TestIsInSubGroupandBenchmarkIsInSubGroupin both the generator’s test template and the curve-specific test files.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/generator/edwards/template/tests/point.go.tmpl | Added subgroup test and benchmark blocks under wrong conditional |
| internal/generator/edwards/template/point.go.tmpl | Added IsInSubGroup template implementation |
| internal/generator/edwards/template/curve.go.tmpl | Added x1, x2, t0, t1, b to CurveParams |
| ecc/bls12-381/twistededwards/point_test.go | Added TestIsInSubGroup and BenchmarkIsInSubGroup |
| ecc/bls12-381/twistededwards/point.go | Added concrete IsInSubGroup (Jubjub) |
| ecc/bls12-381/twistededwards/curve.go | Added constants to CurveParams |
| ecc/bls12-381/bandersnatch/point_test.go | Added TestIsInSubGroup and BenchmarkIsInSubGroup |
| ecc/bls12-381/bandersnatch/point.go | Added concrete IsInSubGroup (Bandersnatch) |
| ecc/bls12-381/bandersnatch/curve.go | Added constants to CurveParams |
Comments suppressed due to low confidence (2)
ecc/bls12-381/bandersnatch/point_test.go:736
- This test only verifies points in the subgroup (multiples of the base); consider adding negative cases (e.g., non-subgroup points) to ensure
IsInSubGroup()correctly returns false.
func TestIsInSubGroup(t *testing.T) {
ecc/bls12-381/twistededwards/point.go:208
- [nitpick] The temporary variables
tmp,res, andoneare quite generic in the complex Miller-function implementation. Consider using more descriptive names (e.g.,l0v1,numPart,denPart) to improve readability and maintainability.
var (
ThomasPiellard
approved these changes
Jul 11, 2025
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
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.
Description
This PR implements a method for subgroup membership testing on Bandersnatch twisted Edwards elliptic curve. It is based on the paper https://eprint.iacr.org/2022/037.pdf. A slight difference with the paper is that we implement the method directly using twisted Edwards coordinates.
This method can be generalized for all the companion twisted Edwards curves gnark-crypto supports but we need optimized higher residue symbols. As far as I can tell this cannot be as efficient as the binary-GCD-based Legendre symbol. For example Jubjub, we need an octic residue symbol.
Bandersnatch
Bandersnatch (
B/Fr: -5x^2+y^2=1+dx^2y^2) has a cofactor 4. Given a point Q purported to be on the prime subgroup ofB, we need to compute two Tate pairings and check they both are equal to 1. The final exponentiation to(r-1)/2is replaced by a Legendre symbol (optimized through Pornin's binary GCD (#704). The Miller functions are justf1=x-x1andf2=x-x2where(x1,0)and(x2,0)are of order 2 on the birationally equivalent Weiestrass curveB'.Given
(x_e, x_e)a point onB,(x, y)is a point onB', where:To avoid inverses we use the fact that
((a/b) / r)_2 = (a * b / r)_2.So
f1andf2are simplified as:where:
Type of change
How has this been tested?
Added
TestIsInSubGroup.How has this been benchmarked?
The new method is
17xfaster than the naive multiplication by order (on my M1).Checklist:
golangci-lintdoes not output errors locally