Fix Mina account FFI

Author: gabrielbosio

operator/mina/lib/src/lib.rs

@@ -70,7 +70,7 @@ fn inner_verify_mina_state_ffi(
     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, proof_len as usize) };
+        unsafe { std::slice::from_raw_parts(pub_input_bytes, pub_input_len as usize) };
 
     verify_mina_state(proof_bytes, pub_input_bytes)
 }

operator/mina_account/lib/mina_account_verifier.h

@@ -1,6 +1,7 @@
 #include <stdbool.h>
+#include <stdint.h>
 
-bool verify_account_inclusion_ffi(unsigned char *proof_buffer,
-                                  unsigned int proof_len,
-                                  unsigned char *public_input_buffer,
-                                  unsigned int public_input_len);
+int32_t verify_account_inclusion_ffi(unsigned char *proof_buffer,
+                                     uint32_t proof_len,
+                                     unsigned char *public_input_buffer,
+                                     uint32_t public_input_len);

operator/mina_account/lib/src/lib.rs

@@ -12,9 +12,25 @@ mod merkle_verifier;
 #[no_mangle]
 pub extern "C" fn verify_account_inclusion_ffi(
     proof_bytes: *const u8,
-    proof_len: usize,
+    proof_len: u32,
     pub_input_bytes: *const u8,
-    pub_input_len: usize,
+    pub_input_len: u32,
+) -> i32 {
+    let result = std::panic::catch_unwind(|| {
+        inner_verify_account_inclusion_ffi(proof_bytes, proof_len, pub_input_bytes, pub_input_len)
+    });
+
+    match result {
+        Ok(v) => v as i32,
+        Err(_) => -1,
+    }
+}
+
+fn inner_verify_account_inclusion_ffi(
+    proof_bytes: *const u8,
+    proof_len: u32,
+    pub_input_bytes: *const u8,
+    pub_input_len: u32,
 ) -> bool {
     if proof_bytes.is_null() || pub_input_bytes.is_null() {
         error!("Input buffer null");
@@ -29,7 +45,7 @@ pub extern "C" fn verify_account_inclusion_ffi(
     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, proof_len as usize) };
+        unsafe { std::slice::from_raw_parts(pub_input_bytes, pub_input_len as usize) };
 
     verify_account_inclusion(proof_bytes, pub_input_bytes)
 }
@@ -45,6 +61,7 @@ pub fn verify_account_inclusion(proof_bytes: &[u8], pub_input_bytes: &[u8]) -> b
             return false;
         }
     };
