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: move some types from common to backend (post-split)(plonk-0) #287

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

duguorong009
Copy link

Description

  • move some types/funcs of plonk from halo2_common to halo2_backend

Related issues

Changes

  • move the following types/mods from halo2_common::plonk to halo2_backend::plonk
    • Challenge*
    • Transcript

Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

LGTM.

But I want to take a second and think/consider moving the transcript to it's own crate such that it can be reused by other libs.

This is a good example on what we would need: https://github.com/dalek-cryptography/merlin
Although is fixed for keccak. Where we need to abstract the hashing algo.

What do you think @ed255 @kilic @han0110 ?

@duguorong009
Copy link
Author

LGTM.

But I want to take a second and think/consider moving the transcript to it's own crate such that it can be reused by other libs.

This is a good example on what we would need: https://github.com/dalek-cryptography/merlin Although is fixed for keccak. Where we need to abstract the hashing algo.

What do you think @ed255 @kilic @han0110 ?

From my side, I think it is good idea.
For the best, I think you should create new issue for discussion. And gets the feedback, suggestions, etc. @CPerezz

On the other hand, I believe we can merge this small PR. @CPerezz
Maybe, is it better to keep the transcript module in halo2_common crate?
Or move it to halo2_backend crate & take care of it later?

@ed255
Copy link
Member

ed255 commented Feb 27, 2024

From my side, I think it is good idea. For the best, I think you should create new issue for discussion. And gets the feedback, suggestions, etc. @CPerezz

On the other hand, I believe we can merge this small PR. @CPerezz Maybe, is it better to keep the transcript module in halo2_common crate? Or move it to halo2_backend crate & take care of it later?

I think we can move it to halo2_backend now and take care of it later. The same applies to poly: it was temporarily in halo2_common and then moved to halo2_backend via #272 and at the same time we discussed the possibility of moving it to its own crate.

@duguorong009 duguorong009 merged commit 1650a0b into main Feb 28, 2024
15 checks passed
@duguorong009 duguorong009 deleted the gr@post-split-move-plonk-0 branch February 28, 2024 02:52
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.

3 participants