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

Halo2: Updates from latest master #168

Closed
wants to merge 5 commits into from
Closed

Conversation

storojs72
Copy link
Member

@storojs72 storojs72 commented Jan 13, 2023

This PR updates halo2 branch of neptune to 0.8.1 as it is in master currently and fixes halo2 <-> master "behind" divergence.

It is required for updating halo2 branch of rust-fil-proofs eventually.

@vmx, @DrPeterVanNostrand, I guess, You are aware of master commits, so I would ask to review 9ac186e more carefully than others, as it contains merge conflicts fixes.

@vmx
Copy link
Contributor

vmx commented Jan 13, 2023

I think https://github.com/filecoin-project/neptune/tree/halo2-gpu is already doing most of this.

@storojs72
Copy link
Member Author

I think https://github.com/filecoin-project/neptune/tree/halo2-gpu is already doing most of this.

Yeah, thanks for pointing. We can pick changes from your branch into halo2 instead, if you prefer. Anyway, it looks like that having single, unified branch with Halo2 "source of trust" is beneficial

Copy link
Contributor

@DrPeterVanNostrand DrPeterVanNostrand left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@vmx vmx left a comment

Choose a reason for hiding this comment

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

I'd prefer if we just force-push the halo2-gpu branch as halo2 as it has a linear history.

@storojs72
Copy link
Member Author

storojs72 commented Jan 17, 2023

@vmx , I don't mind using halo2-gpu. Actually, I didn't know that it exists with necessary changes until you mentioned. What if we use it but rename it to halo2 (and remove existing halo2)? Commit history will be still preserved, but there will be no confusion about single source of Halo2 work along the repositories. So, we can close this PR, remove halo2 branch, rename halo-gpu into halo2 and reference it in filecoin-project/rust-fil-proofs#1656. WDYT?

@vmx
Copy link
Contributor

vmx commented Jan 18, 2023

@storojs72 I've force-pushed halo2-gpu as halo2 (and pushed the former halo2 as halo2-jake in case he still needs it. Force-pushing halo2 branches isn't such a big deal as only our team is using it atm. I think this PR can be closed and the branch can be deleted once you've updated filecoin-project/rust-fil-proofs#1656.

@storojs72
Copy link
Member Author

storojs72 commented Jan 18, 2023

@vmx , thank you. I'm picking edited halo2 branch of neptune and testing filecoin-project/rust-fil-proofs#1656 right now.

@storojs72
Copy link
Member Author

filecoin-project/rust-fil-proofs#1656 is working with updated halo2 branch, which seems to be the relevant source of Halo2 stuff (@vmx thank you!). So this PR is not needed anymore.

@storojs72 storojs72 closed this Jan 18, 2023
@storojs72 storojs72 deleted the halo2-pick-master-changes branch January 18, 2023 15:31
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