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

fix: re-enable and update risc0, clean up runner #32

Closed
wants to merge 6 commits into from

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Sep 26, 2023

Closes #30

I started to fix this since I wanted to benchmark some alternate zkp languages for my own curiosity, and it ended up being a bit of a rabbit hole to fix this. I'll try to summarize the changes:

  • Update toolchain to stable
    • Can't compile bytemuck (risc0 dep) with this toolchain
    • Don't need allocator_api feature for miden bench anymore (stabilized, but too lazy to find out when)
    • If you want this toolchain pinned to a specific version for any reason, I can change
  • Enabled risc0 feature for all/all_compilers
  • Update risc0 to 0.18
    • I'm making some assumptions for where the line is drawn for compile vs prove here based on their APIs. I couldn't quickly find a way to use their APIs in such a way that fits with this crate's traits.
    • DISCLAIMER: I have no affiliation with the risc0 team
  • Remove unnecessary allocation of the hash in the risc0 blake hash program
  • Switch to use no_std version of risc0 programs. std wasn't needed for anything
  • Remove ZKP::Default variant; wasn't used or needed
  • Remove risc0-log files (assuming it was accidentally committed)

I don't care for this to come in, feel free to close or make any changes you want to this. Just upstreaming this in case it helps :)

@mariari
Copy link
Member

mariari commented Sep 26, 2023

Thanks for getting risc0 working again. It was quite out of date and didn't put in the time to properly fix it. I will try it out tomorrow.

The old log files I will likely make a separate topic based from the latest release removing it.

I will be doing some minor changes to the history before merging

Thank you for your work

@austinabell
Copy link
Contributor Author

The old log files I will likely make a separate topic based from the latest release removing it.

I will be doing some minor changes to the history before merging

No worries and no rush!

@mariari
Copy link
Member

mariari commented Sep 26, 2023

The code is now merged, I played with the history a bit:

  1. taking a few of the commits and separating them into their own topics (logs deletion, and move to stable rust)
  2. I rebased the code on v0.1.0 since that is the latest base
  3. removed some of the refactoring of other parts

@mariari mariari closed this Sep 26, 2023
@mariari
Copy link
Member

mariari commented Sep 26, 2023

Thank you for your contribution

@mariari mariari mentioned this pull request Sep 26, 2023
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.

Re-enable RISC0
2 participants