diff --git a/Cargo.lock b/Cargo.lock index ca4f1edaa..1985c39e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2882,6 +2882,7 @@ dependencies = [ "ssv_network_config", "ssv_types", "task_executor", + "thiserror 2.0.12", "tokio", "tower", "tracing", diff --git a/anchor/common/ssv_types/src/lib.rs b/anchor/common/ssv_types/src/lib.rs index 39646a671..644e0fc02 100644 --- a/anchor/common/ssv_types/src/lib.rs +++ b/anchor/common/ssv_types/src/lib.rs @@ -2,7 +2,7 @@ pub use cluster::{Cluster, ClusterId, ClusterMember, ValidatorIndex, ValidatorMe pub use committee::{CommitteeId, CommitteeInfo}; pub use operator::{Operator, OperatorId}; pub use share::Share; -pub use util::parse_rsa; +pub use util::{RsaParseError, parse_rsa}; mod cluster; mod committee; pub mod consensus; diff --git a/anchor/common/ssv_types/src/operator.rs b/anchor/common/ssv_types/src/operator.rs index a376cc2f8..c513c1409 100644 --- a/anchor/common/ssv_types/src/operator.rs +++ b/anchor/common/ssv_types/src/operator.rs @@ -5,7 +5,7 @@ use openssl::{pkey::Public, rsa::Rsa}; use ssz_derive::{Decode, Encode}; use types::Address; -use crate::util::parse_rsa; +use crate::{RsaParseError, util::parse_rsa}; /// Unique identifier for an Operator. #[derive( @@ -41,7 +41,11 @@ pub struct Operator { impl Operator { /// Creates a new operator from its OperatorId and PEM-encoded public key string - pub fn new(pem_data: &str, operator_id: OperatorId, owner: Address) -> Result { + pub fn new( + pem_data: &str, + operator_id: OperatorId, + owner: Address, + ) -> Result { let rsa_pubkey = parse_rsa(pem_data)?; Ok(Self::new_with_pubkey(rsa_pubkey, operator_id, owner)) } diff --git a/anchor/common/ssv_types/src/util.rs b/anchor/common/ssv_types/src/util.rs index 9e9ffc60d..2d397953d 100644 --- a/anchor/common/ssv_types/src/util.rs +++ b/anchor/common/ssv_types/src/util.rs @@ -1,16 +1,27 @@ use base64::prelude::*; use openssl::{pkey::Public, rsa::Rsa}; +use thiserror::Error; + +/// Errors that can occur during RSA key parsing +#[derive(Error, Debug, Clone)] +pub enum RsaParseError { + #[error("Unable to decode base64 PEM data: {0}")] + Base64Decode(#[from] base64::DecodeError), + + #[error("Unable to convert decoded PEM data into a string: {0}")] + Utf8Conversion(#[from] std::string::FromUtf8Error), + + #[error("Failed to parse RSA public key: {0}")] + RsaParsing(#[from] openssl::error::ErrorStack), +} // Parse from a RSA public key string into the associated RSA representation -pub fn parse_rsa(pem_data: &str) -> Result, String> { +pub fn parse_rsa(pem_data: &str) -> Result, RsaParseError> { // First decode the base64 data - let pem_decoded = BASE64_STANDARD - .decode(pem_data) - .map_err(|e| format!("Unable to decode base64 pem data: {e}"))?; + let pem_decoded = BASE64_STANDARD.decode(pem_data)?; // Convert the decoded data to a string - let mut pem_string = String::from_utf8(pem_decoded) - .map_err(|e| format!("Unable to convert decoded pem data into a string: {e}"))?; + let mut pem_string = String::from_utf8(pem_decoded)?; // Fix the header - replace PKCS1 header with PKCS8 header pem_string = pem_string @@ -21,8 +32,7 @@ pub fn parse_rsa(pem_data: &str) -> Result, String> { .replace("-----END RSA PUBLIC KEY-----", "-----END PUBLIC KEY-----"); // Parse the PEM string into an RSA public key using PKCS8 format - let rsa_pubkey = Rsa::public_key_from_pem(pem_string.as_bytes()) - .map_err(|e| format!("Failed to parse RSA public key: {e}"))?; + let rsa_pubkey = Rsa::public_key_from_pem(pem_string.as_bytes())?; Ok(rsa_pubkey) } diff --git a/anchor/eth/Cargo.toml b/anchor/eth/Cargo.toml index 87469a83b..ee36d0730 100644 --- a/anchor/eth/Cargo.toml +++ b/anchor/eth/Cargo.toml @@ -23,6 +23,7 @@ slot_clock = { workspace = true } ssv_network_config = { workspace = true } ssv_types = { workspace = true } task_executor = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true } tower = "0.5.2" tracing = { workspace = true } diff --git a/anchor/eth/src/error.rs b/anchor/eth/src/error.rs index b607ee266..9467b88e8 100644 --- a/anchor/eth/src/error.rs +++ b/anchor/eth/src/error.rs @@ -1,20 +1,121 @@ -use std::fmt::Display; +use thiserror::Error; + +use crate::util::ShareParseError; // Custom execution integration layer errors -#[derive(Debug)] +#[derive(Debug, Error)] pub enum ExecutionError { + #[error("Sync error: {0}")] SyncError(String), - InvalidEvent(String), + + #[error("Invalid event for operator {operator_id:?} (owner: {owner:?}): {message}")] + InvalidOperatorEvent { + operator_id: ssv_types::OperatorId, + owner: alloy::primitives::Address, + message: String, + #[source] + source: Option>, + }, + + #[error( + "Invalid event for validator {validator_pubkey:?} (cluster: {cluster_id:?}): {message}" + )] + InvalidValidatorEvent { + validator_pubkey: Option, + cluster_id: Option, + message: String, + #[source] + source: Option>, + }, + + #[error("Invalid event for cluster {cluster_id:?} (owner: {owner:?}): {message}")] + InvalidClusterEvent { + cluster_id: Option, + owner: Option, + message: String, + #[source] + source: Option>, + }, + + #[error("Invalid event: {message}")] + InvalidEvent { + message: String, + #[source] + source: Option>, + }, + + #[error("RPC error: {0}")] RpcError(String), + + #[error("WebSocket error: {0}")] WsError(String), + + #[error("Decode error: {0}")] DecodeError(String), + + #[error("Miscellaneous error: {0}")] Misc(String), + + #[error("Duplicate error: {0}")] Duplicate(String), + + #[error("Database error: {0}")] Database(String), + + #[error("Share parse error: {0}")] + ShareParseError(#[from] ShareParseError), } -impl Display for ExecutionError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{self:?}") +impl ExecutionError { + pub fn invalid_operator_event( + operator_id: ssv_types::OperatorId, + owner: alloy::primitives::Address, + message: impl Into, + source: Option>, + ) -> Self { + Self::InvalidOperatorEvent { + operator_id, + owner, + message: message.into(), + source, + } + } + + pub fn invalid_validator_event( + validator_pubkey: Option, + cluster_id: Option, + message: impl Into, + source: Option>, + ) -> Self { + Self::InvalidValidatorEvent { + validator_pubkey, + cluster_id, + message: message.into(), + source, + } + } + + pub fn invalid_cluster_event( + cluster_id: Option, + owner: Option, + message: impl Into, + source: Option>, + ) -> Self { + Self::InvalidClusterEvent { + cluster_id, + owner, + message: message.into(), + source, + } + } + + pub fn invalid_event( + message: impl Into, + source: Option>, + ) -> Self { + Self::InvalidEvent { + message: message.into(), + source, + } } } diff --git a/anchor/eth/src/event_processor.rs b/anchor/eth/src/event_processor.rs index c93ed5bce..365f9b8d7 100644 --- a/anchor/eth/src/event_processor.rs +++ b/anchor/eth/src/event_processor.rs @@ -158,22 +158,25 @@ impl EventProcessor { } = SSVContract::OperatorAdded::decode_from_log(log)?; let operator_id = OperatorId(operatorId); - debug!(operator_id = ?operator_id, owner = ?owner, "Processing operator added"); - // Confirm that this operator does not already exist if self.db.state().operator_exists(&operator_id) { - return Err(ExecutionError::Duplicate(format!( - "Operator with id {operator_id:?} already exists in database" - ))); + return Err(ExecutionError::invalid_operator_event( + operator_id, + owner, + format!("Operator with id {operator_id:?} already exists in database"), + None, + )); } // Parse ABI encoded public key string and trim off 0x prefix for hex decoding let public_key_str = publicKey.to_string(); let public_key_str = public_key_str.trim_start_matches("0x"); let data = hex::decode(public_key_str).map_err(|e| { - debug!(operator_id = ?operator_id, error = %e, "Failed to decode public key data from hex"); - ExecutionError::InvalidEvent( - format!("Failed to decode public key data from hex: {e}") + ExecutionError::invalid_operator_event( + operator_id, + owner, + "Failed to decode public key data from hex", + Some(Box::new(e)), ) })?; @@ -181,33 +184,35 @@ impl EventProcessor { let data = if data.len() == 704 { let data = &data[64..]; let data = String::from_utf8(data.to_vec()).map_err(|e| { - debug!(operator_id = ?operator_id, error = %e, "Failed to convert to UTF8 String"); - ExecutionError::InvalidEvent(format!("Failed to convert to UTF8 String: {e}")) + ExecutionError::invalid_operator_event( + operator_id, + owner, + "Failed to convert to UTF8 String", + Some(Box::new(e)), + ) })?; data.trim_matches(char::from(0)).to_string() } else { String::from_utf8(data.to_vec()).map_err(|e| { - debug!(operator_id = ?operator_id, error = %e, "Failed to convert to UTF8 String"); - ExecutionError::InvalidEvent(format!("Failed to convert to UTF8 String: {e}")) + ExecutionError::invalid_operator_event( + operator_id, + owner, + "Failed to convert to UTF8 String", + Some(Box::new(e)), + ) })? }; // Construct the Operator and insert it into the database let operator = Operator::new(&data, operator_id, owner).map_err(|e| { - debug!( - operator_pubkey = ?publicKey, - operator_id = ?operator_id, - error = %e, - "Failed to construct operator" - ); - ExecutionError::InvalidEvent(format!("Failed to construct operator: {e}")) + ExecutionError::invalid_operator_event( + operator_id, + owner, + "Failed to construct operator", + Some(Box::new(e)), + ) })?; self.db.insert_operator(&operator, tx).map_err(|e| { - debug!( - operator_id = ?operator_id, - error = %e, - "Failed to insert operator into database" - ); ExecutionError::Database(format!("Failed to insert operator into database: {e}")) })?; @@ -231,17 +236,11 @@ impl EventProcessor { let SSVContract::OperatorRemoved { operatorId } = SSVContract::OperatorRemoved::decode_from_log(log)?; let operator_id = OperatorId(operatorId); - debug!(operator_id = ?operator_id, "Processing operator removed"); // Delete the operator from database and in memory - self.db.delete_operator(operator_id, tx).map_err(|e| { - debug!( - operator_id = ?operator_id, - error = %e, - "Failed to remove operator" - ); - ExecutionError::Database(format!("Failed to remove operator: {e}")) - })?; + self.db + .delete_operator(operator_id, tx) + .map_err(|e| ExecutionError::Database(format!("Failed to remove operator: {e}")))?; debug!(operator_id = ?operatorId, "Operator removed from network"); metrics::inc_counter_vec(&metrics::EXECUTION_EVENTS_PROCESSED, &["operator_removed"]); @@ -270,14 +269,13 @@ impl EventProcessor { shares, .. } = SSVContract::ValidatorAdded::decode_from_log(log)?; - debug!(owner = ?owner, operator_count = operatorIds.len(), "Processing validator addition"); // Get the expected nonce and then increment it. This will happen regardless of if the // event is malformed or not - let nonce = self.db.bump_and_get_nonce(&owner, tx).map_err(|e| { - debug!(owner = ?owner, "Failed to bump nonce"); - ExecutionError::Database(format!("Failed to bump nonce: {e}")) - })?; + let nonce = self + .db + .bump_and_get_nonce(&owner, tx) + .map_err(|e| ExecutionError::Database(format!("Failed to bump nonce: {e}")))?; // During keysplitting, we only care about the nonce let Mode::Node { @@ -298,7 +296,6 @@ impl EventProcessor { validate_operators(&operator_ids, &cluster_id, &self.db.state())?; // Parse the share byte stream into a list of valid Shares and then verify the signature - debug!(cluster_id = ?cluster_id, "Parsing and verifying shares"); let (signature, shares) = parse_shares( shares.to_vec(), &operator_ids, @@ -306,21 +303,26 @@ impl EventProcessor { &validator_pubkey, ) .map_err(|e| { - debug!(cluster_id = ?cluster_id, error = %e, "Failed to parse shares"); - ExecutionError::InvalidEvent(format!("Failed to parse shares. {e}")) + ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(cluster_id), + "Failed to parse shares", + Some(Box::new(e)), + ) })?; if !verify_signature(signature, nonce, &owner, &validator_pubkey) { - debug!(cluster_id = ?cluster_id, "Signature verification failed"); - return Err(ExecutionError::InvalidEvent( - "Signature verification failed".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(cluster_id), + "Signature verification failed", + None, )); } // Fetch the validator metadata let validator_metadata = construct_validator_metadata(&validator_pubkey, &cluster_id) .map_err(|e| { - debug!(validator_pubkey= ?validator_pubkey, "Failed to fetch validator metadata"); ExecutionError::Database(format!("Failed to fetch validator metadata: {e}")) })?; @@ -341,7 +343,6 @@ impl EventProcessor { self.db .insert_validator(cluster, validator_metadata.clone(), shares, tx) .map_err(|e| { - debug!(cluster_id = ?cluster_id, error = %e, validator_metadata = ?validator_metadata.public_key, "Failed to insert validator into cluster"); ExecutionError::Database(format!("Failed to insert validator into cluster: {e}")) })?; @@ -377,7 +378,6 @@ impl EventProcessor { publicKey, .. } = SSVContract::ValidatorRemoved::decode_from_log(log)?; - debug!(owner = ?owner, public_key = ?publicKey, "Processing Validator Removed"); // Parse the public key let validator_pubkey = parse_validator_pubkey(&publicKey)?; @@ -390,12 +390,11 @@ impl EventProcessor { let metadata = match state.metadata().get_by(&validator_pubkey) { Some(data) => data, None => { - debug!( - cluster_id = ?cluster_id, - "Failed to fetch validator metadata from database" - ); - return Err(ExecutionError::Database( - "Failed to fetch validator metadata from database".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(cluster_id), + "Failed to fetch validator metadata from database", + None, )); } }; @@ -404,40 +403,35 @@ impl EventProcessor { let cluster = match state.clusters().get_by(&validator_pubkey) { Some(data) => data, None => { - debug!( - cluster_id = ?cluster_id, - "Failed to fetch cluster from database" - ); - return Err(ExecutionError::Database( - "Failed to fetch cluster from database".to_string(), + return Err(ExecutionError::invalid_cluster_event( + Some(cluster_id), + Some(owner), + "Failed to fetch cluster from database", + None, )); } }; // Make sure the right owner is removing this validator if owner != cluster.owner { - debug!( - cluster_id = ?cluster_id, - expected_owner = ?cluster.owner, - actual_owner = ?owner, - "Owner mismatch for validator removal" - ); - return Err(ExecutionError::InvalidEvent(format!( - "Cluster already exists with a different owner address. Expected {}. Got {}", - cluster.owner, owner - ))); + return Err(ExecutionError::invalid_cluster_event( + Some(cluster_id), + Some(owner), + format!( + "Cluster already exists with a different owner address. Expected {}. Got {}", + cluster.owner, owner + ), + None, + )); } // Make sure this is the correct validator if validator_pubkey != metadata.public_key { - debug!( - cluster_id = ?cluster_id, - expected_pubkey = %metadata.public_key, - actual_pubkey = %validator_pubkey, - "Validator pubkey mismatch" - ); - return Err(ExecutionError::InvalidEvent( - "Validator does not match".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(cluster_id), + "Validator does not match", + None, )); } drop(state); @@ -445,15 +439,7 @@ impl EventProcessor { // Remove the validator and all corresponding cluster data self.db .delete_validator(&validator_pubkey, tx) - .map_err(|e| { - debug!( - cluster_id = ?cluster_id, - pubkey = ?validator_pubkey, - error = %e, - "Failed to delete valiidator from database" - ); - ExecutionError::Database(format!("Failed to validator cluster: {e}")) - })?; + .map_err(|e| ExecutionError::Database(format!("Failed to validator cluster: {e}")))?; debug!( cluster_id = ?cluster_id, @@ -479,15 +465,8 @@ impl EventProcessor { let cluster_id = compute_cluster_id(owner, operator_ids); - debug!(cluster_id = ?cluster_id, "Processing cluster liquidation"); - // Update the status of the cluster to be liquidated self.db.update_status(cluster_id, true, tx).map_err(|e| { - debug!( - cluster_id = ?cluster_id, - error = %e, - "Failed to mark cluster as liquidated" - ); ExecutionError::Database(format!("Failed to mark cluster as liquidated: {e}")) })?; @@ -518,15 +497,8 @@ impl EventProcessor { let cluster_id = compute_cluster_id(owner, operator_ids); - debug!(cluster_id = ?cluster_id, "Processing cluster reactivation"); - // Update the status of the cluster to be active self.db.update_status(cluster_id, false, tx).map_err(|e| { - debug!( - cluster_id = ?cluster_id, - error = %e, - "Failed to mark cluster as active" - ); ExecutionError::Database(format!("Failed to mark cluster as active: {e}")) })?; @@ -558,11 +530,6 @@ impl EventProcessor { self.db .update_fee_recipient(owner, recipientAddress, tx) .map_err(|e| { - debug!( - owner = ?owner, - error = %e, - "Failed to update fee recipient" - ); ExecutionError::Database(format!("Failed to update fee recipient: {e}")) })?; debug!( @@ -603,7 +570,7 @@ impl EventProcessor { let block_timestamp = log .block_timestamp - .ok_or_else(|| ExecutionError::InvalidEvent("Block timestamp not set".to_string()))?; + .ok_or_else(|| ExecutionError::invalid_event("Block timestamp not set", None))?; let validator_index = match self.get_validator_index(&validator_pubkey) { Ok(Some(value)) => value, @@ -666,12 +633,11 @@ impl EventProcessor { let validator_metadata = match self.db.state().metadata().get_by(validator_pubkey) { Some(metadata) => metadata, None => { - error!( - validator_pubkey = %validator_pubkey, - "Validator metadata not found" - ); - return Err(ExecutionError::InvalidEvent( - "Validator metadata not found".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + None, // No cluster_id available at this point + "Validator metadata not found", + None, )); } }; @@ -722,25 +688,21 @@ impl EventProcessor { let cluster = match state.clusters().get_by(validator_pubkey) { Some(cluster) => cluster, None => { - error!( - validator_pubkey = %validator_pubkey, - "Cluster not found for validator" - ); - return Err(ExecutionError::InvalidEvent( - "Cluster not found for validator".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(*computed_cluster_id), + "Cluster not found for validator", + None, )); } }; if cluster.cluster_id != *computed_cluster_id { - error!( - validator_pubkey = %validator_pubkey, - computed_cluster_id = ?computed_cluster_id, - cluster_id = ?cluster.cluster_id, - "Validator's cluster id is not the same as the computed cluster id" - ); - return Err(ExecutionError::InvalidEvent( - "Validator's cluster id is not the same as the computed cluster id".to_string(), + return Err(ExecutionError::invalid_cluster_event( + Some(*computed_cluster_id), + Some(*owner), + "Validator's cluster id is not the same as the computed cluster id", + None, )); } @@ -758,14 +720,11 @@ impl EventProcessor { // Verify that the owner from the contract event is the one who registered the validator // (which is stored as the cluster's owner in our database) if &cluster.owner != owner { - error!( - validator_pubkey = %validator_pubkey, - registered_owner = ?cluster.owner, - contract_event_owner = ?owner, - "Owner mismatch: the address in the contract event is not the validator's registered owner" - ); - return Err(ExecutionError::InvalidEvent( - "Contract event owner does not match the validator's registered owner".to_string(), + return Err(ExecutionError::invalid_validator_event( + Some(validator_pubkey.to_string()), + Some(*computed_cluster_id), + "Contract event owner does not match the validator's registered owner", + None, )); } diff --git a/anchor/eth/src/network_actions.rs b/anchor/eth/src/network_actions.rs index 1dcf569a2..76ccab544 100644 --- a/anchor/eth/src/network_actions.rs +++ b/anchor/eth/src/network_actions.rs @@ -30,6 +30,18 @@ pub enum NetworkAction { NoOp, } +// Simple wrapper to make String compatible with Error trait +#[derive(Debug)] +struct StringError(String); + +impl std::fmt::Display for StringError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::error::Error for StringError {} + /// Parse a network log into an action to be executed impl TryFrom<&Log> for NetworkAction { type Error = ExecutionError; @@ -41,7 +53,10 @@ impl TryFrom<&Log> for NetworkAction { SSVContract::ValidatorRemoved::decode_from_log(source)?; let validator_pubkey = PublicKeyBytes::from_str(&publicKey.to_string()).map_err(|e| { - ExecutionError::InvalidEvent(format!("Failed to create PublicKey: {e}")) + ExecutionError::invalid_event( + "Failed to create PublicKey", + Some(Box::new(StringError(e.to_string()))), + ) })?; Ok(NetworkAction::StopValidator { validator_pubkey }) } @@ -76,7 +91,10 @@ impl TryFrom<&Log> for NetworkAction { SSVContract::ValidatorExited::decode_from_log(source)?; let validator_pubkey = PublicKeyBytes::from_str(&publicKey.to_string()).map_err(|e| { - ExecutionError::InvalidEvent(format!("Failed to create PublicKey: {e}")) + ExecutionError::invalid_event( + "Failed to create PublicKey", + Some(Box::new(StringError(e.to_string()))), + ) })?; Ok(NetworkAction::ExitValidator { validator_pubkey }) } diff --git a/anchor/eth/src/sync.rs b/anchor/eth/src/sync.rs index 3c0f20d25..0c84c0826 100644 --- a/anchor/eth/src/sync.rs +++ b/anchor/eth/src/sync.rs @@ -130,7 +130,7 @@ impl SsvEventSyncer { exit_tx: ExitTx, config: Config, ) -> Result { - info!("Creating new SSV Event Syncer"); + debug!("Creating new SSV Event Syncer"); // Construct the rpc provider let rpc_client = http_with_timeout_and_fallback(&config.http_urls); @@ -576,9 +576,9 @@ impl SsvEventSyncer { continue; } - let block_number = log.block_number.ok_or_else(|| { - ExecutionError::InvalidEvent("Block number not available".to_string()) - })?; + let block_number = log + .block_number + .ok_or_else(|| ExecutionError::invalid_event("Block number not available", None))?; if let Some(timestamp) = block_timestamp_cache.get(&block_number) { log.block_timestamp = Some(*timestamp); @@ -595,7 +595,7 @@ impl SsvEventSyncer { block } Ok(None) => { - return Err(ExecutionError::InvalidEvent("Block not found".to_string())); + return Err(ExecutionError::invalid_event("Block not found", None)); } Err(e) => { return Err(ExecutionError::RpcError(format!( diff --git a/anchor/eth/src/util.rs b/anchor/eth/src/util.rs index 59538c5c4..791ff247c 100644 --- a/anchor/eth/src/util.rs +++ b/anchor/eth/src/util.rs @@ -10,17 +10,31 @@ use database::NetworkState; use reqwest::Client; use sensitive_url::SensitiveUrl; use ssv_types::{ClusterId, ENCRYPTED_KEY_LENGTH, OperatorId, Share, ValidatorMetadata}; +use thiserror::Error; use tower::ServiceBuilder; -use tracing::{debug, error}; +use tracing::debug; use types::{Graffiti, PublicKeyBytes, Signature}; -use crate::{error::ExecutionError, sync::MAX_OPERATORS}; +use crate::{error::ExecutionError, sync::MAX_OPERATORS, util::ShareParseError::InvalidLength}; // phase0.SignatureLength const SIGNATURE_LENGTH: usize = 96; // phase0.PublicKeyLength const PUBLIC_KEY_LENGTH: usize = 48; +/// Errors that can occur during share parsing +#[derive(Error, Debug)] +pub enum ShareParseError { + #[error("Share data has invalid length: expected {expected}, got {actual}")] + InvalidLength { expected: usize, actual: usize }, + + #[error("Failed to create public key: {0}")] + PublicKeyCreation(String), + + #[error("Encrypted key has wrong length")] + EncryptedKeyLength, +} + // Parses shares from a ValidatorAdded event // Event contains a bytes stream of the form // [signature | public keys | encrypted keys]. @@ -29,7 +43,7 @@ pub fn parse_shares( operator_ids: &[OperatorId], cluster_id: &ClusterId, validator_pubkey: &PublicKeyBytes, -) -> Result<(Vec, Vec), String> { +) -> Result<(Vec, Vec), ShareParseError> { let operator_count = operator_ids.len(); // Calculate offsets for different components within the shares @@ -39,11 +53,10 @@ pub fn parse_shares( // Validate total length of shares if shares_expected_length != shares.len() { - return Err(format!( - "Share data has invalid length: expected {}, got {}", - shares_expected_length, - shares.len() - )); + return Err(InvalidLength { + expected: shares_expected_length, + actual: shares.len(), + }); } // Extract all of the components @@ -66,12 +79,12 @@ pub fn parse_shares( // Create public key let share_pubkey = PublicKeyBytes::from_str(&public_key_hex) - .map_err(|e| format!("Failed to create public key: {e}"))?; + .map_err(ShareParseError::PublicKeyCreation)?; // Convert encrypted key into fixed array let encrypted_array: [u8; 256] = encrypted .try_into() - .map_err(|_| "Encrypted key has wrong length".to_string())?; + .map_err(|_| ShareParseError::EncryptedKeyLength)?; Ok(Share { validator_pubkey: *validator_pubkey, @@ -81,7 +94,7 @@ pub fn parse_shares( encrypted_private_key: encrypted_array, }) }) - .collect::, String>>()?; + .collect::, ShareParseError>>()?; Ok((signature, shares)) } @@ -146,36 +159,48 @@ pub fn validate_operators( cluster_id: &ClusterId, network_state: &NetworkState, ) -> Result<(), ExecutionError> { - debug!(cluster_id = ?cluster_id, "Validating operators"); - let num_operators = operator_ids.len(); // make sure there is a valid number of operators if num_operators > MAX_OPERATORS { - return Err(ExecutionError::InvalidEvent(format!( - "Failed to validate operators: validator has too many operators: {num_operators}" - ))); + return Err(ExecutionError::invalid_cluster_event( + Some(*cluster_id), + None, + format!( + "Failed to validate operators: validator has too many operators: {num_operators}" + ), + None, + )); } if num_operators == 0 { - return Err(ExecutionError::InvalidEvent( - "Failed to validate operators: validator has no operators".to_string(), + return Err(ExecutionError::invalid_cluster_event( + Some(*cluster_id), + None, + "Failed to validate operators: validator has no operators", + None, )); } // make sure count is valid let threshold = (num_operators - 1) / 3; if (num_operators - 1) % 3 != 0 || !(1..=4).contains(&threshold) { - return Err(ExecutionError::InvalidEvent(format!( - "Given {num_operators} operators. Cannot build a 3f+1 quorum" - ))); + return Err(ExecutionError::invalid_cluster_event( + Some(*cluster_id), + None, + format!("Given {num_operators} operators. Cannot build a 3f+1 quorum"), + None, + )); } // make sure there are no duplicates let mut seen = HashSet::new(); let are_duplicates = !operator_ids.iter().all(|x| seen.insert(x)); if are_duplicates { - return Err(ExecutionError::InvalidEvent( - "Operator IDs contain duplicates".to_string(), + return Err(ExecutionError::invalid_cluster_event( + Some(*cluster_id), + None, + "Operator IDs contain duplicates", + None, )); } @@ -183,7 +208,6 @@ pub fn validate_operators( .iter() .any(|id| !network_state.operator_exists(id)) { - error!(cluster_id = ?cluster_id, "One or more operators do not exist"); return Err(ExecutionError::Database( "One or more operators do not exist".to_string(), )); @@ -193,7 +217,7 @@ pub fn validate_operators( } /// Helper function to parse validator public keys -pub fn parse_validator_pubkey(pubkey: &Bytes) -> Result { +pub fn parse_validator_pubkey(pubkey: &Bytes) -> Result { let pubkey_str = pubkey.to_string(); PublicKeyBytes::from_str(&pubkey_str).map_err(|e| { debug!( @@ -201,7 +225,7 @@ pub fn parse_validator_pubkey(pubkey: &Bytes) -> Result