Skip to content

Conversation

@ivokub
Copy link
Collaborator

@ivokub ivokub commented Sep 10, 2025

Description

Update gnark-crypto dependency and regenerate tinyfield implementation to avoid CI failures due to dirty tree after generation.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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

Note

Updates gnark-crypto and testify, regenerates tinyfield with new exponent APIs (scalar and vector) and minor ops tweaks, adds empty-slice tests, and annotates BitReverse uses with nolint across backends/templates/tests.

  • Dependencies:
    • Bump github.com/consensys/gnark-crypto to v0.19.3-0.20251114....
    • Bump github.com/stretchr/testify to v1.11.1.
  • tinyfield regeneration:
    • Add Element.ExpInt64 and route Element.Exp to it for int64 exponents (supports negatives).
    • Add Vector.Exp(a, k int64) for batch exponentiation with negative-exponent handling.
    • Optimize MulBy3 via Double + Add.
  • Tests:
    • Add TestVectorEmptyOps ensuring zero-length vector ops don’t panic and return zero.
  • Lint/compat tweaks:
    • Annotate fft.BitReverse(...) calls with //nolint:staticcheck in backend/plonk/*/prove.go, GKR gate testing, templates, and test/unsafekzg/kzgsrs.go (backward-compatible API usage).

Written by Cursor Bugbot for commit 4de3fa7. This will update automatically on new commits. Configure here.

@ivokub ivokub requested a review from gbotrel September 10, 2025 12:27
@ivokub ivokub self-assigned this Sep 10, 2025
@ivokub ivokub marked this pull request as draft September 10, 2025 13:13
@gbotrel
Copy link
Collaborator

gbotrel commented Sep 10, 2025

was the bitReverse refactor needed because a linter complained the old one is marked Deprecated ?
Asking because I'm not a huge fan of having the package name being utils on gnark-crypto, was planning to revisit before doing a next release so we might have a tiny refactor to re-do.

@ivokub
Copy link
Collaborator Author

ivokub commented Sep 10, 2025

was the bitReverse refactor needed because a linter complained the old one is marked Deprecated ? Asking because I'm not a huge fan of having the package name being utils on gnark-crypto, was planning to revisit before doing a next release so we might have a tiny refactor to re-do.

Exactly! https://github.com/Consensys/gnark/actions/runs/17613708150/job/50041471329#step:7:29

I can also nolint tag it.

@ivokub
Copy link
Collaborator Author

ivokub commented Sep 10, 2025

And there is no big rush from my side. I wanted to merge Consensys/gnark-crypto#725, but then need #1601 on gnark side to fix some existing tests which assume gnark-crypto current behaviour. And that lead to linter errors which I wanted to fix in this PR.

But the initial issue is still imo great to merge - it allows for compatibility with other ecdsa libs.

@ivokub ivokub force-pushed the fix/smallfields-generation branch from 0e6e75c to d0b8fd0 Compare November 14, 2025 11:36
@socket-security
Copy link

socket-security bot commented Nov 14, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatedgithub.com/​consensys/​gnark-crypto@​v0.19.1 ⏵ v0.19.3-0.20251114101102-c7c3213680f876 +1100 +20100100100
Updatedgithub.com/​stretchr/​testify@​v1.10.0 ⏵ v1.11.196 +1100100100100

View full report

@ivokub ivokub marked this pull request as ready for review November 14, 2025 11:43
@ivokub
Copy link
Collaborator Author

ivokub commented Nov 14, 2025

@gbotrel - reverted using utils.BitReverse and added nolint instead.

@gbotrel
Copy link
Collaborator

gbotrel commented Nov 14, 2025

why? dependency issue ?

Copy link
Collaborator

@gbotrel gbotrel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, don't get why we can't use the non-deprecated method in gnark-crypto but it's a quick refactor anyway once we sort all the branches out

@ivokub
Copy link
Collaborator Author

ivokub commented Nov 14, 2025

The BitReverse in fft packages is denoted deprecated and recommended using BitReverse in utils package. It made linter complain and I changed to the utils package version. But you noted above that you'll probably refactor the utils package. So I reverted and added nolint instead.

And I need gnark-crypto update to fix ECDSA. But it introduces the changes to code generation which makes tests here (checking code generation) fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants