refactor: simplify EthRpcClient (#278)

Follow-up on #275 to simplify `EthRpcClient`: 1. simplify instantiation by removing intermediate methods between `CandidRpcClient::new` (the caller) and `EthRpcClient::new` 2. Change the field `EthRpcClient::providers` to be non-optional, since any instantiation of the client checks that the given providers are not empty. 3. Remove the trait `RpcTransport` and the dependency on `async-trait`, since a single implementation was used --------- Co-authored-by: rvanasa <ryan.vandersmith@dfinity.org>
dfinity · Sep 18, 2024 · 557cd56 · 557cd56
1 parent 48b8f3d
commit 557cd56
Show file tree

Hide file tree

Showing 8 changed files with 163 additions and 261 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -43,10 +43,9 @@ serde = { workspace = true }
 serde_json = { workspace = true }
 thousands = "0.2"
 url = "2.5"
-async-trait = "0.1"
 hex = "0.4"
 ethers-core = "2.0"
-zeroize = {version = "1.8", features = ["zeroize_derive"]}
+zeroize = { version = "1.8", features = ["zeroize_derive"] }
 
 [dev-dependencies]
 assert_matches = "1.5"

diff --git a/src/candid_rpc.rs b/src/candid_rpc.rs
@@ -1,79 +1,15 @@
 mod cketh_conversion;
 
-use async_trait::async_trait;
-use candid::Nat;
-use ethers_core::{types::Transaction, utils::rlp};
-use evm_rpc_types::{
-    Hex, Hex32, MultiRpcResult, ProviderError, RpcApi, RpcError, RpcResult, RpcService,
-    RpcServices, ValidationError,
-};
-use ic_cdk::api::management_canister::http_request::{CanisterHttpRequestArgument, HttpResponse};
-
-use crate::constants::{
-    DEFAULT_ETH_MAINNET_SERVICES, DEFAULT_ETH_SEPOLIA_SERVICES, DEFAULT_L2_MAINNET_SERVICES,
-};
-use crate::rpc_client::{EthRpcClient, MultiCallError, RpcTransport};
+use crate::rpc_client::{EthRpcClient, MultiCallError};
 use crate::{
-    accounting::get_http_request_cost,
     add_metric_entry,
     constants::ETH_GET_LOGS_MAX_BLOCKS,
-    http::http_request,
     providers::resolve_rpc_service,
-    types::{MetricRpcHost, MetricRpcMethod, ResolvedRpcService, RpcMethod},
+    types::{MetricRpcHost, ResolvedRpcService, RpcMethod},
 };
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-struct CanisterTransport;
-
-#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
-#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
-impl RpcTransport for CanisterTransport {
-    fn resolve_api(service: &RpcService) -> Result<RpcApi, ProviderError> {
-        Ok(resolve_rpc_service(service.clone())?.api())
-    }
-
-    async fn http_request(
-        service: &RpcService,
-        method: &str,
-        request: CanisterHttpRequestArgument,
-        effective_response_size_estimate: u64,
-    ) -> Result<HttpResponse, RpcError> {
-        let service = resolve_rpc_service(service.clone())?;
-        let cycles_cost = get_http_request_cost(
-            request
-                .body
-                .as_ref()
-                .map(|bytes| bytes.len() as u64)
-                .unwrap_or_default(),
-            effective_response_size_estimate,
-        );
-        let rpc_method = MetricRpcMethod(method.to_string());
-        http_request(rpc_method, service, request, cycles_cost).await
-    }
-}
-
-fn check_services<T>(services: Vec<T>) -> RpcResult<Vec<T>> {
-    if services.is_empty() {
-        Err(ProviderError::ProviderNotFound)?;
-    }
-    Ok(services)
-}
-
-fn get_rpc_client(
-    source: RpcServices,
-    config: evm_rpc_types::RpcConfig,
-) -> RpcResult<EthRpcClient<CanisterTransport>> {
-    use crate::candid_rpc::cketh_conversion::{into_ethereum_network, into_rpc_services};
-
-    let chain = into_ethereum_network(&source);
-    let providers = check_services(into_rpc_services(
-        source,
-        DEFAULT_ETH_MAINNET_SERVICES,
-        DEFAULT_ETH_SEPOLIA_SERVICES,
-        DEFAULT_L2_MAINNET_SERVICES,
-    ))?;
-    Ok(EthRpcClient::new(chain, Some(providers), config))
-}
+use candid::Nat;
+use ethers_core::{types::Transaction, utils::rlp};
+use evm_rpc_types::{Hex, Hex32, MultiRpcResult, RpcResult, ValidationError};
 
 fn process_result<T>(method: RpcMethod, result: Result<T, MultiCallError<T>>) -> MultiRpcResult<T> {
     match result {
@@ -106,7 +42,7 @@ fn process_result<T>(method: RpcMethod, result: Result<T, MultiCallError<T>>) ->
 }
 
 pub struct CandidRpcClient {
-    client: EthRpcClient<CanisterTransport>,
+    client: EthRpcClient,
 }
 
 impl CandidRpcClient {
@@ -115,7 +51,7 @@ impl CandidRpcClient {
         config: Option<evm_rpc_types::RpcConfig>,
     ) -> RpcResult<Self> {
         Ok(Self {
-            client: get_rpc_client(source, config.unwrap_or_default())?,
+            client: EthRpcClient::new(source, config)?,
         })
     }
 
@@ -232,7 +168,7 @@ fn get_transaction_hash(raw_signed_transaction_hex: &Hex) -> Option<Hex32> {
 mod test {
     use super::*;
     use crate::rpc_client::{MultiCallError, MultiCallResults};
-    use evm_rpc_types::RpcError;
+    use evm_rpc_types::{ProviderError, RpcError};
 
     #[test]
     fn test_process_result_mapping() {

diff --git a/src/candid_rpc/cketh_conversion.rs b/src/candid_rpc/cketh_conversion.rs
@@ -182,65 +182,6 @@ pub(super) fn from_send_raw_transaction_result(
     }
 }
 
-pub(super) fn into_ethereum_network(
-    source: &evm_rpc_types::RpcServices,
-) -> crate::rpc_client::EthereumNetwork {
-    match &source {
-        evm_rpc_types::RpcServices::Custom { chain_id, .. } => {
-            crate::rpc_client::EthereumNetwork::from(*chain_id)
-        }
-        evm_rpc_types::RpcServices::EthMainnet(_) => crate::rpc_client::EthereumNetwork::MAINNET,
-        evm_rpc_types::RpcServices::EthSepolia(_) => crate::rpc_client::EthereumNetwork::SEPOLIA,
-        evm_rpc_types::RpcServices::ArbitrumOne(_) => crate::rpc_client::EthereumNetwork::ARBITRUM,
-        evm_rpc_types::RpcServices::BaseMainnet(_) => crate::rpc_client::EthereumNetwork::BASE,
-        evm_rpc_types::RpcServices::OptimismMainnet(_) => {
-            crate::rpc_client::EthereumNetwork::OPTIMISM
-        }
-    }
-}
-
-pub(super) fn into_rpc_services(
-    source: evm_rpc_types::RpcServices,
-    default_eth_mainnet_services: &[evm_rpc_types::EthMainnetService],
-    default_eth_sepolia_services: &[evm_rpc_types::EthSepoliaService],
-    default_l2_mainnet_services: &[evm_rpc_types::L2MainnetService],
-) -> Vec<evm_rpc_types::RpcService> {
-    match source {
-        evm_rpc_types::RpcServices::Custom {
-            chain_id: _,
-            services,
-        } => services
-            .into_iter()
-            .map(evm_rpc_types::RpcService::Custom)
-            .collect(),
-        evm_rpc_types::RpcServices::EthMainnet(services) => services
-            .unwrap_or_else(|| default_eth_mainnet_services.to_vec())
-            .into_iter()
-            .map(evm_rpc_types::RpcService::EthMainnet)
-            .collect(),
-        evm_rpc_types::RpcServices::EthSepolia(services) => services
-            .unwrap_or_else(|| default_eth_sepolia_services.to_vec())
-            .into_iter()
-            .map(evm_rpc_types::RpcService::EthSepolia)
-            .collect(),
-        evm_rpc_types::RpcServices::ArbitrumOne(services) => services
-            .unwrap_or_else(|| default_l2_mainnet_services.to_vec())
-            .into_iter()
-            .map(evm_rpc_types::RpcService::ArbitrumOne)
-            .collect(),
-        evm_rpc_types::RpcServices::BaseMainnet(services) => services
-            .unwrap_or_else(|| default_l2_mainnet_services.to_vec())
-            .into_iter()
-            .map(evm_rpc_types::RpcService::BaseMainnet)
-            .collect(),
-        evm_rpc_types::RpcServices::OptimismMainnet(services) => services
-            .unwrap_or_else(|| default_l2_mainnet_services.to_vec())
-            .into_iter()
-            .map(evm_rpc_types::RpcService::OptimismMainnet)
-            .collect(),
-    }
-}
-
 pub(super) fn into_hash(value: Hex32) -> Hash {
     Hash(value.into())
 }

diff --git a/src/constants.rs b/src/constants.rs
@@ -1,5 +1,3 @@
-use evm_rpc_types::{EthMainnetService, EthSepoliaService, L2MainnetService};
-
 // HTTP outcall cost calculation
 // See https://internetcomputer.org/docs/current/developer-docs/gas-cost#special-features
 pub const INGRESS_OVERHEAD_BYTES: u128 = 100;
@@ -36,23 +34,6 @@ pub const API_KEY_REPLACE_STRING: &str = "{API_KEY}";
 pub const VALID_API_KEY_CHARS: &str =
     "0123456789ABCDEFGHIJKLMNOPQRTSUVWXYZabcdefghijklmnopqrstuvwxyz$-_.+!*";
 
-// Providers used by default (when passing `null` with `RpcServices`)
-pub const DEFAULT_ETH_MAINNET_SERVICES: &[EthMainnetService] = &[
-    EthMainnetService::Ankr,
-    EthMainnetService::Cloudflare,
-    EthMainnetService::PublicNode,
-];
-pub const DEFAULT_ETH_SEPOLIA_SERVICES: &[EthSepoliaService] = &[
-    EthSepoliaService::Ankr,
-    EthSepoliaService::BlockPi,
-    EthSepoliaService::PublicNode,
-];
-pub const DEFAULT_L2_MAINNET_SERVICES: &[L2MainnetService] = &[
-    L2MainnetService::Ankr,
-    L2MainnetService::BlockPi,
-    L2MainnetService::PublicNode,
-];
-
 pub const CONTENT_TYPE_HEADER_LOWERCASE: &str = "content-type";
 pub const CONTENT_TYPE_VALUE: &str = "application/json";
 

diff --git a/src/rpc_client/eth_rpc/mod.rs b/src/rpc_client/eth_rpc/mod.rs
@@ -1,16 +1,18 @@
 //! This module contains definitions for communicating with an Ethereum API using the [JSON RPC](https://ethereum.org/en/developers/docs/apis/json-rpc/)
 //! interface.
 
+use crate::accounting::get_http_request_cost;
 use crate::logs::{DEBUG, TRACE_HTTP};
 use crate::memory::next_request_id;
+use crate::providers::resolve_rpc_service;
 use crate::rpc_client::checked_amount::CheckedAmountOf;
 use crate::rpc_client::eth_rpc_error::{sanitize_send_raw_transaction_result, Parser};
 use crate::rpc_client::numeric::{BlockNumber, LogIndex, TransactionCount, Wei, WeiPerGas};
 use crate::rpc_client::responses::TransactionReceipt;
-use crate::rpc_client::RpcTransport;
+use crate::types::MetricRpcMethod;
 use candid::candid_method;
 use ethnum;
-use evm_rpc_types::{HttpOutcallError, JsonRpcError, RpcError, RpcService};
+use evm_rpc_types::{HttpOutcallError, JsonRpcError, ProviderError, RpcApi, RpcError, RpcService};
 use ic_canister_log::log;
 use ic_cdk::api::call::RejectionCode;
 use ic_cdk::api::management_canister::http_request::{
@@ -615,14 +617,13 @@ impl<T: HttpResponsePayload> HttpResponsePayload for Option<T> {}
 impl HttpResponsePayload for TransactionCount {}
 
 /// Calls a JSON-RPC method on an Ethereum node at the specified URL.
-pub async fn call<T, I, O>(
+pub async fn call<I, O>(
     provider: &RpcService,
     method: impl Into<String>,
     params: I,
     mut response_size_estimate: ResponseSizeEstimate,
 ) -> Result<O, RpcError>
 where
-    T: RpcTransport,
     I: Serialize,
     O: DeserializeOwned + HttpResponsePayload,
 {
@@ -633,7 +634,7 @@ where
         method: eth_method.clone(),
         id: 1,
     };
-    let api = T::resolve_api(provider)?;
+    let api = resolve_api(provider)?;
     let url = &api.url;
     let mut headers = vec![HttpHeader {
         name: "Content-Type".to_string(),
@@ -673,13 +674,8 @@ where
             )),
         };
 
-        let response = match T::http_request(
-            provider,
-            &eth_method,
-            request,
-            effective_size_estimate,
-        )
-        .await
+        let response = match http_request(provider, &eth_method, request, effective_size_estimate)
+            .await
         {
             Err(RpcError::HttpOutcallError(HttpOutcallError::IcError { code, message }))
                 if is_response_too_large(&code, &message) =>
@@ -730,6 +726,29 @@ where
     }
 }
 
+fn resolve_api(service: &RpcService) -> Result<RpcApi, ProviderError> {
+    Ok(resolve_rpc_service(service.clone())?.api())
+}
+
+async fn http_request(
+    service: &RpcService,
+    method: &str,
+    request: CanisterHttpRequestArgument,
+    effective_response_size_estimate: u64,
+) -> Result<HttpResponse, RpcError> {
+    let service = resolve_rpc_service(service.clone())?;
+    let cycles_cost = get_http_request_cost(
+        request
+            .body
+            .as_ref()
+            .map(|bytes| bytes.len() as u64)
+            .unwrap_or_default(),
+        effective_response_size_estimate,
+    );
+    let rpc_method = MetricRpcMethod(method.to_string());
+    crate::http::http_request(rpc_method, service, request, cycles_cost).await
+}
+
 fn http_status_code(response: &HttpResponse) -> u16 {
     use num_traits::cast::ToPrimitive;
     // HTTP status code are always 3 decimal digits, hence at most 999.