chore: merge main#1759
Merged
Merged
Conversation
Bumps [crossbeam-channel](https://github.com/crossbeam-rs/crossbeam) from 0.5.14 to 0.5.15. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/crossbeam-rs/crossbeam/releases">crossbeam-channel's releases</a>.</em></p> <blockquote> <h2>crossbeam-channel 0.5.15</h2> <ul> <li>Fix regression introduced in 0.5.12 that can lead to a double free when dropping unbounded channel. (<a href="https://redirect.github.com/crossbeam-rs/crossbeam/issues/1187">#1187</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/crossbeam-rs/crossbeam/commit/d35ffde18a76ae9667f602d8c0127ff89ac2e4d4"><code>d35ffde</code></a> Prepare for the next release</li> <li><a href="https://github.com/crossbeam-rs/crossbeam/commit/6ec74ecae896df5fc239518b45a1bfd258c9db68"><code>6ec74ec</code></a> crossbeam-channel: prevent double free on Drop (<a href="https://redirect.github.com/crossbeam-rs/crossbeam/issues/1187">#1187</a>)</li> <li>See full diff in <a href="https://github.com/crossbeam-rs/crossbeam/compare/crossbeam-channel-0.5.14...crossbeam-channel-0.5.15">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/openvm-org/openvm/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Performance improvements
Note on commits: the security review was done on commit 0f94c8a (tag v1.0.1-rc.1). This was rebased on top of the security fix from v1.1.0 to commit b4a5652 (tag v1.1.1-rc.1). One can check via ```bash git diff b4a5652 0f94c8a ``` that the only difference in the two commits is the fix from v1.1.0..v1.0.0.
I know you don't acknowledge PR's with minor typo fixes, But I found some typos that should be fixed, and they are not subjective or minor ones.
Truncates the patch version from halo2 verifier. Also bumps the openvm version to 1.1.2
Adds instructions on using the solidity sdk to the book. Closes INT-3974 --------- Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com>
Co-authored-by: Yi Sun <yi-sun@users.noreply.github.com>
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>
- 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
Added a build script that automatically adds calls to `moduli_init, complex_init, sw_init` appropriately by reading the vm config.
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
…ons (#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.
- 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
- 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
- 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
- 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
- `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`.
- `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
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.
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>
Resolves INT-4065.
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 ...`.
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.
A recent PR (#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>
…patch compilation (#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.
addressing typos and links to prevent future PRs --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
## Feature Gating for Halo2 Imports - INT-4112 This PR implements feature gating for all halo2 imports in the openvm-sdk and openvm-cli crates, making EVM proving/verification capabilities optional. This allows users who only need STARK functionality to compile without pulling in halo2 dependencies, improving compile times and reducing binary size. ### Changes Made #### SDK Dependencies (crates/sdk/Cargo.toml) - Made `snark-verifier` and `snark-verifier-sdk` optional dependencies - Removed forced `static-verifier` feature from `openvm-native-recursion` - Updated `evm-prove` feature definition to properly include halo2 dependencies: - `openvm-native-recursion/evm-prove` - `openvm-native-recursion/static-verifier` - `dep:snark-verifier` - `dep:snark-verifier-sdk` #### Feature Gating in SDK Source Files - **lib.rs**: Added `#[cfg(any(feature = "evm-prove", feature = "evm-verify"))]` to `agg_keygen` function - **keygen/mod.rs**: Feature-gated `Halo2ProvingKey` struct and related imports, added conditional compilation to `AggProvingKey.halo2_pk` field - **keygen/static_verifier.rs**: Feature-gated entire module with halo2-related functionality - **types.rs**: Added feature gates to all EVM-related types (`EvmHalo2Verifier`, `EvmProof`, etc.) - **fs.rs**: Feature-gated halo2 file I/O functions and imports #### CLI Updates (crates/cli/Cargo.toml) - Removed `evm-verify` from default features - Default features now: `["parallel", "jemalloc", "bench-metrics"]` #### CLI Source Updates (crates/cli/src/util.rs) - Feature-gated `read_agg_halo2_pk_from_file` import - Feature-gated `read_default_agg_pk()` function ### Verification All compilation scenarios have been tested and pass successfully: **SDK Crate:** - ✅ `cargo check --no-default-features` (STARK-only compilation) - ✅ `cargo check --features evm-prove` (EVM proving enabled) - ✅ `cargo check --features evm-verify` (EVM verification enabled) **CLI Crate:** - ✅ `cargo check --no-default-features` (STARK-only compilation) - ✅ `cargo check --features evm-verify` (EVM verification enabled) ### Impact - **STARK-only users**: Can now compile without halo2 dependencies, resulting in faster compile times and smaller binaries - **EVM users**: No breaking changes - existing functionality remains available when EVM features are enabled - **Backward compatibility**: Maintained through proper feature flag design ### Link to Devin run https://app.devin.ai/sessions/8bb6a0cf896e4b98972d872fffcb68bd **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>
The `NonQrHintSubEx` phantom sub-executor now uses a fixed seed so that execution is deterministic between different instances of the VM. This is not strictly necessary since the constructed non-QR could be passed between runs for reproducibility, but it seems a better property that re-runs of the VM have the same behavior (e.g., for segmentation). The only additional consideration is that now you can potentially add moduli to the VM config that take a little longer for the VM to initialize - but making it truly take a long time seems to involve breaking generalized Riemann hypothesis. Lastly, the phantom sub-executors `DecompressHintSubEx` and `NonQrHintSubEx` in Weierstrass extension were already removed from the spec since we now use the phantom sub-executors in the modular arithmetic extension, but these variants were not deleted from the code. So they are deleted in this PR. @Avaneesh-axiom do you know if this was from a bad merge previously?
# Add VERSIONING.md documentation This PR adds a new `VERSIONING.md` file to the root directory of the OpenVM repository to document our semantic versioning principles and backward compatibility guarantees. ## Changes - Created `VERSIONING.md` with comprehensive documentation of: - Semantic versioning approach (major.minor.patch) - Core principle: "Patch upgrade should be backward compatible" - Backward compatibility guarantees for patch versions - Exceptions for purely additive changes - Guidelines for patch-level changes ## Ticket - Linear ticket: **INT-4159** ## Link to Devin run https://app.devin.ai/sessions/ba99b2c95a5e4506b1d1ed5f42fe570c ## Testing This is a documentation-only change with no code modifications required. --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: lunkai@intrinsictech.xyz <lunkai@intrinsictech.xyz> Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com>
In the case `Is_x_reduced()` is true, the addition of `C::ORDER` to `r` is done in `Uint` and the upstream `ecdsa` crate errors if the addition overflow `Uint`'s fixed size. To match upstream behavior, we switch back to using the same `Uint` logic. Also took some additional care in handling conversions when the `Scalar<C>` num bytes differs from `Coordinate<C>` num bytes. Added a synthetic test case that failed before and now passes.
For some reason, unlike other libraries, the k256 crate has a special requirement that the signature is normalized ahead of time: https://github.com/RustCrypto/elliptic-curves/blob/5ac8f5d77f11399ff48d87b0554935f6eddda342/k256/src/ecdsa.rs#L203 To handle this, we introduce a new trait `VerifyCustomHook` that is called both in `recover_prehashed` and `verify_prehash`. - [x] Provide a proof that valid pubkey recovery implies the ECDSA signature verification algorithm will automatically pass, so there is no need for `recover_prehashed` to call `verify_prehash` again.
Fixes some panics in `from_sec1_bytes` where it should error instead.
However the root cause of many places is that
`IntMod::from_{le,be}_bytes` previously does not require the input bytes
to be reduced. While this is a feature of how we represent field
elements up to equivalence, in practice in cryptographic applications
other implementations always require the input to be in canonical
(reduced) from. To match other implementations and prevent future
errors, I have renamed the former `from_{le,be}_bytes` to
`from_{le,be}_bytes_unchecked` and added new checked versions that
return `Option`.
Also fixed implementations of `Decompress` for p256,k256 such that it
rejects when the input `x` bytes are not reduced.
This comment has been minimized.
This comment has been minimized.
CodSpeed Walltime Performance ReportMerging #1759 will not alter performanceComparing
|
Commit: 724ab98 |
CodSpeed Instrumentation Performance ReportMerging #1759 will not alter performanceComparing Summary
|
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
7 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.
we should do a merge instead of squash merge to reduce future conflicts