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

Update docs with adding a new halo2 circuit in 'mopro-wam' #292

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

sifnoc
Copy link
Collaborator

@sifnoc sifnoc commented Jan 9, 2025

  • Added more item in prerequisite page
  • Describe how to add a new halo2 circuit implementation on mopro-wasm

Comment on lines 15 to 16
- [Android Studio](https://developer.android.com/studio)
- Also see [configuration](#android-configuration) below
- [JDK(Java Development Kit)](https://www.oracle.com/java/technologies/downloads)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can switch the order to

  • Android Studio
  • JDK
  • Also see configuration

because when a user see below, he will miss the JDK


Follow these steps to update and build the user-defined circuit implementation for Wasm:

1. **Update `Cargo.toml` in 'mopro-wasm'**
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the users should use mopro-wasm-lib (in mopro-example-app) instead of mopro-wasm in zkmopro.

Because in the future, the users don't need to clone the repo zkmopro/mopro and they can use CLI
(I think we should support mopro binary for users to use mopro CLI
like circom, they support binaries
so they don't need to clone the repo and use the binary)

Copy link
Collaborator

Choose a reason for hiding this comment

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

we should let users know how to generate bindings with rust setup
like in Flutter setup or React Native setup
(Just redirect them to rust setup and choose web as target)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the users should use mopro-wasm-lib (in mopro-example-app) instead of mopro-wasm in zkmopro.

For align consistancy with other setup doc, we should add create mopro-wasm-lib like "setup the rust project" in Rust setup and remove the "Proving from web browser" section :https://zkmopro.org/docs/setup/web-wasm-setup#proving-from-web-browser.
What do you think?

Because in the future, the users don't need to clone the repo zkmopro/mopro and they can use CLI (I think we should support mopro binary for users to use mopro CLI like circom, they support binaries so they don't need to clone the repo and use the binary)

I totally agree. but currently only simple doc "Getting Started" for about how to setup with the mopro cli.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For align consistancy with other setup doc, we should add create mopro-wasm-lib like "setup the rust project" in Rust setup and remove the "Proving from web browser" section :https://zkmopro.org/docs/setup/web-wasm-setup#proving-from-web-browser.
What do you think?

We should let user simply uses mopro init, mopro build for web
and he will find mopro-wasm-lib within the folder
(I realized that we should update react native setup and flutter setup, and guide them to look Getting Started section instead of Rust Setup section)
and yes we can remove the "Proving from web browser" section

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks good! I will update other setup docs that to rely on using mopro cli instead of starting with Rust Setup page.

docs/docs/setup/web-wasm-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/web-wasm-setup.md Outdated Show resolved Hide resolved
@sifnoc sifnoc force-pushed the updating-doc-2025-1 branch from ee756fb to 37f5b74 Compare January 13, 2025 13:30
Copy link

cloudflare-workers-and-pages bot commented Jan 13, 2025

Deploying mopro with  Cloudflare Pages  Cloudflare Pages

Latest commit: 93f0f4a
Status: ✅  Deploy successful!
Preview URL: https://78b7558c.mopro.pages.dev
Branch Preview URL: https://updating-doc-2025-1.mopro.pages.dev

View logs

@sifnoc
Copy link
Collaborator Author

sifnoc commented Jan 13, 2025

  • Now the Rust Setup page renamed as Manual Setup for Android/iOS Bindings for accuracy.
  • Moved the page item at the last in the side-bar. it looks more last option than using mopro cli
  • Added setup Halo2-based rust project alongside with Circom-based rust project: which was only the content in the Rust Setup

@sifnoc sifnoc requested a review from vivianjeng January 13, 2025 13:47
Copy link
Collaborator

@vivianjeng vivianjeng left a comment

Choose a reason for hiding this comment

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

The current version looks good to me! Thank you!

can you also update the doc in versioned_doc/version-0.1.0?
It is confusing to have 2 docs
but one is for current version (0.1.0) and the other is for the next version (might be0.2.0)
so just copy the current files in docs/ and paste to version-0.1.0/

also the mopro-wasm should be copied to docs/ too
Thank you

docs/docs/setup/flutter-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/react-native-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/react-native-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/rust-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/rust-setup.md Outdated Show resolved Hide resolved
docs/docs/setup/web-wasm-setup.md Outdated Show resolved Hide resolved
@vivianjeng
Copy link
Collaborator

and there is an error in cloudflare

00:38:30.251 | [cause]: Error: Docusaurus found broken links!
-- | --
00:38:30.251 |  
00:38:30.251 | Please check the pages of your site in the list below, and make sure you don't reference any path that does not exist.
00:38:30.251 | Note: it's possible to ignore broken links with the 'onBrokenLinks' Docusaurus configuration, and let the build pass.
00:38:30.252 |  
00:38:30.252 | Exhaustive list of all broken links found:
00:38:30.252 | - Broken link on source page path = /docs/next/setup/react-native-setup:
00:38:30.252 | -> linking to docs/react-native-setup#option-1-use-mopro-cli (resolved as: /docs/next/setup/docs/react-native-setup#option-1-use-mopro-cli)
00:38:30.252 |  
00:38:30.252 | at throwError (/opt/buildhome/repo/docs/.yarn/cache/@docusaurus-logger-npm-3.2.1-11923e9c7b-9d5db5253e.zip/node_modules/@docusaurus/logger/lib/index.js:79:11)
00:38:30.252 | at reportBrokenLinks (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/server/brokenLinks.js:242:47)
00:38:30.252 | at handleBrokenLinks (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/server/brokenLinks.js:274:5)
00:38:30.253 | at executeBrokenLinksCheck (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:182:47)
00:38:30.253 | at /opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:136:66
00:38:30.253 | at Object.async (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/utils.js:36:47)
00:38:30.253 | at buildLocale (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:136:30)
00:38:30.253 | at async tryToBuildLocale (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:46:13)
00:38:30.253 | at async /opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:64:9
00:38:30.253 | at async mapAsyncSequential (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-utils-virtual-daa5b7b8bd/0/cache/@docusaurus-utils-npm-3.2.1-cfade3be35-ea862b178e.zip/node_modules/@docusaurus/utils/lib/jsUtils.js:20:24)
00:38:30.253 | at async Command.build (/opt/buildhome/repo/docs/.yarn/__virtual__/@docusaurus-core-virtual-0694d71873/0/cache/@docusaurus-core-npm-3.2.1-027b2f9feb-9267f08b41.zip/node_modules/@docusaurus/core/lib/commands/build.js:62:5)

You can check by

cd docs && yarn build

to get the error

@sifnoc
Copy link
Collaborator Author

sifnoc commented Jan 14, 2025

The current version looks good to me! Thank you!

can you also update the doc in versioned_doc/version-0.1.0? It is confusing to have 2 docs but one is for current version (0.1.0) and the other is for the next version (might be0.2.0) so just copy the current files in docs/ and paste to version-0.1.0/

also the mopro-wasm should be copied to docs/ too Thank you

Synced at last two commits: 270f9df 93f0f4a

Copy link
Collaborator

@vivianjeng vivianjeng left a comment

Choose a reason for hiding this comment

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

LGTM!
Thank you so much!

@vivianjeng vivianjeng merged commit e424efd into main Jan 16, 2025
35 checks passed
@vivianjeng vivianjeng deleted the updating-doc-2025-1 branch January 16, 2025 07:19
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