+    error!("pub input len: {}", pub_input_bytes.len());
     let MinaAccountPubInputs {
         ledger_hash,
         encoded_account,
@@ -94,80 +111,50 @@ mod test {
 
     #[test]
     fn valid_account_state_proof_verifies() {
-        let result = verify_account_inclusion(PROOF_BYTES, PUB_INPUT_BYTES);
-        assert!(result);
-    }
-
-    #[test]
-    fn empty_account_state_proof_does_not_verify() {
-        let proof_buffer = [0u8; PROOF_BYTES.len()];
-        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_account_inclusion(&proof_buffer, &pub_input_buffer, pub_input_size);
-        assert!(!result);
-    }
-
-    #[test]
-    fn valid_account_state_proof_with_empty_pub_input_does_not_verify() {
         let mut proof_buffer = [0u8; PROOF_BYTES.len()];
         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 mut pub_input_buffer = [0u8; PUB_INPUT_BYTES.len()];
         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_account_inclusion_ffi(
-            &proof_buffer,
-            proof_size,
-            &pub_input_buffer,
-            pub_input_size,
+            proof_buffer.as_ptr(),
+            proof_size as u32,
+            pub_input_buffer.as_ptr(),
+            pub_input_size as u32,
         );
-        assert!(!result);
+        assert_eq!(result, 1);
     }
 
     #[test]
-    fn valid_account_state_proof_with_greater_proof_size_does_not_verify() {
-        let mut proof_buffer = [0u8; PROOF_BYTES.len()];
-        let wrong_proof_size = 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);
+    fn empty_account_state_proof_does_not_verify() {
+        const PROOF_SIZE: usize = PROOF_BYTES.len();
+        let proof_buffer = [0u8; PROOF_SIZE];
 
         let result = verify_account_inclusion_ffi(
-            &proof_buffer,
-            wrong_proof_size,
-            &pub_input_buffer,
-            pub_input_size,
+            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_account_state_proof_with_greater_pub_input_size_does_not_verify() {
-        let mut proof_buffer = [0u8; PROOF_BYTES.len()];
-        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_account_state_proof_with_empty_pub_input_does_not_verify() {
+        const PUB_INPUT_SIZE: usize = PUB_INPUT_BYTES.len();
+        let pub_input_buffer = [0u8; PUB_INPUT_SIZE];
 
         let result = verify_account_inclusion_ffi(
-            &proof_buffer,
-            proof_size,
-            &pub_input_buffer,
-            wrong_pub_input_size,
+            PROOF_BYTES.as_ptr(),
+            PROOF_BYTES.len() as u32,
+            pub_input_buffer.as_ptr(),
+            PUB_INPUT_SIZE as u32,
         );
-        assert!(!result);
+        assert_eq!(result, 0);
     }
 }

operator/mina_account/mina_account.go

@@ -9,20 +9,35 @@ package mina_account
 import "C"
 import (
 	"fmt"
-	"time"
 	"unsafe"
 )
 
-func timer() func() {
-	start := time.Now()
-	return func() {
-		fmt.Printf("Mina account verification took %v\n", time.Since(start))
+func VerifyAccountInclusion(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 VerifyAccountInclusion(proofBuffer []byte, proofLen uint, pubInputBuffer []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_account_inclusion_ffi(proofPtr, (C.uint)(proofLen), pubInputPtr, (C.uint)(pubInputLen)))
+	r := (C.int32_t)(C.verify_account_inclusion_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 account proof")
+		return isVerified, err
+	}
+
+	isVerified = (r == 1)
+
+	return isVerified, err
 }

operator/mina_account/mina_account_test.go

@@ -1,76 +1,71 @@
 package mina_account_test
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
 	"github.com/yetanotherco/aligned_layer/operator/mina_account"
 )
 
+const ProofFilePath = "../../scripts/test_files/mina_account/mina_account.proof"
+
+const PubInputFilePath = "../../scripts/test_files/mina_account/mina_account.pub"
+
 func TestMinaStateProofVerifies(t *testing.T) {
-	fmt.Println(os.Getwd())
-	proofFile, err := os.Open("../../scripts/test_files/mina_account/mina_account.proof")
+	proofBytes, err := os.ReadFile(ProofFilePath)
 	if err != nil {
 		t.Errorf("could not open mina account proof file")
 	}
 
-	proofBuffer := make([]byte, mina_account.MAX_PROOF_SIZE)
-	proofLen, err := proofFile.Read(proofBuffer)
-	if err != nil {
-		t.Errorf("could not read bytes from mina account proof file")
-	}
-
-	pubInputFile, err := os.Open("../../scripts/test_files/mina_account/mina_account.pub")
-	if err != nil {
-		t.Errorf("could not open mina account pub inputs file")
-	}
-	pubInputBuffer := make([]byte, mina_account.MAX_PUB_INPUT_SIZE)
-	pubInputLen, err := pubInputFile.Read(pubInputBuffer)
+	pubInputBytes, err := os.ReadFile(PubInputFilePath)
 	if err != nil {
-		t.Errorf("could not read bytes from mina account pub inputs hash")
+		t.Errorf("could not open mina account pub input file")
 	}
 
-	if !mina_account.VerifyAccountInclusion(([mina_account.MAX_PROOF_SIZE]byte)(proofBuffer), uint(proofLen), ([mina_account.MAX_PUB_INPUT_SIZE]byte)(pubInputBuffer), uint(pubInputLen)) {
+	verified, err := mina_account.VerifyAccountInclusion(proofBytes, pubInputBytes)
+	if err != nil || !verified {
 		t.Errorf("proof did not verify")
 	}
 }
 
 func TestEmptyMinaStateProofDoesNotVerify(t *testing.T) {
-	fmt.Println(os.Getwd())
-	proofBuffer := make([]byte, mina_account.MAX_PROOF_SIZE)
-
-	pubInputFile, err := os.Open("../../scripts/test_files/mina_account/mina_account.pub")
+	proofBytes, err := os.ReadFile(ProofFilePath)
 	if err != nil {
-		t.Errorf("could not open mina account pub inputs file")
+		t.Errorf("could not open mina state proof file")
 	}
-	pubInputBuffer := make([]byte, mina_account.MAX_PUB_INPUT_SIZE)
-	pubInputLen, err := pubInputFile.Read(pubInputBuffer)
+	emptyProofBuffer := make([]byte, len(proofBytes))
+
+	pubInputBytes, err := os.ReadFile(PubInputFilePath)
 	if err != nil {
-		t.Errorf("could not read bytes from mina account pub inputs hash")
+		t.Errorf("could not open mina state pub input file")
 	}
 
-	if mina_account.VerifyAccountInclusion(([mina_account.MAX_PROOF_SIZE]byte)(proofBuffer), mina_account.MAX_PROOF_SIZE, ([mina_account.MAX_PUB_INPUT_SIZE]byte)(pubInputBuffer), uint(pubInputLen)) {
-		t.Errorf("Empty proof should not verify but it did")
+	verified, err := mina_account.VerifyAccountInclusion(emptyProofBuffer, pubInputBytes)
+	if err != nil {
+		t.Errorf("verification failed with error")
+	}
+	if verified {
+		t.Errorf("proof should not verify")
 	}
 }
 
 func TestMinaStateProofWithEmptyPubInputDoesNotVerify(t *testing.T) {
-	fmt.Println(os.Getwd())
-	proofFile, err := os.Open("../../scripts/test_files/mina_account/mina_account.proof")
+	proofBytes, err := os.ReadFile(ProofFilePath)
 	if err != nil {
-		t.Errorf("could not open mina account proof file")
+		t.Errorf("could not open mina state proof file")
 	}
 
-	proofBuffer := make([]byte, mina_account.MAX_PROOF_SIZE)
-	proofLen, err := proofFile.Read(proofBuffer)
+	pubInputBytes, err := os.ReadFile(PubInputFilePath)
 	if err != nil {
-		t.Errorf("could not read bytes from mina account proof file")
+		t.Errorf("could not open mina state pub input file")
 	}
+	emptyPubInputBuffer := make([]byte, len(pubInputBytes))
 
-	pubInputBuffer := make([]byte, mina_account.MAX_PUB_INPUT_SIZE)
-
-	if mina_account.VerifyAccountInclusion(([mina_account.MAX_PROOF_SIZE]byte)(proofBuffer), uint(proofLen), ([mina_account.MAX_PUB_INPUT_SIZE]byte)(pubInputBuffer), mina_account.MAX_PUB_INPUT_SIZE) {
-		t.Errorf("proof with empty public input should not verify but id did")
+	verified, err := mina_account.VerifyAccountInclusion(proofBytes, emptyPubInputBuffer)
+	if err != nil {
+		t.Errorf("verification failed with error")
+	}
+	if verified {
+		t.Errorf("proof should not verify")
 	}
 }

operator/pkg/operator.go

@@ -531,21 +531,13 @@ func (o *Operator) verify(verificationData VerificationData, disabledVerifiersBi
 		o.handleVerificationResult(results, verificationResult, err, "Risc0 proof verification")
 		results <- verificationResult
 	case common.Mina:
-		proofLen := (uint)(len(verificationData.Proof))
-		pubInputLen := (uint)(len(verificationData.PubInput))
-
-		verificationResult := mina.VerifyMinaState(verificationData.Proof, proofLen, verificationData.PubInput, (uint)(pubInputLen))
+		verificationResult, err := mina.VerifyMinaState(verificationData.Proof, verificationData.PubInput)
 		o.Logger.Infof("Mina state proof verification result: %t", verificationResult)
-		// TODO: Remove the nil value passed as err argument
-		o.handleVerificationResult(results, verificationResult, nil, "Mina state proof verification")
+		o.handleVerificationResult(results, verificationResult, err, "Mina state proof verification")
 	case common.MinaAccount:
-		proofLen := (uint)(len(verificationData.Proof))
-		pubInputLen := (uint)(len(verificationData.PubInput))
-
-		verificationResult := mina_account.VerifyAccountInclusion(verificationData.Proof, proofLen, verificationData.PubInput, (uint)(pubInputLen))
+		verificationResult, err := mina_account.VerifyAccountInclusion(verificationData.Proof, verificationData.PubInput)
 		o.Logger.Infof("Mina account inclusion proof verification result: %t", verificationResult)
-		// TODO: Remove the nil value passed as err argument
-		o.handleVerificationResult(results, verificationResult, nil, "Mina account state proof verification")
+		o.handleVerificationResult(results, verificationResult, err, "Mina account state proof verification")
 	default:
 		o.Logger.Error("Unrecognized proving system ID")
 		results <- false