fix: add hook for ed decompress

crates/core/executor/src/hook.rs

@@ -18,6 +18,9 @@ pub const FD_ECRECOVER_HOOK: u32 = 5;
 /// The file descriptor through which to access `hook_ecrecover_2`.
 pub const FD_ECRECOVER_HOOK_2: u32 = 7;
 
+/// The file descriptor through which to access `hook_ed_decompress`.
+pub const FD_EDDECOMPRESS: u32 = 8;
+
 /// A runtime hook. May be called during execution by writing to a specified file descriptor,
 /// accepting and returning arbitrary data.
 pub trait Hook {
@@ -82,6 +85,7 @@ impl<'a> Default for HookRegistry<'a> {
             // add an assertion to the test `hook_fds_match` below.
             (FD_ECRECOVER_HOOK, hookify(hook_ecrecover)),
             (FD_ECRECOVER_HOOK_2, hookify(hook_ecrecover_v2)),
+            (FD_EDDECOMPRESS, hookify(hook_ed_decompress)),
         ]);
 
         Self { table }
@@ -179,8 +183,9 @@ pub fn hook_ecrecover_v2(_: HookEnv, buf: &[u8]) -> Vec<Vec<u8>> {
         sig = sig_normalized;
         recovery_id ^= 1;
     };
-    let recid = RecoveryId::from_byte(recovery_id).expect("Computed recovery ID is invalid, this is a bug.");
-
+    let recid = RecoveryId::from_byte(recovery_id)
+        .expect("Computed recovery ID is invalid, this is a bug.");
+
     // Attempting to recvover the public key has failed, write a 0 to indicate to the caller.
     let Ok(recovered_key) = VerifyingKey::recover_from_prehash(&msg_hash[..], &sig, recid) else {
         return vec![vec![0]];
@@ -194,6 +199,31 @@ pub fn hook_ecrecover_v2(_: HookEnv, buf: &[u8]) -> Vec<Vec<u8>> {
     vec![vec![1], bytes.to_vec(), s_inverse.to_bytes().to_vec()]
 }
 
+/// Checks if a compressed Edwards point can be decompressed.
+///
+/// # Arguments
+/// * `env` - The environment in which the hook is invoked.
+/// * `buf` - The buffer containing the compressed Edwards point.
+///    - The compressed Edwards point is 32 bytes.
+///    - The high bit of the last byte is the sign bit.
+///
+/// The result is either `0` if the point cannot be decompressed, or `1` if it can.
+///
+/// WARNING: This function merely hints at the validity of the compressed point. These values must
+/// be constrained by the zkVM for correctness.
+#[must_use]
+pub fn hook_ed_decompress(_: HookEnv, buf: &[u8]) -> Vec<Vec<u8>> {
+    let Ok(point) = sp1_curves::curve25519_dalek::CompressedEdwardsY::from_slice(buf) else {
+        return vec![vec![0]];
+    };
+
+    if sp1_curves::edwards::ed25519::decompress(&point).is_some() {
+        vec![vec![1]]
+    } else {
+        vec![vec![0]]
+    }
+}
+
 #[cfg(test)]
 pub mod tests {
     use super::*;

crates/core/executor/src/syscalls/precompiles/edwards/decompress.rs

@@ -53,7 +53,8 @@ impl<E: EdwardsParameters> Syscall for EdwardsDecompressSyscall<E> {
 
         // Compute actual decompressed X
         let compressed_y = CompressedEdwardsY(compressed_edwards_y);
-        let decompressed = decompress(&compressed_y);
+        let decompressed =
+            decompress(&compressed_y).expect("Decompression failed, syscall invariant violated.");
 
         let mut decompressed_x_bytes = decompressed.x.to_bytes_le();
         decompressed_x_bytes.resize(32, 0u8);

crates/core/machine/src/syscall/precompiles/edwards/ed_decompress.rs

@@ -99,7 +99,9 @@ impl<F: PrimeField32> EdDecompressCols<F> {
         let dyy = self.dyy.populate(blu_events, shard, &E::d_biguint(), &yy, FieldOperation::Mul);
         let v = self.v.populate(blu_events, shard, &one, &dyy, FieldOperation::Add);
         let u_div_v = self.u_div_v.populate(blu_events, shard, &u, &v, FieldOperation::Div);
-        let x = self.x.populate(blu_events, shard, &u_div_v, ed25519_sqrt);
+        let x = self.x.populate(blu_events, shard, &u_div_v, |p| {
+            ed25519_sqrt(p).expect("ed25519_sqrt failed, syscall invariant violated")
+        });
         self.neg_x.populate(blu_events, shard, &BigUint::zero(), &x, FieldOperation::Sub);
     }
 }

crates/curves/Cargo.toml

@@ -13,7 +13,6 @@ categories = { workspace = true }
 num = "0.4.3"
 serde = { version = "1.0.207", features = ["derive"] }
 typenum = "1.17.0"
-curve25519-dalek = { version = "4.1.2" }
 k256 = { version = "0.13.3", features = ["expose-field"] }
 generic-array = { version = "1.1.0", features = ["alloc", "serde"] }
 amcl = { package = "snowbridge-amcl", version = "1.0.2", default-features = false, features = [

crates/curves/src/edwards/ed25519.rs

@@ -1,6 +1,6 @@
 use std::str::FromStr;
 
-use curve25519_dalek::edwards::CompressedEdwardsY;
+use crate::curve25519_dalek::CompressedEdwardsY;
 use generic_array::GenericArray;
 use num::{BigUint, Num, One};
 use serde::{Deserialize, Serialize};
@@ -72,7 +72,7 @@ impl EdwardsParameters for Ed25519Parameters {
 ///
 /// This function always returns the nonnegative square root, in the sense that the least
 /// significant bit of the result is always 0.
-pub fn ed25519_sqrt(a: &BigUint) -> BigUint {
+pub fn ed25519_sqrt(a: &BigUint) -> Option<BigUint> {
     // Here is a description of how to calculate sqrt in the Curve25519 base field:
     // ssh://[email protected]/succinctlabs/curve25519-dalek/blob/
     // e2d1bd10d6d772af07cac5c8161cd7655016af6d/curve25519-dalek/src/field.rs#L256
@@ -108,18 +108,18 @@ pub fn ed25519_sqrt(a: &BigUint) -> BigUint {
     let flipped_sign_sqrt = beta_squared == neg_a;
 
     if !correct_sign_sqrt && !flipped_sign_sqrt {
-        panic!("a is not a square");
+        return None;
     }
 
     let beta_bytes = beta.to_bytes_le();
     if (beta_bytes[0] & 1) == 1 {
         beta = (&modulus - &beta) % &modulus;
     }
 
-    beta
+    Some(beta)
 }
 
-pub fn decompress(compressed_point: &CompressedEdwardsY) -> AffinePoint<Ed25519> {
+pub fn decompress(compressed_point: &CompressedEdwardsY) -> Option<AffinePoint<Ed25519>> {
     let mut point_bytes = *compressed_point.as_bytes();
     let sign = point_bytes[31] >> 7 == 1;
     // mask out the sign bit
@@ -134,15 +134,15 @@ pub fn decompress(compressed_point: &CompressedEdwardsY) -> AffinePoint<Ed25519>
     let v_inv = v.modpow(&(modulus - BigUint::from(2u64)), modulus);
     let u_div_v = (u * &v_inv) % modulus;
 
-    let mut x = ed25519_sqrt(&u_div_v);
+    let mut x = ed25519_sqrt(&u_div_v)?;
 
     // sqrt always returns the nonnegative square root,
     // so we negate according to the supplied sign bit.
     if sign {
         x = modulus - &x;
     }
 
-    AffinePoint::new(x, y.clone())
+    Some(AffinePoint::new(x, y.clone()))
 }
 
 #[cfg(test)]
@@ -178,7 +178,7 @@ mod tests {
 
                 CompressedEdwardsY(compressed)
             };
-            assert_eq!(point, decompress(&compressed_point));
+            assert_eq!(point, decompress(&compressed_point).unwrap());
 
             // Double the point to create a "random" point for the next iteration.
             point = point.clone() + point.clone();

crates/curves/src/lib.rs

@@ -7,7 +7,39 @@ pub mod utils;
 pub mod weierstrass;
 
 pub mod curve25519_dalek {
-    pub use curve25519_dalek::edwards::CompressedEdwardsY;
+    /// In "Edwards y" / "Ed25519" format, the curve point \\((x,y)\\) is
+    /// determined by the \\(y\\)-coordinate and the sign of \\(x\\).
+    ///
+    /// The first 255 bits of a `CompressedEdwardsY` represent the
+    /// \\(y\\)-coordinate.  The high bit of the 32nd byte gives the sign of \\(x\\).
+    ///
+    /// Note: This is taken from the `curve25519-dalek` crate.
+    #[derive(Copy, Clone, Eq, PartialEq, Hash)]
+    pub struct CompressedEdwardsY(pub [u8; 32]);
+
+    impl CompressedEdwardsY {
+        /// View this `CompressedEdwardsY` as a byte array.
+        pub fn as_bytes(&self) -> &[u8; 32] {
+            &self.0
+        }
+
+        /// Consume this `CompressedEdwardsY` and return the underlying byte array.
+        pub fn to_bytes(&self) -> [u8; 32] {
+            self.0
+        }
+
+        /// Construct a `CompressedEdwardsY` from a slice of bytes.
+        ///
+        /// # Errors
+        ///
+        /// Returns [`TryFromSliceError`] if the input `bytes` slice does not have
+        /// a length of 32.
+        pub fn from_slice(
+            bytes: &[u8],
+        ) -> Result<CompressedEdwardsY, core::array::TryFromSliceError> {
+            bytes.try_into().map(CompressedEdwardsY)
+        }
+    }
 }
 
 pub mod k256 {

crates/zkvm/lib/src/io.rs

@@ -18,6 +18,9 @@ pub const FD_ECRECOVER_HOOK: u32 = 5;
 /// The file descriptor through which to access `hook_ecrecover_2`.
 pub const FD_ECRECOVER_HOOK_2: u32 = 7;
 
+/// The file descriptor through which to access `hook_ed_decompress`.
+pub const FD_EDDECOMPRESS: u32 = 8;
+
 /// A writer that writes to a file descriptor inside the zkVM.
 struct SyscallWriter {
     fd: u32,

examples/Cargo.toml

@@ -61,7 +61,7 @@ sp1-lib = { path = "../crates/zkvm/lib", default-features = false }
 sp1-zkvm = { path = "../crates/zkvm/entrypoint", default-features = false }
 
 [patch.crates-io]
-curve25519-dalek = { git = "https://github.com/sp1-patches/curve25519-dalek", tag = "curve25519_dalek-v4.1.3-patch-v1" }
+# curve25519-dalek = { git = "https://github.com/sp1-patches/curve25519-dalek", tag = "curve25519_dalek-v4.1.3-patch-v1" }
 curve25519-dalek-ng = { git = "https://github.com/sp1-patches/curve25519-dalek-ng", tag = "curve25519_dalek_ng-v4.1.1-patch-v1" } 
 ecdsa-core = { git = "https://github.com/sp1-patches/signatures", package = "ecdsa", branch = "umadayal/secp256r1" }
 ed25519-consensus = { git = "https://github.com/sp1-patches/ed25519-consensus", tag = "ed25519_consensus-v2.1.0-patch-v1" }
@@ -73,3 +73,8 @@ sha2-v0-9-8 = { git = "https://github.com/sp1-patches/RustCrypto-hashes", packag
 tiny-keccak = { git = "https://github.com/sp1-patches/tiny-keccak", tag = "tiny_keccak-v2.0.2-patch-v1" }
 substrate-bn = { git = "https://github.com/sp1-patches/bn", tag = "substrate_bn-v0.6.0-patch-v1" }
 bls12_381 = { git = "https://github.com/sp1-patches/bls12_381", tag = "bls12_381-v0.8.0-patch-v1" }
+
+# todo remove
+sp1-lib = { path = "../crates/zkvm/lib" }
+sp1-curves = { path = "../crates/curves" }
+curve25519-dalek = { git = "https://github.com/sp1-patches/curve25519-dalek", branch = "n/patch-eddecompress-panic" }

examples/patch-testing/program/src/main.rs

@@ -81,15 +81,38 @@ fn test_curve25519_dalek_ng() {
 
 /// Emits ED_DECOMPRESS syscall.
 fn test_curve25519_dalek() {
-    let input = [1u8; 32];
-    let y = CompressedEdwardsY_dalek(input);
+    let input_passing = [1u8; 32];
+
+    // This y-coordinate is not square, and therefore not on the curve
+    let limbs: [u64; 4] = [
+        8083970408152925034,
+        11907700107021980321,
+        16259949789167878387,
+        5695861037411660086,
+    ];
+
+    // convert to bytes
+    let mut input_failing = [0u8; 32];
+    for (i, limb) in limbs.iter().enumerate() {
+        let bytes = limb.to_be_bytes();
+        input_failing[i..i+8].copy_from_slice(&bytes);
+    }
+
+    let y_passing = CompressedEdwardsY_dalek(input_passing);
 
     println!("cycle-tracker-start: curve25519-dalek decompress");
-    let decompressed_key = y.decompress().unwrap();
+    let decompressed_key = y_passing.decompress().unwrap();
     println!("cycle-tracker-end: curve25519-dalek decompress");
 
     let compressed_key = decompressed_key.compress();
-    assert_eq!(compressed_key, y);
+    assert_eq!(compressed_key, y_passing);
+
+    let y_failing = CompressedEdwardsY_dalek(input_failing);
+    println!("cycle-tracker-start: curve25519-dalek decompress");
+    let decompressed_key = y_failing.decompress();
+    println!("cycle-tracker-end: curve25519-dalek decompress");
+
+    assert!(decompressed_key.is_none());
 }
 
 /// Emits KECCAK_PERMUTE syscalls.