chore: no panics + fix ci pt 2 (#54)

* chore: get rid of panics

* format changes

* state

* aggregation

* don't use deprecated type

Author: jtguibas

.github/workflows/pr.yml

@@ -62,6 +62,10 @@ jobs:
       - name: Setup CI
         uses: ./.github/actions/setup
 
+      - name: Build RISC-V ELFs
+        run: |
+          ./x.sh
+
       - name: Run cargo test
         uses: actions-rs/cargo@v1
         with:
@@ -86,6 +90,10 @@ jobs:
       - name: Setup CI
         uses: ./.github/actions/setup
 
+      - name: Build RISC-V ELFs
+        run: |
+          ./x.sh
+
       - name: Run cargo fmt
         uses: actions-rs/cargo@v1
         with:

crates/rpc/src/retry.rs

@@ -55,10 +55,10 @@ where
                 // Check for tonic status errors.
                 if let Some(status) = e.downcast_ref::<tonic::Status>() {
                     match status.code() {
-                        Code::Unavailable |
-                        Code::DeadlineExceeded |
-                        Code::Internal |
-                        Code::Aborted => {
+                        Code::Unavailable
+                        | Code::DeadlineExceeded
+                        | Code::Internal
+                        | Code::Aborted => {
                             warn!(
                                 "Network temporarily unavailable when {} due to {}, retrying...",
                                 operation_name,
@@ -83,14 +83,14 @@ where
                 } else {
                     // Check for common transport errors.
                     let error_msg = e.to_string().to_lowercase();
-                    let is_transient = error_msg.contains("tls handshake") ||
-                        error_msg.contains("dns error") ||
-                        error_msg.contains("connection reset") ||
-                        error_msg.contains("broken pipe") ||
-                        error_msg.contains("transport error") ||
-                        error_msg.contains("failed to lookup") ||
-                        error_msg.contains("timeout") ||
-                        error_msg.contains("deadline exceeded");
+                    let is_transient = error_msg.contains("tls handshake")
+                        || error_msg.contains("dns error")
+                        || error_msg.contains("connection reset")
+                        || error_msg.contains("broken pipe")
+                        || error_msg.contains("transport error")
+                        || error_msg.contains("failed to lookup")
+                        || error_msg.contains("timeout")
+                        || error_msg.contains("deadline exceeded");
 
                     if is_transient {
                         warn!(

crates/vapp/src/errors.rs

@@ -133,6 +133,12 @@ pub enum VAppPanic {
 
     #[error("Hashing body failed")]
     HashingBodyFailed,
+
+    #[error("Failed to parse hash")]
+    FailedToParseBytes,
+
+    #[error("Missing public values hash")]
+    MissingPublicValuesHash,
 }
 
 impl From<VAppRevert> for VAppError {

crates/vapp/src/lib.rs

@@ -13,12 +13,10 @@
 #![allow(clippy::needless_range_loop)]
 #![allow(clippy::cast_lossless)]
 #![allow(clippy::bool_to_int_with_if)]
-#![allow(clippy::should_panic_without_expect)]
 #![allow(clippy::field_reassign_with_default)]
 #![allow(clippy::manual_assert)]
 #![allow(clippy::unreadable_literal)]
 #![allow(clippy::match_wildcard_for_single_variants)]
-#![allow(clippy::missing_panics_doc)]
 #![allow(clippy::missing_errors_doc)]
 #![allow(clippy::explicit_iter_loop)]
 #![allow(clippy::struct_excessive_bools)]

crates/vapp/src/signing.rs

@@ -2,7 +2,7 @@
 //!
 //! This module contains the functions for signing and verifying messages.
 
-use alloy_primitives::{Address, PrimitiveSignature};
+use alloy_primitives::{Address, Signature};
 use eyre::Result;
 use prost::Message;
 use sha2::{Digest, Sha256};
@@ -26,7 +26,7 @@ pub fn proto_hash<T: Message>(message: &T, sender: &Address) -> Vec<u8> {
 
 /// Verifies an Ethereum signature using the `personal_sign` format.
 pub fn eth_sign_verify(message: &[u8], signature: &[u8]) -> Result<Address> {
-    let signature = PrimitiveSignature::from_raw(signature)?;
+    let signature = Signature::from_raw(signature)?;
     let address = signature.recover_address_from_msg(message)?;
     Ok(address)
 }

crates/vapp/src/state.rs

@@ -307,9 +307,11 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
                 let domain = B256::try_from(body.domain.as_slice())
                     .map_err(|_| VAppPanic::DomainDeserializationFailed)?;
                 if domain != self.domain {
-                    return Err(
-                        VAppPanic::DomainMismatch { expected: self.domain, actual: domain }.into()
-                    );
+                    return Err(VAppPanic::DomainMismatch {
+                        expected: self.domain,
+                        actual: domain,
+                    }
+                    .into());
                 }
 
                 // Verify the proto signature.
@@ -364,9 +366,11 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
                 let domain = B256::try_from(body.domain.as_slice())
                     .map_err(|_| VAppPanic::DomainDeserializationFailed)?;
                 if domain != self.domain {
-                    return Err(
-                        VAppPanic::DomainMismatch { expected: self.domain, actual: domain }.into()
-                    );
+                    return Err(VAppPanic::DomainMismatch {
+                        expected: self.domain,
+                        actual: domain,
+                    }
+                    .into());
                 }
 
                 // Transfer the amount from the requester to the recipient.
@@ -446,10 +450,10 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
 
                 // Validate that the request ID is the same for all proto bodies.
                 debug!("validate that request ID is the same for all proto bodies");
-                if request_id.as_slice() != bid.request_id.as_slice() ||
-                    request_id.as_slice() != settle.request_id.as_slice() ||
-                    request_id.as_slice() != execute.request_id.as_slice() ||
-                    request_id.as_slice() != fulfill.request_id.as_slice()
+                if request_id.as_slice() != bid.request_id.as_slice()
+                    || request_id.as_slice() != settle.request_id.as_slice()
+                    || request_id.as_slice() != execute.request_id.as_slice()
+                    || request_id.as_slice() != fulfill.request_id.as_slice()
                 {
                     return Err(VAppPanic::RequestIdMismatch {
                         request_id: address(&request_id)?,
@@ -476,8 +480,8 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
 
                 // Validate that the prover is in the request whitelist, if a whitelist is provided.
                 debug!("validate prover is in whitelist");
-                if !request.whitelist.is_empty() &&
-                    !request.whitelist.contains(&prover_address.to_vec())
+                if !request.whitelist.is_empty()
+                    && !request.whitelist.contains(&prover_address.to_vec())
                 {
                     return Err(VAppPanic::ProverNotInWhitelist { prover: prover_address }.into());
                 }
@@ -518,7 +522,13 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
                 debug!("verify proof");
                 let mode = ProofMode::try_from(request.mode)
                     .map_err(|_| VAppPanic::UnsupportedProofMode { mode: request.mode })?;
-                let vk = bytes_to_words_be(&request.vk_hash.clone().try_into().unwrap());
+                let vk = bytes_to_words_be(
+                    &request
+                        .vk_hash
+                        .clone()
+                        .try_into()
+                        .map_err(|_| VAppPanic::FailedToParseBytes)?,
+                )?;
                 match mode {
                     ProofMode::Compressed => {
                         let verifier = V::default();
@@ -527,7 +537,12 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
                                 vk,
                                 // TODO(jtguibas): this should be either execute.public_values_hash
                                 // or request.public_values_hash
-                                execute.public_values_hash.clone().unwrap().try_into().unwrap(),
+                                execute
+                                    .public_values_hash
+                                    .clone()
+                                    .ok_or(VAppPanic::MissingPublicValuesHash)?
+                                    .try_into()
+                                    .map_err(|_| VAppPanic::FailedToParseBytes)?,
                             )
                             .map_err(|_| VAppPanic::InvalidProof)?;
                     }
@@ -589,8 +604,15 @@ impl<A: Storage<Address, Account>, R: Storage<RequestId, bool>> VAppState<A, R>
 
                 // Log the clear event.
                 debug!("log clear event");
-                let request_id: [u8; 32] =
-                    clear.fulfill.body.as_ref().unwrap().request_id.clone().try_into().unwrap();
+                let request_id: [u8; 32] = clear
+                    .fulfill
+                    .body
+                    .as_ref()
+                    .ok_or(VAppPanic::MissingProtoBody)?
+                    .request_id
+                    .clone()
+                    .try_into()
+                    .map_err(|_| VAppPanic::FailedToParseBytes)?;
                 info!(
                     "STEP {}: CLEAR(request_id={}, requester={}, prover={}, cost={})",
                     self.tx_id,

crates/vapp/src/utils.rs

@@ -7,14 +7,14 @@ use alloy_primitives::Address;
 use crate::errors::{VAppError, VAppPanic};
 
 /// Converts a 32-byte array to a 8-word array in big-endian order.
-#[must_use]
-pub fn bytes_to_words_be(bytes: &[u8; 32]) -> [u32; 8] {
+pub fn bytes_to_words_be(bytes: &[u8; 32]) -> Result<[u32; 8], VAppError> {
     let mut words = [0u32; 8];
     for i in 0..8 {
-        let chunk: [u8; 4] = bytes[i * 4..(i + 1) * 4].try_into().unwrap();
+        let chunk: [u8; 4] =
+            bytes[i * 4..(i + 1) * 4].try_into().map_err(|_| VAppPanic::FailedToParseBytes)?;
         words[i] = u32::from_be_bytes(chunk);
     }
-    words
+    Ok(words)
 }
 
 /// Converts a byte array to an address.
@@ -24,6 +24,7 @@ pub fn address(bytes: &[u8]) -> Result<Address, VAppError> {
 
 /// Test utilities for creating and verifying signatures.
 #[cfg(test)]
+#[allow(clippy::missing_panics_doc)]
 pub mod tests {
     /// Signing utilities for creating and verifying signatures.
     ///
@@ -32,7 +33,7 @@ pub mod tests {
     #[cfg(any(test, feature = "network"))]
     pub mod signers {
         use alloy::signers::{local::PrivateKeySigner, SignerSync};
-        use alloy_primitives::{keccak256, PrimitiveSignature, U256};
+        use alloy_primitives::{keccak256, Signature, U256};
         use prost::Message;
         use spn_network_types::{
             BidRequest, BidRequestBody, ExecuteProofRequest, ExecuteProofRequestBody,
@@ -59,7 +60,7 @@ pub mod tests {
         pub fn proto_sign<T: Message>(
             signer: &PrivateKeySigner,
             message: &T,
-        ) -> PrimitiveSignature {
+        ) -> Signature {
             let mut buf = Vec::new();
             message.encode(&mut buf).unwrap();
             signer.sign_message_sync(&buf).unwrap()

programs/vapp/aggregation/src/key.rs

@@ -1 +1 @@
-pub const STF_VKEY: [u32; 8] = [1792726344, 421991264, 1066483455, 1706997859, 1529590964, 965212562, 1130506634, 1658718291];
+pub const STF_VKEY: [u32; 8] = [1585405315, 815119958, 945268820, 963338962, 1420702568, 1573962290, 723711314, 266230300];

programs/vapp/aggregation/src/main.rs

@@ -27,8 +27,8 @@ pub fn main() {
     // Decode all StepPublicValues and validate state transitions.
     let mut decoded_steps: Vec<StepPublicValues> = Vec::new();
     for public_value in &public_values {
-        let step = StepPublicValues::abi_decode(public_value, false)
-            .expect("failed to decode StepPublicValues");
+        let step =
+            StepPublicValues::abi_decode(public_value).expect("failed to decode StepPublicValues");
         decoded_steps.push(step);
     }
 

x.sh

@@ -34,18 +34,4 @@ cd programs/vapp/aggregation
 cargo prove build --elf-name spn-vapp-aggregation $USE_DOCKER --tag v4.0.0 --output-directory ../../../elf
 cd ../../..
 echo "Done!"
-echo ""
-
-# Build the CLI.
-cargo build --bin spn-cli --release
-
-# Compute the verification key of the STF program.
-echo "Computing verification key of STF program..."
-cargo run --bin spn-cli --release -- vkey --elf-path elf/spn-vapp-stf
-echo "Done!"
-echo ""
-
-# Compute the verification key of the aggregation program.
-echo "Computing verification key of aggregation program..."
-cargo run --bin spn-cli --release -- vkey --elf-path elf/spn-vapp-aggregation
-echo "Done!"
+echo ""