Skip to content

feat: Add primitive guest libraries #1642

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 26 commits into from
May 19, 2025

Conversation

Avaneesh-axiom
Copy link
Contributor

No description provided.

This comment has been minimized.

@jonathanpwang jonathanpwang force-pushed the refactor/guest-library-reorg-final branch from 656670c to bea1fb1 Compare May 13, 2025 01:29
@Avaneesh-axiom Avaneesh-axiom force-pushed the refactor/guest-library-reorg-final branch from bea1fb1 to 4357ece Compare May 13, 2025 20:57
@Avaneesh-axiom Avaneesh-axiom force-pushed the feat/guest-libs branch 2 times, most recently from 0bff331 to c3c3506 Compare May 13, 2025 22:14

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

Before anything else, as mentioned in Xinding's PR, I feel for guest-libs the crate organization is unnecessarily complicated because it is a single rust library, not some collection like in VM extensions. Please follow the pattern in this PR: axiom-crypto/openvm-kzg#18

Copy link
Contributor

Choose a reason for hiding this comment

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

the best test now would be if you could patch k256 with openvm-k256 and see if ecrecover from unpatched revm still works

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

use openvm::io::read_vec;
use openvm_k256::Secp256k1Point;
use openvm_k256::{Secp256k1, Secp256k1Point};
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if you could just not have the openvm-k256 import at all here, and instead in Cargo.toml patch k256 with openvm-k256? that's how we would likely use it in practice

This comment has been minimized.

This comment has been minimized.

@Avaneesh-axiom Avaneesh-axiom force-pushed the feat/guest-libs branch 2 times, most recently from 64d99ed to 1613445 Compare May 19, 2025 18:09

This comment has been minimized.

@Avaneesh-axiom Avaneesh-axiom force-pushed the feat/guest-libs branch 3 times, most recently from ae9d4ab to 1221bd5 Compare May 19, 2025 19:43

This comment has been minimized.

Copy link

group app.proof_time_ms app.cycles app.cells_used leaf.proof_time_ms leaf.cycles leaf.cells_used
verify_fibair (-64 [-5.3%]) 1,142 334,086 17,676,626 - - -
fibonacci (+13 [+0.5%]) 2,482 1,500,277 50,589,503 - - -
regex (+238 [+3.2%]) 7,718 4,165,226 166,511,152 - - -
ecrecover (+596 [+43.0%]) 1,982 (+341127 [+117.9%]) 630,574 (+15216222 [+105.2%]) 29,686,408 - - -
pairing (-88 [-1.9%]) 4,484 (+36828 [+2.0%]) 1,857,264 (+1216224 [+1.3%]) 97,048,631 - - -

Commit: 3fd0a29

Benchmark Workflow

@jonathanpwang jonathanpwang marked this pull request as ready for review May 19, 2025 22:17
@jonathanpwang jonathanpwang merged commit ea95b66 into refactor/guest-library-reorg-final May 19, 2025
31 of 33 checks passed
@jonathanpwang jonathanpwang deleted the feat/guest-libs branch May 19, 2025 22:18
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