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

refactor: expose all typed backends in gnark/backend (moved from internal/) #561

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

gbotrel
Copy link
Collaborator

@gbotrel gbotrel commented Mar 13, 2023

Makes third party workflow and integration easier, for groth16 things are relatively stable now, may have couple of changes in PlonK in the future releases.

@gbotrel
Copy link
Collaborator Author

gbotrel commented Mar 13, 2023

Probably, the big ugly type switches we have in backend/groth16/groth16.go & plonk.go should go in an internal/ package; we use them for test purposes to test across backends, but most users probably don't care about testing their circuit across curves;..

Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Two no-op duplicate directory make, but otherwise good.

plonkFriDir := filepath.Join(d.RootPath, "plonkfri")
groth16Dir := filepath.Join(d.RootPath, "groth16")
plonkDir := filepath.Join(d.RootPath, "plonk")

if err := os.MkdirAll(groth16Dir, 0700); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicate make

plonkFriDir := filepath.Join(d.RootPath, "plonkfri")
groth16Dir := filepath.Join(d.RootPath, "groth16")
plonkDir := filepath.Join(d.RootPath, "plonk")

if err := os.MkdirAll(groth16Dir, 0700); err != nil {
panic(err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Next line duplicate make.

@ivokub
Copy link
Collaborator

ivokub commented Mar 14, 2023

Now when the methods are not internal anymore it would be good if the package documentation would be a bit more thorough, but I would keep it as a future work as part of a bigger documentation overhaul.

@gbotrel gbotrel merged commit 0814ece into develop Mar 14, 2023
@gbotrel gbotrel deleted the refactor/exposebackends branch March 14, 2023 13:38
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 this pull request may close these issues.

2 participants