From 3d0923027de43bd0e042d753988fc02a12e6f572 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Wed, 18 Sep 2024 16:54:40 -0300 Subject: [PATCH] Improve error handling in fireblocks crate --- .../chainio/clients/fireblocks/src/client.rs | 17 +-- .../clients/fireblocks/src/contract_call.rs | 13 +- .../fireblocks/src/get_asset_addresses.rs | 18 +-- .../clients/fireblocks/src/get_transaction.rs | 17 +-- crates/chainio/clients/fireblocks/src/lib.rs | 112 +++++++----------- .../clients/fireblocks/src/list_contracts.rs | 27 ++--- .../fireblocks/src/list_external_accounts.rs | 20 +--- .../fireblocks/src/list_vault_accounts.rs | 16 +-- 8 files changed, 76 insertions(+), 164 deletions(-) diff --git a/crates/chainio/clients/fireblocks/src/client.rs b/crates/chainio/clients/fireblocks/src/client.rs index 0479b4f1..950bdd50 100644 --- a/crates/chainio/clients/fireblocks/src/client.rs +++ b/crates/chainio/clients/fireblocks/src/client.rs @@ -115,18 +115,11 @@ impl Client { body_hash, }; - let encoding_key_result = EncodingKey::from_rsa_pem(self.private_key.as_bytes()); - match encoding_key_result { - Ok(encoding_key) => { - let token_result = encode(&Header::new(Algorithm::RS256), &claims, &encoding_key); - - match token_result { - Ok(token) => Ok(token), - Err(e) => Err(FireBlockError::JsonWebTokenError(e)), - } - } - Err(e) => Err(FireBlockError::JsonWebTokenError(e)), - } + let encoding_key = EncodingKey::from_rsa_pem(self.private_key.as_bytes()) + .map_err(FireBlockError::JsonWebTokenError)?; + + encode(&Header::new(Algorithm::RS256), &claims, &encoding_key) + .map_err(FireBlockError::JsonWebTokenError) } /// GET : Request to the fireblocks endpoint using the given path. diff --git a/crates/chainio/clients/fireblocks/src/contract_call.rs b/crates/chainio/clients/fireblocks/src/contract_call.rs index 2f1cfa31..5f1f6873 100644 --- a/crates/chainio/clients/fireblocks/src/contract_call.rs +++ b/crates/chainio/clients/fireblocks/src/contract_call.rs @@ -124,17 +124,10 @@ impl ContractCall for Client { serde_json::to_string(&transaction_request).map_err(FireBlockError::SerdeError)?; // Call POST request - let contract_call_result = self + let contract_call = self .post_request("/v1/transactions/", Some(&transaction_json.to_string())) - .await; - match contract_call_result { - Ok(contract_call) => { - let contract_call_req: TransactionResponse = - serde_json::from_str(&contract_call).unwrap(); - Ok(contract_call_req) - } - Err(e) => Err(e), - } + .await?; + serde_json::from_str(&contract_call).map_err(FireBlockError::SerdeError) } } diff --git a/crates/chainio/clients/fireblocks/src/get_asset_addresses.rs b/crates/chainio/clients/fireblocks/src/get_asset_addresses.rs index dbb39f75..01c759ef 100644 --- a/crates/chainio/clients/fireblocks/src/get_asset_addresses.rs +++ b/crates/chainio/clients/fireblocks/src/get_asset_addresses.rs @@ -41,26 +41,14 @@ impl GetAssetAddresses for Client { vault_id: String, asset_id: AssetID, ) -> Result { - let asset_addresses_result = self + let asset_addresses = self .get_request(&format!( "/v1/vault/accounts/{}/{}/addresses_paginated", vault_id, asset_id )) - .await; + .await?; - match asset_addresses_result { - Ok(asset_addresses) => { - let asset_address_result: Result = - serde_json::from_str(&asset_addresses); - - match asset_address_result { - Ok(asset_address) => Ok(asset_address), - - Err(e) => Err(FireBlockError::SerdeError(e)), - } - } - Err(e) => Err(e), - } + serde_json::from_str(&asset_addresses).map_err(FireBlockError::SerdeError) } } diff --git a/crates/chainio/clients/fireblocks/src/get_transaction.rs b/crates/chainio/clients/fireblocks/src/get_transaction.rs index 58062f4b..0cbb6705 100644 --- a/crates/chainio/clients/fireblocks/src/get_transaction.rs +++ b/crates/chainio/clients/fireblocks/src/get_transaction.rs @@ -89,22 +89,11 @@ pub trait GetTransaction { impl GetTransaction for Client { async fn get_transaction(&self, tx_id: String) -> Result { - let transaction_object_result = self + let transaction = self .get_request(&format!("/v1/transactions/{}", tx_id)) - .await; + .await?; - match transaction_object_result { - Ok(transaction) => { - let serialized_tx_result: Result = - serde_json::from_str(&transaction); - - match serialized_tx_result { - Ok(serialized_tx) => Ok(serialized_tx), - Err(e) => Err(FireBlockError::SerdeError(e)), - } - } - Err(e) => Err(e), - } + serde_json::from_str(&transaction).map_err(FireBlockError::SerdeError) } } diff --git a/crates/chainio/clients/fireblocks/src/lib.rs b/crates/chainio/clients/fireblocks/src/lib.rs index 9a4a4582..034514d0 100644 --- a/crates/chainio/clients/fireblocks/src/lib.rs +++ b/crates/chainio/clients/fireblocks/src/lib.rs @@ -53,43 +53,35 @@ impl FireblocksWallet { vault_account_name: String, ) -> Result { let provider_ = get_provider(&provider); - let chain_id_result = provider_.get_chain_id().await; + let chain_id = provider_.get_chain_id().await.map_err(|e| { + FireBlockError::AlloyContractError(alloy_contract::Error::TransportError(e)) + })?; - match chain_id_result { - Ok(chain_id) => Ok(Self { - fireblocks_client, - vault_account_name, - chain_id, - provider, - vault_account: None, - whitelisted_accounts: HashMap::new(), - whitelisted_contracts: HashMap::new(), - tx_id_to_nonce: HashMap::new(), - }), - Err(e) => Err(FireBlockError::AlloyContractError( - alloy_contract::Error::TransportError(e), - )), - } + Ok(Self { + fireblocks_client, + vault_account_name, + chain_id, + provider, + vault_account: None, + whitelisted_accounts: HashMap::new(), + whitelisted_contracts: HashMap::new(), + tx_id_to_nonce: HashMap::new(), + }) } /// Get Vault Accounts pub async fn get_account(&mut self) -> Result, FireBlockError> { match &self.vault_account { None => { - let accounts_result = self.fireblocks_client.list_vault_accounts().await; + let accounts = self.fireblocks_client.list_vault_accounts().await?; - match accounts_result { - Ok(accounts) => { - for account in accounts.vault_accounts().iter() { - if account.name().eq(&self.vault_account_name) { - self.vault_account = Some(account.clone()); - break; - } - } - Ok(self.vault_account.clone()) + for account in accounts.vault_accounts().iter() { + if account.name().eq(&self.vault_account_name) { + self.vault_account = Some(account.clone()); + break; } - Err(e) => Err(e), } + Ok(self.vault_account.clone()) } Some(account) => Ok(Some(account.clone())), } @@ -114,29 +106,23 @@ impl FireblocksWallet { } false => { let whitelisted_accounts; - let accounts_result = self.fireblocks_client.list_external_accounts().await; + let accounts = self.fireblocks_client.list_external_accounts().await?; - match accounts_result { - Ok(accounts) => { - for account in accounts.iter() { - for asset in account.assets.iter() { - if asset.address.as_ref().unwrap().eq(&address.to_string()) - && asset.status.as_ref().unwrap().as_str() == "APPROVED" - && *asset.id.as_ref().unwrap() - == *ASSET_ID_BY_CHAIN.get(&self.chain_id).unwrap() - { - self.whitelisted_accounts - .insert(address, account.clone()); - whitelisted_accounts = account; - return Ok(whitelisted_accounts.clone()); - } - } + for account in accounts.iter() { + for asset in account.assets.iter() { + if asset.address.as_ref().unwrap().eq(&address.to_string()) + && asset.status.as_ref().unwrap().as_str() == "APPROVED" + && *asset.id.as_ref().unwrap() + == *ASSET_ID_BY_CHAIN.get(&self.chain_id).unwrap() + { + self.whitelisted_accounts.insert(address, account.clone()); + whitelisted_accounts = account; + return Ok(whitelisted_accounts.clone()); } - - Ok(WhitelistedAccount::default()) } - Err(e) => Err(e), } + + Ok(WhitelistedAccount::default()) } } } @@ -203,34 +189,24 @@ impl FireblocksWallet { .fireblocks_client .get_transaction(tx_id.clone()) .await?; + match fireblocks_tx.status() { Status::Completed => { let provider = get_provider(&self.provider); - let hash_result = - alloy_primitives::FixedBytes::<32>::from_str(&fireblocks_tx.tx_hash()); - match hash_result { - Ok(hash) => { - let tx_hash_result = provider.get_transaction_receipt(hash).await; + let hash = alloy_primitives::FixedBytes::<32>::from_str(&fireblocks_tx.tx_hash()) + .map_err(|e| FireBlockError::OtherError(e.to_string()))?; - match tx_hash_result { - Ok(tx_hash) => { - if let Some(tx) = tx_hash { - if self.tx_id_to_nonce.contains_key(&tx_id) { - self.tx_id_to_nonce.remove(&tx_id); - } - Ok(tx) - } else { - Err(FireBlockError::TransactionReceiptNotFound(tx_id)) - } - } - Err(e) => Err(FireBlockError::AlloyContractError( - alloy_contract::Error::TransportError(e), - )), - } - } + let tx_hash = provider.get_transaction_receipt(hash).await.map_err(|e| { + FireBlockError::AlloyContractError(alloy_contract::Error::TransportError(e)) + })?; + + let tx = + tx_hash.ok_or(FireBlockError::TransactionReceiptNotFound(tx_id.clone()))?; - Err(e) => Err(FireBlockError::OtherError(e.to_string())), + if self.tx_id_to_nonce.contains_key(&tx_id) { + self.tx_id_to_nonce.remove(&tx_id); } + Ok(tx) } Status::Failed | Status::Rejected | Status::Cancelled | Status::Blocked => { Err(FireBlockError::TransactionFailed( diff --git a/crates/chainio/clients/fireblocks/src/list_contracts.rs b/crates/chainio/clients/fireblocks/src/list_contracts.rs index 9eb07347..6689fad1 100644 --- a/crates/chainio/clients/fireblocks/src/list_contracts.rs +++ b/crates/chainio/clients/fireblocks/src/list_contracts.rs @@ -53,23 +53,18 @@ pub trait ListContracts { impl ListContracts for Client { async fn list_contracts(&self) -> Result { - let list_contracts_result = self.get_request("/v1/contracts").await; - match list_contracts_result { - Ok(list_contracts_object) => { - if list_contracts_object.trim() == "[]" { - return Ok(WhitelistedContractResponse { - contracts: Vec::new(), - }); - } - let serialized_tx: Result, _> = - serde_json::from_str(&list_contracts_object); - match serialized_tx { - Ok(contracts) => Ok(WhitelistedContractResponse { contracts }), - Err(e) => Err(FireBlockError::SerdeError(e)), - } - } - Err(e) => Err(e), + let list_contracts_object = self.get_request("/v1/contracts").await?; + + if list_contracts_object.trim() == "[]" { + return Ok(WhitelistedContractResponse { + contracts: Vec::new(), + }); } + + let contracts: Vec = + serde_json::from_str(&list_contracts_object).map_err(FireBlockError::SerdeError)?; + + Ok(WhitelistedContractResponse { contracts }) } } diff --git a/crates/chainio/clients/fireblocks/src/list_external_accounts.rs b/crates/chainio/clients/fireblocks/src/list_external_accounts.rs index b2f9ff2b..9eeb0f8f 100644 --- a/crates/chainio/clients/fireblocks/src/list_external_accounts.rs +++ b/crates/chainio/clients/fireblocks/src/list_external_accounts.rs @@ -24,23 +24,13 @@ pub trait ListExternalAccounts { impl ListExternalAccounts for Client { async fn list_external_accounts(&self) -> Result, FireBlockError> { - let list_external_accounts_result = self.get_request("/v1/external_wallets").await; + let list_external_accounts = self.get_request("/v1/external_wallets").await?; - match list_external_accounts_result { - Ok(list_external_accounts) => { - if list_external_accounts.trim() == "[]" { - let default_accounts = vec![WhitelistedAccount::default()]; - return Ok(default_accounts); - } - let serialized_tx: Result, _> = - serde_json::from_str(&list_external_accounts); - match serialized_tx { - Ok(whitelisted_accounts) => Ok(whitelisted_accounts), - Err(e) => Err(FireBlockError::SerdeError(e)), - } - } - Err(e) => Err(e), + if list_external_accounts.trim() == "[]" { + let default_accounts = vec![WhitelistedAccount::default()]; + return Ok(default_accounts); } + serde_json::from_str(&list_external_accounts).map_err(FireBlockError::SerdeError) } } diff --git a/crates/chainio/clients/fireblocks/src/list_vault_accounts.rs b/crates/chainio/clients/fireblocks/src/list_vault_accounts.rs index be296d5c..0be38b72 100644 --- a/crates/chainio/clients/fireblocks/src/list_vault_accounts.rs +++ b/crates/chainio/clients/fireblocks/src/list_vault_accounts.rs @@ -61,20 +61,8 @@ pub trait ListVaultAccounts { impl ListVaultAccounts for Client { async fn list_vault_accounts(&self) -> Result { - let list_vault_accounts_result = self.get_request("/v1/vault/accounts_paged").await; - - match list_vault_accounts_result { - Ok(list_vault_accounts) => { - let vault_accounts_result: Result = - serde_json::from_str(&list_vault_accounts); - - match vault_accounts_result { - Ok(vault_accounts) => Ok(vault_accounts), - Err(e) => Err(FireBlockError::SerdeError(e)), - } - } - Err(e) => Err(e), - } + let list_vault_accounts = self.get_request("/v1/vault/accounts_paged").await?; + serde_json::from_str(&list_vault_accounts).map_err(FireBlockError::SerdeError) } }