Skip to content

refactor: Guest library re-organization (Part 1) #1604

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

Closed

Conversation

Avaneesh-axiom
Copy link
Contributor

@Avaneesh-axiom Avaneesh-axiom commented Apr 25, 2025

  • Deleted the guest library methods for Sha2 and Keccak256 extensions (only bindings remain)
  • Added a hint-based square root instruction to the algebra extension

The changes in this repo related to the guest library re-organization have been split into two PRs. This is the first one.

Notes:

  • some of the CI jobs are expected to fail since the re-org is not yet complete. For example, the ecdsa tests use keccak, but keccak has been deleted.
  • the mod_sqrt implementation was moved from the ecc extension to the algebra extension

Depends on #1603

@Avaneesh-axiom Avaneesh-axiom changed the base branch from main to feat/binding-primitive-libs April 25, 2025 21:39
@Avaneesh-axiom Avaneesh-axiom force-pushed the refactor/guest-library-reorg branch 3 times, most recently from 807eedc to 3f49069 Compare April 25, 2025 23:11
@Avaneesh-axiom Avaneesh-axiom force-pushed the refactor/guest-library-reorg branch from 3f49069 to 8f76dcd Compare April 25, 2025 23:34
@Avaneesh-axiom Avaneesh-axiom force-pushed the refactor/guest-library-reorg branch 4 times, most recently from 19ae003 to b5635f2 Compare April 29, 2025 02:16
@Avaneesh-axiom Avaneesh-axiom force-pushed the refactor/guest-library-reorg branch from b5635f2 to 1c56817 Compare April 29, 2025 02:25
@@ -26,11 +29,15 @@ To [leverage](./overview.md) compile-time known moduli for performance, you decl
```rust
moduli_declare! {
Bls12_381Fp { modulus = "0x1a0111ea397fe69a4b1ba7b6434bacd764774b84f38512bf6730d2a0f6b0f6241eabfffeb153ffffb9feffffffffaaab" },
Bn254Fp { modulus = "21888242871839275222246405745257275088696311157297823662689037894645226208583" },
Bn254Fp {
modulus = "21888242871839275222246405745257275088696311157297823662689037894645226208583", impl_field = true
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not quite a fan of the impl_field field, though I could live with it if we needed it. But I think it might actually be easy inside the proc-macro to just check if the modulus as a big-int is prime, so impl_field could be inferred?


/// Find the square root of `x` modulo `modulus` with `non_qr` a
/// quadratic nonresidue of the field.
pub fn mod_sqrt(x: &BigUint, modulus: &BigUint, non_qr: &BigUint) -> Option<BigUint> {
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be good in the PR desc to note that this is moved from weierstrass_extension

@@ -40,7 +20,7 @@ pub fn keccak256(input: &[u8]) -> [u8; 32] {
#[cfg(target_os = "zkvm")]
#[inline(always)]
#[no_mangle]
extern "C" fn native_keccak256(bytes: *const u8, len: usize, output: *mut u8) {
pub extern "C" fn native_keccak256(bytes: *const u8, len: usize, output: *mut u8) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is the pub needed? I just want to make sure it works with a non-pub extern: https://docs.rs/alloy-primitives/latest/src/alloy_primitives/utils/mod.rs.html#166

@Avaneesh-axiom Avaneesh-axiom reopened this May 1, 2025
@Avaneesh-axiom Avaneesh-axiom marked this pull request as ready for review May 1, 2025 03:52
@jonathanpwang
Copy link
Contributor

Assuming this was re-opened by accident

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