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

Centralize RISC0 constants in Rust implementation #276

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

augustin-v
Copy link

I think I've successfully implemented the WASM binding for the RISC0 constants and updated the TypeScript code to use it. However, I'm unsure about the correct approach for the Python implementation. I initially attempted to import the constants directly from the WASM package in Python, but this approach may not be correct. I need guidance on whether to create a separate Python binding using PyO3 in the python_bindings/mod.rs file and create a risc0.rs file in garaga_rs/python_bindingsor if there's another recommended method for accessing these centralized constants in Python.

Issue Number: #270

@feltroidprime
Copy link
Collaborator

feltroidprime commented Dec 16, 2024

Hello, you're on the good path, here what you'll likely need to do to finalize and make CI pass:

  • fix npm CI
  • yes add a pyO3 binding, you can add it directly in mod.rs since it's a very small function and it takes no input parameter, no need to add a new file
  • then in python/parsing_utils.py the constant will be defined using the binding ie : CONTROL_ROOT_, CONTROL_ID = garaga_rs.get_risc0_constants()

use maturin develop --release to compile the python binding
do pytest -n auto to test everyting

go to tools/.../garaga_ts and do docker compose up --build as described https://felt.gitbook.io/garaga/installation/npm-package to update the .wasm file

@augustin-v augustin-v marked this pull request as ready for review December 17, 2024 15:51
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