chore: merge main#1844
Merged
Merged
Conversation
The point decompression function takes the x-coordinate and the parity of the y-coordinate. Then it solves $y^2 = x^3 + ax + b$ for $y$ up to parity. If the recovered $y$ doesn't have the correct parity, it returns the point $(x, -y)$, otherwise it returns $(x, y)$. The problem happens if the recovered $y$ is $0$ and `rec_id = 1`, in which case $-y$ has the same parity as $y$ (namely, even), so although the resulting point $(x, y)$ is a valid point on the curve, it does not correspond to `rec_id = 1`. This is not an issue for Secp256k1 and Secp256r1 since these curves do not have points with $y = 0$ (such a point would have order 2 since $P = -P$ and the order of these curves is not even) so any such decompression will fail. However, a curve could be constructed for which this is an issue. This PR adds a failing test with such a curve and then fixes the bug by checking the special case. Note that the curve equation for `CurvePoint5mod8` in the `decompress.rs` test was changed so that a point with y-coordinate zero exists in that curve. --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com>
Fixes the `verify_stark` integration test following some folder reorganization.
close INT-4031 --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com> Co-authored-by: HrikB <23041116+HrikB@users.noreply.github.com>
Runs the `verify-stark` integration test in CI
# Add versioning compatibility workflow for INT-4160 ## Overview This PR implements a new GitHub workflow to enforce OpenVM's versioning policy as described in ticket INT-4160. The workflow verifies that verification keys remain consistent between the main branch and the latest patch version (v1.2.0). ## Changes - **New workflow file**: `.github/workflows/versioning.yml` - **Workflow functionality**: - Runs `cargo openvm keygen` with default config on the main branch - Runs `cargo openvm keygen` with default config on v1.2.0 tag - Compares the generated verification keys (`app.vk`) using `cmp` command - Fails the workflow if verification keys differ, indicating a versioning policy violation ## Technical Implementation The workflow follows the established patterns in the repository: - Uses the same runner configuration as other workflows (`32cpu-linux-arm64`) - Includes proper Rust toolchain setup and caching - Installs the CLI tool using `cargo install --force --path crates/cli` - Uses default keygen configuration (no custom config file specified) - Outputs keys to separate directories for comparison ## Versioning Policy Enforcement As stated in `VERSIONING.md`, the core principle is: **"Patch upgrade should be backward compatible"**. This means the verification key (`MultiStarkVerifyingKey`) must not change across patch versions. This workflow automatically verifies this requirement. ## Testing The workflow has been structured based on existing patterns in the codebase: - Follows the same YAML structure as other workflows in `.github/workflows/` - Uses the same actions and runner configurations - Implements the keygen functionality as described in `crates/cli/src/commands/keygen.rs` Successful test run here: https://github.com/openvm-org/openvm/actions/runs/15722075897/job/44304740887 ## Ticket Reference - **Linear Ticket**: INT-4160 - **Link to Devin run**: https://app.devin.ai/sessions/b18b4684e92c44daabd69e34e47bd138 - **Requested by**: Yi Sun ## Workflow Triggers - Runs on push to `main` branch - Supports manual dispatch via `workflow_dispatch` --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com> Co-authored-by: Hrik Bhowal <23041116+HrikB@users.noreply.github.com>
For reasons I am not entirely clear on, when you use `getrandom_v02::Error`, the `getrandom` crate pulls in `std` library. But if you just directly implement what the `register_custom_getrandom!` macro does, this goes away... Fixes CI failure. Full disclosure: I got the idea from https://github.com/risc0/risc0/pull/3106/files#diff-fd010fc66ba8d918981d2705c0d10cd65d19db599969bb29bb14caffa62d3b65R95
## Summary - Updates the version command to display in the format `v1.3.0-rc.1 (abc123...)` instead of showing build timestamp - Adds version tag to openvm dependency when running `cargo openvm init` - Dependencies now include `tag = "v1.3.0-rc.1"` for reproducible builds ## Changes 1. Modified `OPENVM_VERSION_MESSAGE` in `crates/cli/src/lib.rs` to show version and commit hash 2. Updated `crates/cli/build.rs` to only emit git SHA (removed build timestamp) 3. Enhanced `add_openvm_dependency` function in `crates/cli/src/commands/init.rs` to include version tag ## Test plan - [x] Built cargo-openvm with `cargo build -p cargo-openvm` - [x] Tested version command: `./target/debug/cargo-openvm openvm --version` outputs `v1.3.0-rc.1 (46160dd)` - [x] Tested init command: `cargo openvm init` now adds dependency with version tag 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude <noreply@anthropic.com>
Separates the functions exposed by guest libs into their own section in the book. Closes INT-4294 --------- Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com>
Clarify that OpenVM's use of the Harvard architecture and how it relates to reflective programming. Closes INT-4352 --------- Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com>
Currently if we change the workspace version before tagging, the integration test for `cargo openvm init` will break because it creates a guest crate that references the tagged version. I updated the test so it patches to the local openvm if the environmental variable `USE_LOCAL_OPENVM=1` is set.
Contributor
Author
|
@shuklaayush @nyunyunyunyu here is the failing verify_stark test |
This comment has been minimized.
This comment has been minimized.
CodSpeed WallTime Performance ReportMerging #1844 will not alter performanceComparing
|
The patching doesn't work because cargo still tries to fetch from git first before doing the patch, so instead we use a find replace approach. I have tested this with v1.4.0-rc.0 to ensure it works.
Commit: ac7576d |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
DO NOT SQUASH MERGE