feat: apply coderabbit review 1st round

Signed-off-by: Smuu <18609909+Smuu@users.noreply.github.com>

crates/cli/src/main.rs

@@ -37,17 +37,14 @@ async fn main() -> std::io::Result<()> {
                 )
             })?;
 
-            let verifying_key_algorithm = config.verifying_key_algorithm.as_str();
+            let verifying_key_algorithm = validate_algorithm(&config.verifying_key_algorithm)?;
 
-            if verifying_key_algorithm.is_empty() {
-                return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "verifying key algorithm is required"));
-            }
-
-            if !["ed25519", "secp256k1", "secp256r1"].contains(&verifying_key_algorithm) {
-                return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid verifying key algorithm"));
-            }
-
-            let prover_vk = VerifyingKey::from_algorithm_and_bytes(verifying_key_algorithm, config.verifying_key.unwrap().as_bytes()).unwrap();
+            let prover_vk = VerifyingKey::from_algorithm_and_bytes(
+                verifying_key_algorithm,
+                config.verifying_key.unwrap().as_bytes(),
+            ).map_err(|e| std::io::Error::new(
+              std::io::ErrorKind::InvalidData, format!("invalid prover verifying key: {}", e),
+            ))?;
 
             Arc::new(LightClient::new(da, celestia_config, Some(prover_vk)))
         }
@@ -72,14 +69,7 @@ async fn main() -> std::io::Result<()> {
                 .get_signing_key()
                 .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?;
 
-            let verifying_key_algorithm = config.verifying_key_algorithm.as_str();
-            if verifying_key_algorithm.is_empty() {
-                return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "verifying key algorithm is required"));
-            }
-
-            if !["ed25519", "secp256k1", "secp256r1"].contains(&verifying_key_algorithm) {
-                return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid verifying key algorithm"));
-            }
+            let verifying_key_algorithm = validate_algorithm(&config.verifying_key_algorithm)?;
 
             let signing_key = SigningKey::from_algorithm_and_bytes(verifying_key_algorithm, signing_key_chain.as_bytes()).unwrap();
             let verifying_key = signing_key.verifying_key();
@@ -128,16 +118,7 @@ async fn main() -> std::io::Result<()> {
                 .get_signing_key()
                 .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?;
 
-            // error if config.verifying_key_algorithm.as_str() is not present in config or invalid
-            if config.verifying_key_algorithm.is_empty() {
-              return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "verifying key algorithm is required"));
-            }
-
-            let verifying_key_algorithm = config.verifying_key_algorithm.as_str();
-
-            if !["ed25519", "secp256k1", "secp256r1"].contains(&verifying_key_algorithm) {
-              return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid verifying key algorithm"));
-            }
+            let verifying_key_algorithm = validate_algorithm(&config.verifying_key_algorithm)?;
 
             let signing_key = SigningKey::from_algorithm_and_bytes(verifying_key_algorithm, signing_key_chain.as_bytes()).unwrap();
 
@@ -149,10 +130,10 @@ async fn main() -> std::io::Result<()> {
                         "prover verifying key not found",
                     )
                 })
-                .and_then(|vk| VerifyingKey::from_algorithm_and_bytes(verifying_key_algorithm, vk.as_bytes()).map_err(|_| {
+                .and_then(|vk| VerifyingKey::from_algorithm_and_bytes(verifying_key_algorithm, vk.as_bytes()).map_err(|e| {
                     std::io::Error::new(
                         std::io::ErrorKind::InvalidData,
-                        "invalid prover verifying key",
+                        format!("invalid prover verifying key: {}", e),
                     )
                 }))?;
 
@@ -178,3 +159,15 @@ async fn main() -> std::io::Result<()> {
 
     node.start().await.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))
 }
+
+fn validate_algorithm(algorithm: &str) -> Result<&str, std::io::Error> {
+    if algorithm.is_empty() {
+        return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "verifying key algorithm is required"));
+    }
+
+    if !["ed25519", "secp256k1", "secp256r1"].contains(&algorithm) {
+        return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, "invalid verifying key algorithm"));
+    }
+
+    Ok(algorithm)
+}

crates/keys/src/signing_keys.rs

@@ -31,12 +31,12 @@ impl SigningKey {
         SigningKey::Secp256r1(Secp256r1SigningKey::random(&mut OsRng))
     }
 
-    pub fn new_with_algorithm(algorithm: &str) -> Self {
+    pub fn new_with_algorithm(algorithm: &str) -> Result<Self> {
         match algorithm {
-            "ed25519" => SigningKey::Ed25519(Box::new(Ed25519SigningKey::new(OsRng))),
-            "secp256k1" => SigningKey::Secp256k1(Secp256k1SigningKey::new(&mut OsRng)),
-            "secp256r1" => SigningKey::Secp256r1(Secp256r1SigningKey::random(&mut OsRng)),
-            _ => panic!("Unexpected key algorithm for SigningKey: {}", algorithm),
+            "ed25519" => Ok(SigningKey::Ed25519(Box::new(Ed25519SigningKey::new(OsRng)))),
+            "secp256k1" => Ok(SigningKey::Secp256k1(Secp256k1SigningKey::new(&mut OsRng))),
+            "secp256r1" => Ok(SigningKey::Secp256r1(Secp256r1SigningKey::random(&mut OsRng))),
+            _ => bail!("Unexpected key algorithm for SigningKey: '{}'. Expected one of: ed25519, secp256k1, secp256r1", algorithm),
         }
     }
 

crates/node_types/prover/src/prover/mod.rs

@@ -282,10 +282,9 @@ impl Prover {
         }
 
         // TODO: Issue #144
-        match epoch.verify_signature(self.cfg.verifying_key.clone()) {
-            Ok(_) => trace!("valid signature for epoch {}", epoch.height),
-            Err(e) => panic!("invalid signature in epoch {}: {:?}", epoch.height, e),
-        }
+        epoch.verify_signature(&self.cfg.verifying_key)
+            .with_context(|| format!("Invalid signature in epoch {}", epoch.height))?;
+        trace!("valid signature for epoch {}", epoch.height);
 
         let prev_commitment = self.db.get_commitment(&current_epoch)?;
 

crates/tests/src/lib.rs

@@ -47,7 +47,7 @@ async fn test_light_client_prover_talking() -> Result<()> {
     let bridge_da_layer = Arc::new(CelestiaConnection::new(&bridge_cfg, None).await.unwrap());
     let lc_da_layer = Arc::new(CelestiaConnection::new(&lc_cfg, None).await.unwrap());
     let db = setup_db();
-    let signing_key = SigningKey::new_with_algorithm(algorithm);
+    let signing_key = SigningKey::new_with_algorithm(algorithm)?;
     let pubkey = signing_key.verifying_key();
 
     let prover_cfg = prism_prover::Config {
@@ -80,7 +80,7 @@ async fn test_light_client_prover_talking() -> Result<()> {
 
         let mut transaction_builder = TransactionBuilder::new();
         let register_service_req =
-            transaction_builder.register_service_with_random_keys(algorithm, "test_service").commit();
+            transaction_builder.register_service_with_random_keys(algorithm, "test_service")?.commit();
 
         let mut i = 0;
 
@@ -93,7 +93,7 @@ async fn test_light_client_prover_talking() -> Result<()> {
             for _ in 0..num_new_accounts {
                 let random_user_id = format!("{}@gmail.com", i);
                 let new_acc = transaction_builder
-                    .create_account_with_random_key_signed(algorithm, random_user_id.as_str(), "test_service")
+                    .create_account_with_random_key_signed(algorithm, random_user_id.as_str(), "test_service")?
                     .commit();
                 match prover.clone().validate_and_queue_update(new_acc).await {
                     Ok(_) => {
@@ -112,7 +112,7 @@ async fn test_light_client_prover_talking() -> Result<()> {
                         .map_or("Could not find random account id", |id| id.as_str());
 
                     let update_acc =
-                        transaction_builder.add_random_key_verified_with_root(algorithm, acc_id).commit();
+                        transaction_builder.add_random_key_verified_with_root(algorithm, acc_id)?.commit();
 
                     match prover.clone().validate_and_queue_update(update_acc).await {
                         Ok(_) => (),