-
Notifications
You must be signed in to change notification settings - Fork 70
fix(ci): remove hashFiles from guest-lib-tests #1710
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathanpwang
added a commit
that referenced
this pull request
Jun 4, 2025
leonardoalt
pushed a commit
to powdr-labs/openvm
that referenced
this pull request
Jun 20, 2025
* fix(recursion): final_poly & FRI missing randomness (openvm-org#1703) Updating the recursive verifier to match with the fix in GHSA-f69f-5fx9-w9r9 ## Final poly length plonky3 has changed so that `proof.final_poly.len()` must equal `config.final_poly_len()`. Since our recursion program only supports final poly length = 0, we can remove the previous checks that higher degree coefficients are zero. ## Missing randomness in mixed domain handling in FRI folding The folding needs to add `beta^2` to roll in new terms from reduced opening. We introduce a new array `betas_squared` to precompute and cache the squares since the `betas` are re-used across different FRI queries. (Side note on optimization: Zach and I discussed and it seemed like there is no difference between iter_zip'ing two arrays and loading them both versus storing a pair in one array since we load each Ext as a separate instruction). I changed the recursion code to use `iter_zip` now that another `betas_squared` needed to be passed in and we generally want to use `iter_zip` everywhere. There is no enumerate support in `iter_zip`, so I created another array of the indices to also zip over. Note: I think we previously were just skipping and not checking `ro[log_blowup]` corresponding to trace height 1 matrices. This is now fixed as explained in the comments (the reasoning is slightly different than in the Plonky3 implementation). --------- Co-authored-by: HrikB <23041116+HrikB@users.noreply.github.com> Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com> * refactor: ELF and Program (openvm-org#1638) - Remove `max_num_public_values` in `Program`/`Elf`. - Improve serialization/deserialization of `Program`. For a Program with 100,000 instructions: Previous: - Serialize: 3.1969ms - Deserialize: 3.2417ms - Size: 3625020 bytes Now: - Serialize: 1.3015ms - Deserialize: 1.9343ms - Size: 4000024 bytes closes INT-3131 closes INT-3771 * feat: Build script for calling init macros (openvm-org#1596) Added a build script that automatically adds calls to `moduli_init, complex_init, sw_init` appropriately by reading the vm config. * feat: Lazily call setup function for moduli and curves on first use (openvm-org#1603) We no longer need to manually call `setup_*` at the start of main for the moduli and curves. Instead, setup is automatically done on first use of each modulus or curve. Depends on openvm-org#1596 --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> * chore: update `getrandom` to v0.3 in `openvm` lib (openvm-org#1635) delete getrandom dependency from platform since getrandom now makes enabling of the custom backend opt-in via extern function and --cfg flag closes INT-3934 * feat: openvm build can build multiple targets + additional cargo options (openvm-org#1647) Resolves INT-4000, INT-3883, and INT-3662. In general, we would like `cargo openvm build` to align as closely with `cargo build` as possible. OpenVM users should be able to almost completely replace the latter with the former in their development workflows. Implements the following: - Parity between `cargo openvm build` and `cargo build` options - Ability to build multiple targets when building at the workspace level - `cargo openvm build` works outside of project root - Default paths in OpenVM CLI evaluate environment variable `HOME` at runtime OpenVM-generated artifacts being stored in `${CARGO_TARGET_DIR}/.openvm` is still to be implemented, but in order to maintain coherency between `cargo openvm build` and `cargo openvm run/prove` this change will be added later. * feat: Add e2e stark proof support (openvm-org#1597) - Add new type `E2eStarkProof` as the format of the final internal proof. `E2eStarkProof` supports `Encode` so it can be deserialized in other languages. - Add function `wrap_e2e_stark_proof` to wrap the final internal proof to `RootVerifierInput`. - Add `cargo openvm prove stark` to generate e2e stark proof. closes INT-3784 * feat: Add Rv32HintLoadByKey (openvm-org#1606) - Add `kv_store` into `Streams`. More details at `docs/specs/ISA.md`. - Add a new phantom instruction `Rv32HintLoadByKey` which can hint data based on a key at runtime. More details at `docs/specs/ISA.md`. - SDK support will be added in the future PRs. close INT-3893 * feat: Root Verifier ASM (openvm-org#1615) - Add a function `build_kernel_asm` into `RootVmVerifierConfig`. The generated ASM can be used in kernel functions for guest program to verify starks. More details could be found in the doc comments of the function. - Fix a confusing naming. `num_public_values` is actually the number of user public values instead of the number of public values of the proof. Use `num_user_public_values` instead. closes INT-3894 * feat: Macro define_verify_openvm_stark (openvm-org#1620) - Add a RISC-V custom instruction `nativestorew` into `Rv32IoTranspilerExtension`. - Add macro `define_verify_openvm_stark`, which can define a function for guest program to verify a stark proof. closes INT-3896 * feat: helper functions for verify_openvm_stark (openvm-org#1631) - `AppExecutionCommit` stores `u32` instead of `BabyBear`. - Rename `E2eStarkProof` to `VmStarkProof`. - Add `compute_hint_key_for_verify_openvm_stark`/`encode_rv32_public_values` for users to prepare inputs for `verify_openvm_stark`. But it still seems hard to understand what happens. - Improve docs about `Rv32HintLoadByKey`. * refactor: CLI Build & Setup (openvm-org#1649) - `cargo openvm setup` supports skipping halo2 proving keys. - `cargo openvm setup` outputs halo2 PK and stark PK as separated files. - `cargo openvm build` outputs `AppExecutionCommit` in json. The old output(`exe_commit.bytes`) was incorrect. close INT-3950 * fix: verifier read dir (openvm-org#1658) Fixes the path from which verifier contract is read. I tested the full CI flow on an aws instance to double check everything is working. * docs: add CHANGELOG.md (openvm-org#1660) Adds CHANGELOG.md to the repo. - [x] CHANGELOG notes for `v1.1.0` to be added --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> * chore: add entry point to book examples without openvm::init!() (openvm-org#1663) Resolves INT-4065. * feat: cargo CLI update target directory (openvm-org#1662) Resolves INT-3882, and records changes in the book as per INT-4038. Most importantly, reworks command outputs to primarily use the `target/` directory that Cargo uses and assumes full control over. - User can specify `target_dir`; if unspecified, `cargo openvm` will use the Cargo default - Keys are stored in `${target_dir}/openvm/`, while all other artifacts are stored in `${target_dir}/openvm/${profile}` - User can copy outputs to `${output_dir}` - **NOTE** - generated proofs are stored at `.`, as this file should generally be the final output Other changes include: - `build` no longer generates app commits - `run` will call `build` before attempting to execute - `prove` will call `build` before attempting to generate a proof - Replaced all references to `evm-proving-setup` with references to `setup` - `openvm.toml`'s default location is in the working directory instead of `.` The basic workflow should now be `cargo openvm prove ...` into `cargo openvm verify ...`. * feat: commit command for cargo openvm (openvm-org#1667) * feat: init command for cargo openvm (openvm-org#1670) * feat: CLI prove use bin name for output path + e2e stark verify (openvm-org#1675) Resolves INT-4074. Primarily accomplishes the following: - `cargo openvm prove` outputs proofs to `${bin_name}.app.proof` instead of `app.proof`, where `bin_name` is the file stem of the executable (same for `stark` and `evm`) - `cargo openvm verify` by default searches the working directory for files with extension `.app.proof` Additionally, the following are also in this PR: - `cargo openvm init ${dir}` creates directory `dir` if it doesn't already exist - Updates to the book to reflect recent CLI changes The following were merged from openvm-org#1689: - SDK functionality to verify e2e STARK proofs - `cargo openvm verify stark` subcommand * feat: ensure all CLI commit outputs are in hex + fix verify STARK (openvm-org#1697) Resolves INT-3880 and INT-4073. Implements the following: - All CLI commit outputs are in hex - `cargo openvm prove stark` outputs a JSON similar to `cargo openvm prove evm` - Updates STARK verification to check app executable and VM commits `cargo openvm prove stark` output looks like this: ``` { "app_exe_commit": "004807b0289c7eeaa279e610af936bec5fa6f677d4f2da64bb53f39d37debe3c", "app_vm_commit": "00249a27204ce75ed124a9cc45a722cab52744d57400c6967901260e5a510cd2", "user_public_values": "200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "proof": "0100000002000..." } ``` --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> * refactor: guest bindings (openvm-org#1613) This PR removes parts of the guest libraries for each extension. These libraries are to be moved into a new repo as part of the guest library re-org. Summary of changes for each extension: - algebra: minor changes - ecc: the re-export of halo2curves was removed, also minor changes - pairing: the traits, the `halo2curves_shims` module, along with a minimal set of constants remain in the openvm repo, while the bn254 and bls12_381 implementations were moved out - bigint: the U256 and I256 structs were deleted. Only Rust bindings remain. - sha256: the `sha256` and `set_sha256` functions were deleted, leaving behind only the Rust binding - keccak256: the `keccak256` and `set_keccak256` functions were deleted, leaving behind only the Rust binding Notes: - the k256 and p256 modules of the ecc extension were not deleted as they are useful for testing the ecc macros and traits. For this reason, the ecc tests were also not deleted - the algebra tests were also not deleted for the same reason - the `mod_sqrt` implementation was moved from the ecc extension to the algebra extension Benchmarks and examples are now outdated since they depend on guest libraries that no longer exist. These need to be redesigned and rewritten to use the new guest libraries. I wasn't able to easily port the benchmarks and examples because depending on the new guest libraries creates weird linker issues (I'm guessing due to double-importing openvm). TODO: - switch all deps to main after merge - uncomment ecdsa test Closes INT-3788 --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Co-authored-by: Stephen Hwang <stephenh@intrinsictech.xyz> * refactor: guest bindings (openvm-org#1613) This PR removes parts of the guest libraries for each extension. These libraries are to be moved into a new repo as part of the guest library re-org. Summary of changes for each extension: - algebra: minor changes - ecc: the re-export of halo2curves was removed, also minor changes - pairing: the traits, the `halo2curves_shims` module, along with a minimal set of constants remain in the openvm repo, while the bn254 and bls12_381 implementations were moved out - bigint: the U256 and I256 structs were deleted. Only Rust bindings remain. - sha256: the `sha256` and `set_sha256` functions were deleted, leaving behind only the Rust binding - keccak256: the `keccak256` and `set_keccak256` functions were deleted, leaving behind only the Rust binding Notes: - the k256 and p256 modules of the ecc extension were not deleted as they are useful for testing the ecc macros and traits. For this reason, the ecc tests were also not deleted - the algebra tests were also not deleted for the same reason - the `mod_sqrt` implementation was moved from the ecc extension to the algebra extension Benchmarks and examples are now outdated since they depend on guest libraries that no longer exist. These need to be redesigned and rewritten to use the new guest libraries. I wasn't able to easily port the benchmarks and examples because depending on the new guest libraries creates weird linker issues (I'm guessing due to double-importing openvm). TODO: - switch all deps to main after merge - uncomment ecdsa test Closes INT-3788 --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Co-authored-by: Stephen Hwang <stephenh@intrinsictech.xyz> * feat: Patch ecdsa `VerifyingKey::recover_from_prehash` to skip message verification (openvm-org#1691) The k256 and p256 guest libraries use the `ecdsa::VerifyingKey::recover_from_prehash` method which recovers the key and then verifies the message with that key. Our ecdsa implementation in the ECC extension does not do the final verification step. Furthermore, the latest pre-release version of the `ecdsa` crate also skips this step. This PR patches the `VerifyingKey` struct to skip the final message verification in the `recover_from_prehash` method. This is achieved by wrapping `ecdsa::VerifyingKey`. Changes: - ECC extension - The `VerifyingKey` and `PublicKey` structs have been renamed to `OpenVMVerifyingKey` and `OpenVMPublicKey` - A new struct `VerifyingKey` which patches the function was created - Guest libraries - the k256 and p256 guest libraries now re-export the new `openvm_ecc_guest::ecdsa::VerifyingKey` struct. Guest programs should now import `openvm_k256::ecdsa::VerifyingKey` instead of `ecdsa::VerifyingKey` to access the new changes. --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> * perf: update stark-backend commit * perf(ecc-guest): optimize setup handling in ecc guest bindings (openvm-org#1706) Changed the following to `unsafe` since they don't check identity and not equal cases. Also added a const generic `CHECK_SETUP` to optimize msm in Weierstrass case: ``` /// Hazmat: Assumes self != +- p2 and self != identity and p2 != identity. fn add_ne_nonidentity(&self, p2: &Self) -> Self; /// Hazmat: Assumes self != +- p2 and self != identity and p2 != identity. fn add_ne_assign_nonidentity(&mut self, p2: &Self); /// Hazmat: Assumes self != +- p2 and self != identity and p2 != identity. fn sub_ne_nonidentity(&self, p2: &Self) -> Self; /// Hazmat: Assumes self != +- p2 and self != identity and p2 != identity. fn sub_ne_assign_nonidentity(&mut self, p2: &Self); /// Hazmat: Assumes self != identity and 2 * self != identity. fn double_nonidentity(&self) -> Self; /// Hazmat: Assumes self != identity and 2 * self != identity. fn double_assign_nonidentity(&mut self); ``` Added new functions with const generic into the `WeierstrassPoint` trait: ``` fn add_assign_impl<const CHECK_SETUP: bool>(&mut self, p2: &Self); fn double_assign_impl<const CHECK_SETUP: bool>(&mut self); ``` This required moving the implementation of `add_assign` from the `impl_sw_group_ops` macro into the main `sw-macro/src/lib` and `impl_sw_affine` macros. It was **unintentional** but this seems to have triggered a compiler optimization that halved the number of cycles in ecrecover. I can only attribute it to the more custom `add_assign_impl` implementation moved into the `sw-macro` itself. * chore: update workspace to v1.2.1-rc.0 * feat: optimize `Group::is_identity` (openvm-org#1709) For some reason, the cycle count on ecrecover benchmark halves when the `Group::is_identity` default implementation is overridden (even when overridden with the same code as the default implementation). My guess is the compiler refuses to inline the default implementation for some reason. * chore: move directory * chore(ecc-guest): remove `k256, p256` modules and fix guest-libs to allow patching (openvm-org#1708) Cleaned up so that our custom `k256, p256` crates have the same name and version as the crates.io crates so they can be patched. I tested patching `revm` for both k256 and p256 precompiles. To remove code duplication, the `k256, p256` modules in `ecc-guest` have been removed. The integration tests now use the `guest-libs/{k256,p256}` crates. This seems acceptable for now; we would have to figure out something if we ever move `guest-libs` out of the monorepo. Note that `ecrecover` **increased** from 114972 cycles to 121638 cycles (5.8% increase) because revm uses `.to_encoded_point().to_bytes()` which is worse then the custom encoding we do directly. This indicates revm should still be patched directly. Updated all examples to the new guest-libs. Updated ruint to fork v1.14.0 -- unfortunately it's a bit hard to update the patches; I did it via a subtree to a separate repo forking ruint. * fix(ci): remove hashFiles from guest-lib-tests (openvm-org#1710) * chore(ci): patch examples to use local crates (openvm-org#1712) * chore: change to dual license (openvm-org#1713) * fix(sw-macro): host `set_up_once` undefined (openvm-org#1717) When the `sw-macro` generated struct is compiled for host (`target_os != "zkvm"`), there is no `set_up_once()` function. Apparently the compiler didn't catch this and the trait implementation of `fn set_up_once { Self::set_up_once() }` becomes an infinite loop. * docs: Add comment explaining the `is_identity` optimization (openvm-org#1716) A recent PR (openvm-org#1709) introduced the optimization of overriding the default implementation of `Group::is_identity` in the `impl_sw_group_ops` macro. The overridden implementation was identical to the default one. This PR adds a comment to explain the code duplication. --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> * fix(cli): generate `openvm_init.rs` before building (openvm-org#1721) * chore: macro crate should use workspace version (openvm-org#1723) * docs: update division typo in ISA.md (openvm-org#1726) Resolves openvm-org#1680. * chore(ecc-guest): placeholder trait and function implementations for patch compilation (openvm-org#1720) The `alloy-primitives` and `alloy-consensus` crates turn on ECDSA signing and verification algorithms together when `"k256"` feature is enabled. In order for our `k256` crate to be used as a patch, I had to provide functions to match. Because we use a custom `VerifyingKey` struct (unavoidable since the underlying types are custom to `openvm`), the `SigningKey` struct must also be custom (the required methods fundamentally will not work if we try to use the `ecdsa` crate's `SigningKey` but our own `VerifyingKey`). I have left the implementations `todo!` because we do not yet support signing as a functionality -- I could try to do a byte conversion to the `ecdsa::SigningKey` to passthrough the implementation, but I wanted to reduce the security surface area for now. Fixed a regression in ecrecover benchmark because the crates.io version of `revm` always set `sha3-keccak` feature on for `revm-precompile` which was turning off `native-keccak` linkage. * update stark-backend --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Co-authored-by: HrikB <23041116+HrikB@users.noreply.github.com> Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com> Co-authored-by: Xinding Wei <weixinding@gmail.com> Co-authored-by: Avaneesh-axiom <avaneeshk@intrinsictech.xyz> Co-authored-by: stephenh-axiom-xyz <stephenh@intrinsictech.xyz>
jonathanpwang
added a commit
that referenced
this pull request
Jul 15, 2025
# Update CHANGELOG with breaking changes since v1.2.0 This PR updates the CHANGELOG.md file to document all breaking interface changes between tag v1.2.0 and the current main branch, following the Keep a Changelog format. ## Changes Made - Added comprehensive [Unreleased] section documenting breaking changes in CLI, SDK, and Library Interfaces - Included migration guides for each category of breaking changes - Preserved existing version history while updating format to follow Keep a Changelog convention ## Breaking Changes Documented ### CLI - New `init` command for project initialization - New `commit` command for viewing executable commits - Setup command renamed from `EvmProvingSetupCmd` to `SetupCmd` - All CLI commit outputs now consistently formatted in hexadecimal - Prove command now uses binary name for default output file paths ### SDK - Import path change for `DEFAULT_MAX_NUM_PUBLIC_VALUES` - Configuration structure updates in `AggregationTreeConfig` ### Library Interfaces - Major guest bindings refactor removing guest library components from this repository - Impact on benchmarks and examples that depend on guest libraries ## Complete Commit List (v1.2.0 to main) * chore(ecc-guest): placeholder trait and function implementations for patch compilation (#1720) (Jonathan Wang) * docs: update division typo in ISA.md (#1726) (stephenh-axiom-xyz) * chore: macro crate should use workspace version (#1723) (Jonathan Wang) * fix(cli): generate `openvm_init.rs` before building (#1721) (Jonathan Wang) * docs: Add comment explaining the `is_identity` optimization (#1716) (Avaneesh-axiom) * fix(sw-macro): host `set_up_once` undefined (#1717) (Jonathan Wang) * chore: change to dual license (#1713) (Yi Sun) * chore(ci): patch examples to use local crates (#1712) (Jonathan Wang) * fix(ci): remove hashFiles from guest-lib-tests (#1710) (Jonathan Wang) * chore(ecc-guest): remove `k256, p256` modules and fix guest-libs to allow patching (#1708) (Jonathan Wang) * chore: move directory (Jonathan Wang) * feat: optimize `Group::is_identity` (#1709) (Avaneesh-axiom) * chore: update workspace to v1.2.1-rc.0 (Jonathan Wang) * perf(ecc-guest): optimize setup handling in ecc guest bindings (#1706) (Jonathan Wang) * perf: update stark-backend commit (Jonathan Wang) * feat: Patch ecdsa `VerifyingKey::recover_from_prehash` to skip message verification (#1691) (Avaneesh-axiom) * refactor: guest bindings (#1613) (Jonathan Wang) * refactor: guest bindings (#1613) (Avaneesh-axiom) * feat: ensure all CLI commit outputs are in hex + fix verify STARK (#1697) (stephenh-axiom-xyz) * feat: CLI prove use bin name for output path + e2e stark verify (#1675) (stephenh-axiom-xyz) * feat: init command for cargo openvm (#1670) (stephenh-axiom-xyz) * feat: commit command for cargo openvm (#1667) (stephenh-axiom-xyz) * feat: cargo CLI update target directory (#1662) (stephenh-axiom-xyz) * chore: add entry point to book examples without openvm::init!() (#1663) (stephenh-axiom-xyz) * docs: add CHANGELOG.md (#1660) (HrikB) * fix: verifier read dir (#1658) (HrikB) * refactor: CLI Build & Setup (#1649) (Xinding Wei) * feat: helper functions for verify_openvm_stark (#1631) (Xinding Wei) * feat: Macro define_verify_openvm_stark (#1620) (Xinding Wei) * feat: Root Verifier ASM (#1615) (Xinding Wei) * feat: Add Rv32HintLoadByKey (#1606) (Xinding Wei) * feat: Add e2e stark proof support (#1597) (Xinding Wei) * feat: openvm build can build multiple targets + additional cargo options (#1647) (stephenh-axiom-xyz) * chore: update `getrandom` to v0.3 in `openvm` lib (#1635) (Jonathan Wang) * feat: Lazily call setup function for moduli and curves on first use (#1603) (Avaneesh-axiom) * feat: Build script for calling init macros (#1596) (Avaneesh-axiom) * refactor: ELF and Program (#1638) (Xinding Wei) Fixes INT-4185 --- **Link to Devin run**: https://app.devin.ai/sessions/75c7bcb2466641609349ad3d02c31edd **Requested by**: Jonathan Wang --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Jonathan Wang <jpw@intrinsictech.xyz> Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Co-authored-by: Tim Hwang <41269763+timothyahwang@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.