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

[develop] Add o1js-stub to Mina repo in place of SnarkyJS #14649

Merged
merged 19 commits into from
Dec 19, 2023

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented Nov 30, 2023

Summary

Addresses #14466

🔗 o1js: o1-labs/o1js#1318
🔗 o1js-bindings: o1-labs/o1js-bindings#222

This PR aims to add support for building mina as a submodule of o1js. To do so, the following was done:

  1. Removed the snarkyjs submodule
  2. Added a o1js_stub module, that has the same dependencies as o1js. The goal with this module is to have a way for Mina CI to check that o1js can still be successfully compiled after changes are made. This gives a safety net to not totally break things if changes to the dependencies are made.
  3. Adds the kimchi library from o1js-bindings into src/lib/crypto. This is to make sure that we can compile o1js_stub since we need the WASM dependencies generated from the library, as well as to support a longer term plan to create a o1 crypto repo that recombines the wasm bindings with o1caml-rust stubs.
  4. Removes snarkyjs from nix and updates the Rust part of nix to reference the new kimchi library location
  5. Removed codeowners entry for snarkyjs

@MartinMinkov MartinMinkov marked this pull request as ready for review November 30, 2023 22:24
@MartinMinkov
Copy link
Contributor Author

!ci-build-me

@MartinMinkov
Copy link
Contributor Author

!ci-build-me

@MartinMinkov
Copy link
Contributor Author

!ci-build-me

@MartinMinkov
Copy link
Contributor Author

!ci-build-me

Copy link
Member

@bkase bkase left a comment

Choose a reason for hiding this comment

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

I approve the CODEOWNERS change -- what tags me as a blocker on (I delegate the protocol and infra review to others even those GitHub will say it's ✅ )

Copy link
Member

@dannywillems dannywillems left a comment

Choose a reason for hiding this comment

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

Clean history! I like this, we should do more often. I have some comments though.

src/lib/crypto/kimchi/wasm/src/pasta_fp_plonk_index.rs Outdated Show resolved Hide resolved
src/lib/crypto/kimchi/wasm/src/pasta_fp_plonk_index.rs Outdated Show resolved Hide resolved
src/lib/crypto/kimchi/js/node_js/dune Outdated Show resolved Hide resolved
src/lib/crypto/kimchi/wasm/src/pasta_fq_plonk_index.rs Outdated Show resolved Hide resolved
@dannywillems
Copy link
Member

And same comment related to the CI and build than in #14461

@MartinMinkov
Copy link
Contributor Author

!ci-build-me

@mitschabaude
Copy link
Contributor

!ci-build-me

…of kimchi in source path to correctly locate the wasm source files
@MartinMinkov
Copy link
Contributor Author

!ci-build-me

@MartinMinkov MartinMinkov merged commit 4934295 into develop Dec 19, 2023
55 checks passed
@MartinMinkov MartinMinkov deleted the feat/add-o1js-stubs-develop branch December 19, 2023 20:21
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.

5 participants