Skip to content

Conversation

@ivokub
Copy link
Collaborator

@ivokub ivokub commented Oct 6, 2025

Description

The ICICILE backend already has the support for the curves, but we never exposed them before. I added the support without code generation for now, which can be done in the future.

I also refactored the package paths not to contaminate the "native" accelerators with different external accelerators. This now allows to provide some accelerator-specific options. I kept the existing options for backwards compatibility, but it now returns runtime error with helpful message on how to upgrade the implementation.

Tested on AWS g6e.2xlarge and works for all curves.

I also clarified documentation and added examples.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

  • Example for using the accelerated backend.

How has this been benchmarked?

Very brief benchmarks show 20-40x speedup over the native prover on CPU (only prover runtime excluding solver runtime).

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

Adds ICICLE-accelerated Groth16 backend for BLS12-377/381/BW6-761 with a new API and config, refactors BN254 integration, deprecates the old switch option, and updates docs, examples, and tests.

  • Accelerated Groth16 backend (ICICLE):
    • Add curve-specific ICICLE implementations for bls12-377, bls12-381, and bw6-761 with Prove and device-backed ProvingKey.
    • Introduce backend/accelerated/icicle/groth16 wrapper exposing Prove, Setup, DummySetup, and NewProvingKey, plus one-time device warmup.
    • New configurable options (WithDeviceID, WithBackend, WithBackendLibrary, WithProverOptions) in backend/accelerated/icicle/opts.go.
    • Refactor BN254 ICICLE code into backend/accelerated/icicle/groth16/bn254 and remove legacy in-place ICICLE hooks from native groth16.
  • Core API changes:
    • backend.WithIcicleAcceleration now returns a runtime error (deprecated); users must call the ICICLE backend explicitly.
    • Native groth16 no longer conditionally switches to ICICLE.
  • Docs & examples:
    • Update README.md GPU section; add detailed ICICLE setup/usage doc (backend/accelerated/icicle/doc.go).
    • Add examples/accelerated_gpu demonstrating end-to-end accelerated proving.
  • Tests:
    • Add cross-serialization and proving tests for ICICLE across BLS12-377, BLS12-381, BN254, and BW6-761.

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

@ivokub ivokub self-assigned this Oct 6, 2025
@ivokub ivokub added type: consolidate strengthen an existing feature type: perf priority: P2-medium Issue priority: medium labels Oct 6, 2025
cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds GPU acceleration support for Groth16 proving using the ICICLE library across three additional curves: BLS12-377, BLS12-381, and BW6-761. The implementation refactors the package structure to provide a cleaner separation between native and accelerated backends, while maintaining backward compatibility.

  • Refactors ICICLE acceleration to use dedicated package paths instead of automatic switching
  • Adds GPU acceleration support for BLS12-377, BLS12-381, and BW6-761 curves
  • Deprecates automatic ICICLE switching in favor of explicit usage of accelerated backends

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
examples/accelerated_gpu/* New examples demonstrating GPU acceleration usage
backend/accelerated/icicle/* New package structure for ICICLE-accelerated backends
backend/groth16/groth16.go Removes automatic ICICLE switching logic
backend/backend.go Deprecates WithIcicleAcceleration option
backend/groth16/bn254/icicle/* Refactored BN254 ICICLE implementation
README.md Updated documentation for new acceleration approach
Comments suppressed due to low confidence (4)

backend/accelerated/icicle/groth16/bls12-377/marshal_test.go:1

  • Incorrect curve used for compilation. Should use ecc.BLS12_377.ScalarField() instead of ecc.BLS12_381.ScalarField() for BLS12-377 test.
//go:build icicle

backend/accelerated/icicle/groth16/bls12-377/marshal_test.go:1

  • Incorrect curve ID used. Should use ecc.BLS12_377 instead of ecc.BLS12_381 for BLS12-377 test.
//go:build icicle

backend/accelerated/icicle/groth16/bls12-377/marshal_test.go:1

  • Incorrect curve scalar field used. Should use ecc.BLS12_377.ScalarField() instead of ecc.BLS12_381.ScalarField() for BLS12-377 test.
//go:build icicle

backend/accelerated/icicle/groth16/bls12-377/marshal_test.go:1

  • Incorrect curve used for compilation. Should use ecc.BLS12_377.ScalarField() instead of ecc.BLS12_381.ScalarField() for BLS12-377 test.
//go:build icicle

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ivokub ivokub marked this pull request as draft October 6, 2025 19:07
@ivokub
Copy link
Collaborator Author

ivokub commented Oct 6, 2025

Converting to draft - Cursor has found valid issues but takes time to fix.

@ivokub ivokub marked this pull request as ready for review October 23, 2025 13:49
@ivokub ivokub requested a review from Copilot October 23, 2025 13:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Enum Case Missing in String Conversion

The Backend.String() method omits a case for the METAL enum, causing it to return "unknown" instead of "METAL". This can lead to confusion in logs or during debugging.

Fix in Cursor Fix in Web

@ivokub
Copy link
Collaborator Author

ivokub commented Oct 24, 2025

Bug: Enum Case Missing in String Conversion

The Backend.String() method omits a case for the METAL enum, causing it to return "unknown" instead of "METAL". This can lead to confusion in logs or during debugging.

Fix in Cursor Fix in Web

Intentional, I just left it for placeholder right now. We don't support it currently.

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

Labels

priority: P2-medium Issue priority: medium type: consolidate strengthen an existing feature type: perf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant