Merge pull request #56 from lambdaclass/improve_ffi_arg_types

Improve Mina verification FFI

Author: gabrielbosio

batcher/aligned-batcher/src/zk_utils/mod.rs

@@ -4,8 +4,8 @@ use crate::sp1::verify_sp1_proof;
 use aligned_sdk::core::types::{ProvingSystemId, VerificationData};
 use ethers::types::U256;
 use log::{debug, warn};
-use mina_account_verifier_ffi::verify_account_inclusion_ffi;
-use mina_state_verifier_ffi::verify_mina_state_ffi;
+use mina_account_verifier_ffi::verify_account_inclusion;
+use mina_state_verifier_ffi::verify_mina_state;
 
 pub(crate) async fn verify(verification_data: &VerificationData) -> bool {
     let verification_data = verification_data.clone();
@@ -67,45 +67,15 @@ fn verify_internal(verification_data: &VerificationData) -> bool {
                 return false;
             };
 
-            const MAX_PROOF_SIZE: usize = 48 * 1024;
-            const MAX_PUB_INPUT_SIZE: usize = 6 * 1024;
-
-            let mut proof_buffer = [0; MAX_PROOF_SIZE];
-            for (buffer_item, proof_item) in proof_buffer.iter_mut().zip(&verification_data.proof) {
-                *buffer_item = *proof_item;
-            }
-            let proof_len = verification_data.proof.len();
-
-            let mut pub_input_buffer = [0; MAX_PUB_INPUT_SIZE];
-            for (buffer_item, pub_input_item) in pub_input_buffer.iter_mut().zip(pub_input) {
-                *buffer_item = *pub_input_item;
-            }
-            let pub_input_len = pub_input.len();
-
-            verify_mina_state_ffi(&proof_buffer, proof_len, &pub_input_buffer, pub_input_len)
+            verify_mina_state(&verification_data.proof, &pub_input)
         }
         ProvingSystemId::MinaAccount => {
             let Some(pub_input) = verification_data.pub_input.as_ref() else {
                 warn!("Mina Account public input is missing");
                 return false;
             };
 
-            const MAX_PROOF_SIZE: usize = 16 * 1024;
-            const MAX_PUB_INPUT_SIZE: usize = 6 * 1024;
-
-            let mut proof_buffer = [0; MAX_PROOF_SIZE];
-            for (buffer_item, proof_item) in proof_buffer.iter_mut().zip(&verification_data.proof) {
-                *buffer_item = *proof_item;
-            }
-            let proof_len = verification_data.proof.len();
-
-            let mut pub_input_buffer = [0; MAX_PUB_INPUT_SIZE];
-            for (buffer_item, pub_input_item) in pub_input_buffer.iter_mut().zip(pub_input) {
-                *buffer_item = *pub_input_item;
-            }
-            let pub_input_len = pub_input.len();
-
-            verify_account_inclusion_ffi(&proof_buffer, proof_len, &pub_input_buffer, pub_input_len)
+            verify_account_inclusion(&verification_data.proof, &pub_input)
         }
     }
 }

operator/mina/lib/mina_verifier.h

@@ -1,6 +1,6 @@
 #include <stdbool.h>
+#include <stdint.h>
 
-bool verify_mina_state_ffi(unsigned char *proof_buffer,
-                            unsigned int proof_len,
-                            unsigned char *pub_input_buffer,
-                            unsigned int pub_input_len);
+int32_t verify_mina_state_ffi(unsigned char *proof_buffer, uint32_t proof_len,
+                              unsigned char *pub_input_buffer,
+                              uint32_t pub_input_len);

operator/mina/lib/src/lib.rs

@@ -34,35 +34,56 @@ lazy_static! {
     static ref MINA_SRS: SRS<Vesta> = SRS::<Vesta>::create(Fq::SRS_DEPTH);
 }
 
-// TODO(xqft): check proof size
-const MAX_PROOF_SIZE: usize = 48 * 1024;
-const MAX_PUB_INPUT_SIZE: usize = 6 * 1024;
-
 #[no_mangle]
 pub extern "C" fn verify_mina_state_ffi(
-    proof_buffer: &[u8; MAX_PROOF_SIZE],
-    proof_len: usize,
-    pub_input_buffer: &[u8; MAX_PUB_INPUT_SIZE],
-    pub_input_len: usize,
+    proof_bytes: *const u8,
+    proof_len: u32,
+    pub_input_bytes: *const u8,
+    pub_input_len: u32,
+) -> i32 {
+    let result = std::panic::catch_unwind(|| {
+        inner_verify_mina_state_ffi(proof_bytes, proof_len, pub_input_bytes, pub_input_len)
+    });
+
+    match result {
+        Ok(v) => v as i32,
+        Err(_) => -1,
+    }
+}
+
+fn inner_verify_mina_state_ffi(
+    proof_bytes: *const u8,
+    proof_len: u32,
+    pub_input_bytes: *const u8,
+    pub_input_len: u32,
 ) -> bool {
-    let Some(proof_buffer_slice) = proof_buffer.get(..proof_len) else {
-        error!("Proof length argument is greater than max proof size");
+    if proof_bytes.is_null() || pub_input_bytes.is_null() {
+        error!("Input buffer null");
         return false;
-    };
+    }
 
-    let Some(pub_input_buffer_slice) = pub_input_buffer.get(..pub_input_len) else {
-        error!("Public input length argument is greater than max public input size");
+    if proof_len == 0 || pub_input_len == 0 {
+        error!("Input buffer length zero size");
         return false;
-    };
+    }
+
+    let proof_bytes = unsafe { std::slice::from_raw_parts(proof_bytes, proof_len as usize) };
+
+    let pub_input_bytes =
+        unsafe { std::slice::from_raw_parts(pub_input_bytes, pub_input_len as usize) };
+
+    verify_mina_state(proof_bytes, pub_input_bytes)
+}
 
-    let proof: MinaStateProof = match bincode::deserialize(proof_buffer_slice) {
+pub fn verify_mina_state(proof_bytes: &[u8], pub_input_bytes: &[u8]) -> bool {
+    let proof: MinaStateProof = match bincode::deserialize(proof_bytes) {
         Ok(proof) => proof,
         Err(err) => {
             error!("Failed to deserialize state proof: {}", err);
             return false;
         }
     };
-    let pub_inputs: MinaStatePubInputs = match bincode::deserialize(pub_input_buffer_slice) {
+    let pub_inputs: MinaStatePubInputs = match bincode::deserialize(pub_input_bytes) {
         Ok(pub_inputs) => pub_inputs,
         Err(err) => {
             error!("Failed to deserialize state pub inputs: {}", err);
@@ -228,105 +249,51 @@ mod test {
 
     #[test]
     fn valid_mina_state_proof_verifies() {
-        let mut proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let proof_size = PROOF_BYTES.len();
-        assert!(proof_size <= proof_buffer.len());
-        proof_buffer[..proof_size].clone_from_slice(PROOF_BYTES);
-
-        let mut pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let pub_input_size = PUB_INPUT_BYTES.len();
-        assert!(pub_input_size <= pub_input_buffer.len());
-        pub_input_buffer[..pub_input_size].clone_from_slice(PUB_INPUT_BYTES);
-
-        let result =
-            verify_mina_state_ffi(&proof_buffer, proof_size, &pub_input_buffer, pub_input_size);
-        assert!(result);
+        let result = verify_mina_state_ffi(
+            PROOF_BYTES.as_ptr(),
+            PROOF_BYTES.len() as u32,
+            PUB_INPUT_BYTES.as_ptr(),
+            PUB_INPUT_BYTES.len() as u32,
+        );
+        assert_eq!(result, 1);
     }
 
     #[test]
     fn mina_state_proof_with_bad_bridge_tip_hash_does_not_verify() {
-        let mut proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let proof_size = PROOF_BYTES.len();
-        assert!(proof_size <= proof_buffer.len());
-        proof_buffer[..proof_size].clone_from_slice(PROOF_BYTES);
-
-        let mut pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let pub_input_size = BAD_HASH_PUB_INPUT_BYTES.len();
-        assert!(pub_input_size <= pub_input_buffer.len());
-        pub_input_buffer[..pub_input_size].clone_from_slice(BAD_HASH_PUB_INPUT_BYTES);
-
-        let result =
-            verify_mina_state_ffi(&proof_buffer, proof_size, &pub_input_buffer, pub_input_size);
-        assert!(!result);
+        let result = verify_mina_state_ffi(
+            PROOF_BYTES.as_ptr(),
+            PROOF_BYTES.len() as u32,
+            BAD_HASH_PUB_INPUT_BYTES.as_ptr(),
+            BAD_HASH_PUB_INPUT_BYTES.len() as u32,
+        );
+        assert_eq!(result, 0);
     }
 
     #[test]
     fn empty_mina_state_proof_does_not_verify() {
-        let proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let proof_size = PROOF_BYTES.len();
-
-        let mut pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let pub_input_size = PUB_INPUT_BYTES.len();
-        assert!(pub_input_size <= pub_input_buffer.len());
-        pub_input_buffer[..pub_input_size].clone_from_slice(PUB_INPUT_BYTES);
-
-        let result =
-            verify_mina_state_ffi(&proof_buffer, proof_size, &pub_input_buffer, pub_input_size);
-        assert!(!result);
-    }
-
-    #[test]
-    fn valid_mina_state_proof_with_empty_pub_input_does_not_verify() {
-        let mut proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let proof_size = PROOF_BYTES.len();
-        assert!(proof_size <= proof_buffer.len());
-        proof_buffer[..proof_size].clone_from_slice(PROOF_BYTES);
-
-        let pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let pub_input_size = PUB_INPUT_BYTES.len();
-
-        let result =
-            verify_mina_state_ffi(&proof_buffer, proof_size, &pub_input_buffer, pub_input_size);
-        assert!(!result);
-    }
-
-    #[test]
-    fn valid_mina_state_proof_with_greater_proof_size_does_not_verify() {
-        let mut proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let wrong_proof_size = super::MAX_PROOF_SIZE + 1;
-        proof_buffer[..PROOF_BYTES.len()].clone_from_slice(PROOF_BYTES);
-
-        let mut pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let pub_input_size = PUB_INPUT_BYTES.len();
-        assert!(pub_input_size <= pub_input_buffer.len());
-        pub_input_buffer[..pub_input_size].clone_from_slice(PUB_INPUT_BYTES);
+        const PROOF_SIZE: usize = PROOF_BYTES.len();
+        let empty_proof_buffer = [0u8; PROOF_SIZE];
 
         let result = verify_mina_state_ffi(
-            &proof_buffer,
-            wrong_proof_size,
-            &pub_input_buffer,
-            pub_input_size,
+            empty_proof_buffer.as_ptr(),
+            PROOF_SIZE as u32,
+            PUB_INPUT_BYTES.as_ptr(),
+            PUB_INPUT_BYTES.len() as u32,
         );
-        assert!(!result);
+        assert_eq!(result, 0);
     }
 
     #[test]
-    fn valid_mina_state_proof_with_greater_pub_input_size_does_not_verify() {
-        let mut proof_buffer = [0u8; super::MAX_PROOF_SIZE];
-        let proof_size = PROOF_BYTES.len();
-        assert!(proof_size <= proof_buffer.len());
-        proof_buffer[..proof_size].clone_from_slice(PROOF_BYTES);
-
-        let mut pub_input_buffer = [0u8; super::MAX_PUB_INPUT_SIZE];
-        let wrong_pub_input_size = MAX_PUB_INPUT_SIZE + 1;
-        pub_input_buffer[..PUB_INPUT_BYTES.len()].clone_from_slice(PUB_INPUT_BYTES);
+    fn valid_mina_state_proof_with_empty_pub_input_does_not_verify() {
+        const PUB_INPUT_SIZE: usize = PUB_INPUT_BYTES.len();
+        let empty_pub_input_buffer = [0u8; PUB_INPUT_SIZE];
 
         let result = verify_mina_state_ffi(
-            &proof_buffer,
-            proof_size,
-            &pub_input_buffer,
-            wrong_pub_input_size,
+            PROOF_BYTES.as_ptr(),
+            PROOF_BYTES.len() as u32,
+            empty_pub_input_buffer.as_ptr(),
+            PUB_INPUT_SIZE as u32,
         );
-        assert!(!result);
+        assert_eq!(result, 0);
     }
 }

operator/mina/mina.go

@@ -9,24 +9,36 @@ package mina
 import "C"
 import (
 	"fmt"
-	"time"
 	"unsafe"
 )
 
-// TODO(xqft): check proof size
-const MAX_PROOF_SIZE = 48 * 1024
-const MAX_PUB_INPUT_SIZE = 6 * 1024
-
-func timer() func() {
-	start := time.Now()
-	return func() {
-		fmt.Printf("Mina block verification took %v\n", time.Since(start))
+func VerifyMinaState(proofBuffer []byte, pubInputBuffer []byte) (isVerified bool, err error) {
+	// Here we define the return value on failure
+	isVerified = false
+	err = nil
+	if len(proofBuffer) == 0 || len(pubInputBuffer) == 0 {
+		return isVerified, err
 	}
-}
 
-func VerifyMinaState(proofBuffer [MAX_PROOF_SIZE]byte, proofLen uint, pubInputBuffer [MAX_PUB_INPUT_SIZE]byte, pubInputLen uint) bool {
-	defer timer()()
+	// This will catch any go panic
+	defer func() {
+		rec := recover()
+		if rec != nil {
+			err = fmt.Errorf("Panic was caught while verifying sp1 proof: %s", rec)
+		}
+	}()
+
 	proofPtr := (*C.uchar)(unsafe.Pointer(&proofBuffer[0]))
 	pubInputPtr := (*C.uchar)(unsafe.Pointer(&pubInputBuffer[0]))
-	return (bool)(C.verify_mina_state_ffi(proofPtr, (C.uint)(proofLen), pubInputPtr, (C.uint)(pubInputLen)))
+
+	r := (C.int32_t)(C.verify_mina_state_ffi(proofPtr, (C.uint32_t)(len(proofBuffer)), pubInputPtr, (C.uint32_t)(len(pubInputBuffer))))
+
+	if r == -1 {
+		err = fmt.Errorf("Panic happened on FFI while verifying Mina proof")
+		return isVerified, err
+	}
+
+	isVerified = (r == 1)
+
+	return isVerified, err
 }