Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Icicle integration is out of date #1166

Open
wwared opened this issue Jun 12, 2024 · 2 comments · May be fixed by #1318
Open

bug: Icicle integration is out of date #1166

wwared opened this issue Jun 12, 2024 · 2 comments · May be fixed by #1318

Comments

@wwared
Copy link
Contributor

wwared commented Jun 12, 2024

The Icicle integration for the Groth16/BN254 backend is out of date and does not currently compile, and the version of icicle that gnark depends on is old.

Description

Trying to build gnark with -tags=icicle fails due to accumulated changes in the rest of the gnark internal API that were not reflected in the Icicle backend. See the specific compile errors mentioned in "Actual Behavior" below.

Additionally, the icicle dependency used by gnark is currently commit ingonyama-zk/icicle@97f0079 (specified in go.mod here, indirectly from the iciclegnark dependency), which means that the current instructions specified here do not work for gnark: the current version of the icicle Go bindings create static libraries like libingo_curve_bn254.a and libingo_field_bn254.a while the older version that gnark relies on creates a shared library libbn254.so and the build process is documented here instead. It would be very nice if the gnark integration could be updated to support the latest icicle version as well, besides simply fixing the compile errors mentioned in this issue, though that probably requires Ingonyama to make the respective updates to the iciclegnark package.

Expected Behavior

The code should compile without errors and generated proofs should verify correctly.

Actual Behavior

Building with -tags=icicle gives the following compile errors:

# github.com/consensys/gnark/backend/groth16/bn254/icicle [github.com/consensys/gnark.test]
backend/groth16/bn254/icicle/icicle.go:44:30: cannot use pk.Domain.CosetTableInv (value of type func() ([]"github.com/consensys/gnark-crypto/ecc/bn254/fr".Element, error)) as []"github.com/consensys/gnark-crypto/ecc/bn254/fr".Element value in argument to iciclegnark.CopyToDevice
backend/groth16/bn254/icicle/icicle.go:47:30: cannot use pk.Domain.CosetTable (value of type func() ([]"github.com/consensys/gnark-crypto/ecc/bn254/fr".Element, error)) as []"github.com/consensys/gnark-crypto/ecc/bn254/fr".Element value in argument to iciclegnark.CopyToDevice
backend/groth16/bn254/icicle/icicle.go:160:73: commitmentInfo[i].HintID undefined (type constraint.Groth16Commitment has no field or method HintID)

Possible Fix

I tried fixing this locally in this commit wwared@186e9de

However, even though this enabled me to compile gnark with Icicle support (linking against the older version at commit ingonyama-zk/icicle@97f0079), proofs generated with this would fail to verify with panic: pairing doesn't match, so this attempted fix is either wrong or incomplete.

Steps to Reproduce

  1. Run go test -tags=icicle on the repository root of current master, or attempt to build a gnark-using program by passing -tags=icicle to the build/run command line.

Context

I am trying to generate a proof using GPU acceleration for a circuit defined on the latest gnark. It's not easy for me to revert to an older gnark version.

Your Environment

  • gnark version used: HEAD@master (db299ce as of writing)
  • gnark-crypto version used: Whichever one is used by default (v0.12.2-0.20240504013751-564b6f724c3b according to go.mod)
  • go version: 1.22.3
  • Operating System and version: Linux 6.6.30
@ivokub
Copy link
Collaborator

ivokub commented Jul 3, 2024

Acknowledged. I cannot give deadlines when we fix it though, it is a bit of work.

@p4u
Copy link

p4u commented Oct 31, 2024

It would be nice to have icicle support back.

@jeremyfelder jeremyfelder linked a pull request Nov 18, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants