From 00bd4713b9194f5b9753902de242941190765c88 Mon Sep 17 00:00:00 2001 From: Daniel Savu <23065004+daniel-savu@users.noreply.github.com> Date: Wed, 1 Nov 2023 11:30:36 +0000 Subject: [PATCH] fix: progress on removing unwraps (#2880) ### Description ### Drive-by changes ### Related issues ### Backward compatibility ### Testing --- rust/Cargo.lock | 1 + .../hyperlane-cosmos/src/interchain_gas.rs | 43 +++++++----------- rust/chains/hyperlane-cosmos/src/mailbox.rs | 45 ++++++++++--------- .../hyperlane-cosmos/src/merkle_tree_hook.rs | 36 ++++++--------- .../hyperlane-cosmos/src/providers/rpc.rs | 14 +++--- rust/hyperlane-core/Cargo.toml | 1 + rust/hyperlane-core/src/error.rs | 30 ++++++++----- rust/hyperlane-core/src/types/mod.rs | 2 +- 8 files changed, 84 insertions(+), 88 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 93b8e212c1..7bd66adcd7 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -4109,6 +4109,7 @@ version = "0.1.0" dependencies = [ "async-trait", "auto_impl 1.1.0", + "base64 0.21.4", "borsh 0.9.3", "bs58 0.5.0", "bytes", diff --git a/rust/chains/hyperlane-cosmos/src/interchain_gas.rs b/rust/chains/hyperlane-cosmos/src/interchain_gas.rs index 34b881cf19..0b32b519b9 100644 --- a/rust/chains/hyperlane-cosmos/src/interchain_gas.rs +++ b/rust/chains/hyperlane-cosmos/src/interchain_gas.rs @@ -67,15 +67,11 @@ impl CosmosInterchainGasPaymasterIndexer { } } - fn get_parser(&self) -> fn(attrs: Vec) -> Option { - |attrs: Vec| -> Option { - let mut res = InterchainGasPayment { - message_id: H256::zero(), - payment: U256::zero(), - gas_amount: U256::zero(), - destination: 0, - }; - + fn get_parser( + &self, + ) -> fn(attrs: Vec) -> ChainResult> { + |attrs: Vec| -> ChainResult> { + let mut res = InterchainGasPayment::default(); for attr in attrs { let key = attr.key.as_str(); let value = attr.value; @@ -83,41 +79,34 @@ impl CosmosInterchainGasPaymasterIndexer { match key { "message_id" => { - res.message_id = H256::from_slice(hex::decode(value).unwrap().as_slice()) + res.message_id = H256::from_slice(hex::decode(value)?.as_slice()) } "bWVzc2FnZV9pZA==" => { res.message_id = H256::from_slice( - hex::decode( - String::from_utf8(STANDARD.decode(value).unwrap()).unwrap(), - ) - .unwrap() - .as_slice(), + hex::decode(String::from_utf8(STANDARD.decode(value)?)?)?.as_slice(), ) } - "payment" => res.payment = value.parse().unwrap(), + "payment" => res.payment = value.parse()?, "cGF5bWVudA==" => { - let dec_str = String::from_utf8(STANDARD.decode(value).unwrap()).unwrap(); + let dec_str = String::from_utf8(STANDARD.decode(value)?)?; // U256's from_str assumes a radix of 16, so we explicitly use from_dec_str. - res.payment = U256::from_dec_str(dec_str.as_str()).unwrap(); + res.payment = U256::from_dec_str(dec_str.as_str())?; } - "gas_amount" => res.gas_amount = value.parse().unwrap(), + "gas_amount" => res.gas_amount = value.parse()?, "Z2FzX2Ftb3VudA==" => { - let dec_str = String::from_utf8(STANDARD.decode(value).unwrap()).unwrap(); + let dec_str = String::from_utf8(STANDARD.decode(value)?)?; // U256's from_str assumes a radix of 16, so we explicitly use from_dec_str. - res.gas_amount = U256::from_dec_str(dec_str.as_str()).unwrap(); + res.gas_amount = U256::from_dec_str(dec_str.as_str())?; } - "dest_domain" => res.destination = value.parse().unwrap(), + "dest_domain" => res.destination = value.parse()?, "ZGVzdF9kb21haW4=" => { - res.destination = String::from_utf8(STANDARD.decode(value).unwrap()) - .unwrap() - .parse() - .unwrap() + res.destination = String::from_utf8(STANDARD.decode(value)?)?.parse()? } _ => {} } } - Some(res) + Ok(Some(res)) } } } diff --git a/rust/chains/hyperlane-cosmos/src/mailbox.rs b/rust/chains/hyperlane-cosmos/src/mailbox.rs index 690c2375cb..2c0c0eb9ad 100644 --- a/rust/chains/hyperlane-cosmos/src/mailbox.rs +++ b/rust/chains/hyperlane-cosmos/src/mailbox.rs @@ -24,7 +24,9 @@ use hyperlane_core::{ HyperlaneMessage, HyperlaneProvider, Indexer, LogMeta, Mailbox, TxCostEstimate, TxOutcome, H256, U256, }; -use hyperlane_core::{ContractLocator, Decode, RawHyperlaneMessage, SequenceIndexer}; +use hyperlane_core::{ + ChainCommunicationError, ContractLocator, Decode, RawHyperlaneMessage, SequenceIndexer, +}; use tracing::{instrument, warn}; /// A reference to a Mailbox contract on some Cosmos chain @@ -171,7 +173,7 @@ impl Mailbox for CosmosMailbox { .await?; Ok(TxOutcome { transaction_id: h256_to_h512(H256::from_slice( - hex::decode(response.txhash).unwrap().as_slice(), + hex::decode(response.txhash)?.as_slice(), )), executed: response.code == 0, gas_used: U256::from(response.gas_used), @@ -194,7 +196,14 @@ impl Mailbox for CosmosMailbox { let response: SimulateResponse = self.provider.wasm_simulate(process_message).await?; let result = TxCostEstimate { - gas_limit: U256::from(response.gas_info.unwrap().gas_used), + gas_limit: U256::from( + response + .gas_info + .ok_or(ChainCommunicationError::TxCostEstimateError( + "Failed to estimate gas limit".to_string(), + ))? + .gas_used, + ), gas_price: U256::from(2500), l2_gas_limit: None, }; @@ -250,8 +259,10 @@ impl CosmosMailboxIndexer { } } - fn get_parser(&self) -> fn(attrs: Vec) -> Option { - |attrs: Vec| -> Option { + fn get_parser( + &self, + ) -> fn(attrs: Vec) -> ChainResult> { + |attrs: Vec| -> ChainResult> { let res = HyperlaneMessage::default(); for attr in attrs { @@ -260,26 +271,18 @@ impl CosmosMailboxIndexer { let value = value.as_str(); if key == "message" { - let mut reader = Cursor::new(hex::decode(value).unwrap()); - return Some(HyperlaneMessage::read_from(&mut reader).unwrap()); + let mut reader = Cursor::new(hex::decode(value)?); + return Ok(Some(HyperlaneMessage::read_from(&mut reader)?)); } if key == "bWVzc2FnZQ==" { - let mut reader = Cursor::new( - hex::decode( - String::from_utf8( - base64::engine::general_purpose::STANDARD - .decode(value) - .unwrap(), - ) - .unwrap(), - ) - .unwrap(), - ); - return Some(HyperlaneMessage::read_from(&mut reader).unwrap()); + let mut reader = Cursor::new(hex::decode(String::from_utf8( + base64::engine::general_purpose::STANDARD.decode(value)?, + )?)?); + return Ok(Some(HyperlaneMessage::read_from(&mut reader)?)); } } - None + Ok(None) } } } @@ -304,7 +307,7 @@ impl Indexer for CosmosMailboxIndexer { #[async_trait] impl Indexer for CosmosMailboxIndexer { async fn fetch_logs(&self, range: RangeInclusive) -> ChainResult> { - let parser: fn(Vec) -> Option = self.get_parser(); + let parser = self.get_parser(); let result = self.indexer.get_range_event_logs(range, parser).await?; Ok(result diff --git a/rust/chains/hyperlane-cosmos/src/merkle_tree_hook.rs b/rust/chains/hyperlane-cosmos/src/merkle_tree_hook.rs index d6374b65fb..e6295ac6c1 100644 --- a/rust/chains/hyperlane-cosmos/src/merkle_tree_hook.rs +++ b/rust/chains/hyperlane-cosmos/src/merkle_tree_hook.rs @@ -178,8 +178,10 @@ impl CosmosMerkleeTreeHookIndexer { } /// Get the parser for the indexer - fn get_parser(&self) -> fn(attrs: Vec) -> Option { - |attrs: Vec| -> Option { + fn get_parser( + &self, + ) -> fn(attrs: Vec) -> ChainResult> { + |attrs: Vec| -> ChainResult> { let mut message_id = H256::zero(); let mut leaf_index: u32 = 0; let mut attr_count = 0; @@ -191,35 +193,25 @@ impl CosmosMerkleeTreeHookIndexer { match key { "message_id" => { - message_id = H256::from_slice(hex::decode(value).unwrap().as_slice()); + message_id = H256::from_slice(hex::decode(value)?.as_slice()); attr_count += 1; } "index" => { - leaf_index = value.parse().unwrap(); + leaf_index = value.parse::()?; attr_count += 1; } "aW5kZXg=" => { leaf_index = String::from_utf8( - base64::engine::general_purpose::STANDARD - .decode(value) - .unwrap(), - ) - .unwrap() - .parse() - .unwrap(); + base64::engine::general_purpose::STANDARD.decode(value)?, + )? + .parse()?; attr_count += 1; } "bWVzc2FnZV9pZA==" => { message_id = H256::from_slice( - hex::decode( - String::from_utf8( - base64::engine::general_purpose::STANDARD - .decode(value) - .unwrap(), - ) - .unwrap(), - ) - .unwrap() + hex::decode(String::from_utf8( + base64::engine::general_purpose::STANDARD.decode(value)?, + )?)? .as_slice(), ); attr_count += 1; @@ -229,10 +221,10 @@ impl CosmosMerkleeTreeHookIndexer { } if attr_count != 2 { - return None; + return Ok(None); } - Some(MerkleTreeInsertion::new(leaf_index, message_id)) + Ok(Some(MerkleTreeInsertion::new(leaf_index, message_id))) } } } diff --git a/rust/chains/hyperlane-cosmos/src/providers/rpc.rs b/rust/chains/hyperlane-cosmos/src/providers/rpc.rs index 2ae6473b30..540434c592 100644 --- a/rust/chains/hyperlane-cosmos/src/providers/rpc.rs +++ b/rust/chains/hyperlane-cosmos/src/providers/rpc.rs @@ -26,7 +26,7 @@ pub trait WasmIndexer: Send + Sync { async fn get_range_event_logs( &self, range: RangeInclusive, - parser: fn(Vec) -> Option, + parser: fn(Vec) -> ChainResult>, ) -> ChainResult> where T: Send + Sync; @@ -93,7 +93,7 @@ impl WasmIndexer for CosmosWasmIndexer { async fn get_range_event_logs( &self, range: RangeInclusive, - parser: fn(Vec) -> Option, + parser: fn(Vec) -> ChainResult>, ) -> ChainResult> where T: Send + Sync, @@ -120,7 +120,7 @@ impl WasmIndexer for CosmosWasmIndexer { let last_page = total_count / PAGINATION_LIMIT as u32 + (total_count % PAGINATION_LIMIT as u32 != 0) as u32; - let handler = |txs: Vec| -> Vec<(T, LogMeta)> { + let handler = |txs: Vec| -> ChainResult> { let mut result: Vec<(T, LogMeta)> = vec![]; let target_type = format!("{}-{}", Self::WASM_TYPE, self.event_type); @@ -137,7 +137,7 @@ impl WasmIndexer for CosmosWasmIndexer { for (log_idx, event) in tx.tx_result.events.clone().into_iter().enumerate() { if event.kind.as_str() == target_type { - if let Some(msg) = parser(event.attributes.clone()) { + if let Some(msg) = parser(event.attributes.clone())? { let meta = LogMeta { address: bech32_decode(contract_address.clone()), block_number: tx.height.value(), @@ -156,10 +156,10 @@ impl WasmIndexer for CosmosWasmIndexer { result.extend(parse_result); } - result + Ok(result) }; - let mut result = handler(tx_search_result.txs); + let mut result = handler(tx_search_result.txs)?; for page in 2..=last_page { debug!(page, "Making tx search RPC"); @@ -174,7 +174,7 @@ impl WasmIndexer for CosmosWasmIndexer { ) .await?; - result.extend(handler(tx_search_result.txs)); + result.extend(handler(tx_search_result.txs)?); } Ok(result) diff --git a/rust/hyperlane-core/Cargo.toml b/rust/hyperlane-core/Cargo.toml index 2730f8f422..f07417da89 100644 --- a/rust/hyperlane-core/Cargo.toml +++ b/rust/hyperlane-core/Cargo.toml @@ -12,6 +12,7 @@ version = { workspace = true } [dependencies] async-trait.workspace = true auto_impl.workspace = true +base64 = { workspace = true } borsh.workspace = true bs58.workspace = true bytes = { workspace = true, features = ["serde"] } diff --git a/rust/hyperlane-core/src/error.rs b/rust/hyperlane-core/src/error.rs index 6c0f705aa5..a69ee46d97 100644 --- a/rust/hyperlane-core/src/error.rs +++ b/rust/hyperlane-core/src/error.rs @@ -6,9 +6,7 @@ use std::ops::Deref; use crate::config::StrOrIntParseError; use cosmrs::proto::prost; use cosmrs::Error as CosmrsError; -// use ethers_contract::ContractError; -// use ethers_core::types::SignatureError; -// use ethers_providers::{Middleware, ProviderError}; +use std::string::FromUtf8Error; use crate::HyperlaneProviderError; use crate::H256; @@ -105,12 +103,27 @@ pub enum ChainCommunicationError { /// protobuf error #[error("{0}")] Protobuf(#[from] prost::DecodeError), + /// base64 error + #[error("{0}")] + Base64(#[from] base64::DecodeError), + /// utf8 error + #[error("{0}")] + Utf8(#[from] FromUtf8Error), /// Serde JSON error #[error("{0}")] JsonParseError(#[from] serde_json::Error), - /// Hex parse error + /// String hex parsing error #[error("{0}")] HexParseError(#[from] hex::FromHexError), + /// Uint hex parsing error + #[error("{0}")] + UintParseError(#[from] uint::FromHexError), + /// Decimal string parsing error + #[error("{0}")] + FromDecStrError(#[from] uint::FromDecStrErr), + /// Int string parsing error + #[error("{0}")] + ParseIntError(#[from] std::num::ParseIntError), /// Invalid Request #[error("Invalid Request: {msg:?}")] InvalidRequest { @@ -123,12 +136,9 @@ pub enum ChainCommunicationError { /// Error message msg: String, }, - /// Not match connection type - #[error("Not match connection type: require {msg:?}")] - NotMatchConnectionType { - /// Error message - msg: String, - }, + /// Failed to estimate transaction gas cost. + #[error("Failed to estimate transaction gas cost {0}")] + TxCostEstimateError(String), } impl ChainCommunicationError { diff --git a/rust/hyperlane-core/src/types/mod.rs b/rust/hyperlane-core/src/types/mod.rs index 4ef1f4411b..b6bdf50a25 100644 --- a/rust/hyperlane-core/src/types/mod.rs +++ b/rust/hyperlane-core/src/types/mod.rs @@ -115,7 +115,7 @@ pub struct GasPaymentKey { } /// A payment of a message's gas costs. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, Default)] pub struct InterchainGasPayment { /// Id of the message pub message_id: H256,