Skip to content

test(bn254, bls12-381): test points intentionally not on sugroups G1/2#658

Merged
yelhousni merged 5 commits into
masterfrom
test/not-in-subgroup
Mar 11, 2025
Merged

test(bn254, bls12-381): test points intentionally not on sugroups G1/2#658
yelhousni merged 5 commits into
masterfrom
test/not-in-subgroup

Conversation

@yelhousni
Copy link
Copy Markdown
Collaborator

Description

  • Generate test points that are intentionally on the G1/2 cofactor-torsions for fuzzing purposes.
  • Renamed some test methods to make more sense.

This is useful for geth fuzzer for bn254 and bls12-381. We could also generify the test for other curves but it's not a priority now IMO.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Last test in TestIsOnG2 generates points (using fuzzCofactorOfG*Jac) that pass the IsOnCurve test but not IsInSubGroup test.

How has this been benchmarked?

N/A

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@yelhousni yelhousni added type: cleanup Suggestion to clean up the code priority: P2-medium Issue priority: medium dep: geth Issues affecting geth downstream labels Mar 7, 2025
@yelhousni yelhousni added this to the v0.10.0 milestone Mar 7, 2025
@yelhousni yelhousni requested a review from gbotrel March 7, 2025 18:10
@yelhousni yelhousni self-assigned this Mar 7, 2025
@holiman
Copy link
Copy Markdown

holiman commented Mar 8, 2025

This is very useful, thanks!

Currently, as far as I can tell this is not exported?

t would be highly useful for external callers to be able to use this, to create points that are not in the right subgroups. Ideally using a custom source, so that fuzzing can produce points reproducably.

Either maybe something like GeneratePointInWrontgSubgroup( io.Reader) or something which converts a point into a "not-on-subgroup-point) somehow.

Alternatively: not expose the full "generate a point not on subgroup" but the individual smaller parts, making it easier for the outer caller to construct it by themselves.

But maybe I'm just misunderstanding, and this is already doable? I'm not a cryptographer, this stuff is hard for me :)

@yelhousni
Copy link
Copy Markdown
Collaborator Author

yelhousni commented Mar 8, 2025

This is very useful, thanks!

Currently, as far as I can tell this is not exported?

t would be highly useful for external callers to be able to use this, to create points that are not in the right subgroups. Ideally using a custom source, so that fuzzing can produce points reproducably.

Either maybe something like GeneratePointInWrontgSubgroup( io.Reader) or something which converts a point into a "not-on-subgroup-point) somehow.

Alternatively: not expose the full "generate a point not on subgroup" but the individual smaller parts, making it easier for the outer caller to construct it by themselves.

But maybe I'm just misunderstanding, and this is already doable? I'm not a cryptographer, this stuff is hard for me :)

Hey!
Would something like:

func GeneratePointInWrongSubgroup() (*G1Jac, error) {
        var f fp.Element
        var p G1Jac
        _, err := f.SetRandom()
        if err != nil {
                return &p, err
        }
        p = fuzzCofactorOfG1Jac(f)
        return &p, nil
}

be sufficient?
where SetRandom() is derived from go/src/crypto/rand.

We can export this method or similar if so.

@holiman
Copy link
Copy Markdown

holiman commented Mar 8, 2025 via email

@holiman
Copy link
Copy Markdown

holiman commented Mar 8, 2025 via email

Comment thread ecc/bls12-381/g1_test.go Outdated
Comment thread ecc/bls12-381/g2_test.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dep: geth Issues affecting geth downstream priority: P2-medium Issue priority: medium type: cleanup Suggestion to clean up the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants