Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to remove cketh-minter dependency #243

Closed
rvanasa opened this issue Jul 22, 2024 · 1 comment · Fixed by #257, #261, #263, #268 or #270
Closed

Refactor to remove cketh-minter dependency #243

rvanasa opened this issue Jul 22, 2024 · 1 comment · Fixed by #257, #261, #263, #268 or #270
Assignees

Comments

@rvanasa
Copy link
Collaborator

rvanasa commented Jul 22, 2024

The current EVM RPC implementation reuses logic from a fork of the ckETH minter canister codebase. Removing this dependency would speed up development and improve testability of the canister.

@gregorydemay
Copy link
Member

@rvanasa @THLO I started to look into a bit more details as to what this would entail and came up with the following rough plan (please feel free to comment/challenge any point or suggest alternative)

Analysis

Usage of cketh_common in production code as of 04505cce853362ff1b8ea4954028333f54394419

  • use cketh_common::eth_rpc_client::providers::RpcApi; here
  • use cketh_common::{ eth_rpc::{ into_nat, Block, FeeHistory, GetLogsParam, Hash, LogEntry, ProviderError, RpcError, SendRawTransactionResult, ValidationError, }, eth_rpc_client::{ providers::{RpcApi, RpcService}, requests::GetTransactionCountParams, EthRpcClient as CkEthRpcClient, MultiCallError, RpcConfig, RpcTransport, }, lifecycle::EthereumNetwork } here
  • use cketh_common::eth_rpc_client::providers::{EthMainnetService, EthSepoliaService, L2MainnetService}; here
  • use cketh_common::eth_rpc::{HttpOutcallError, ProviderError, RpcError, ValidationError}; here
  • use cketh_common::{ eth_rpc::{Block, FeeHistory, LogEntry, RpcError}, eth_rpc_client::{providers::RpcService, RpcConfig}, logs::INFO}; here
  • use cketh_common::logs::{Log, Priority, Sort}; here
  • use cketh_common::{ eth_rpc::ProviderError, eth_rpc_client::providers::{ EthMainnetService, EthSepoliaService, L2MainnetService, RpcApi, RpcService, }, logs::INFO}; here
  • use cketh_common::{ ::eth_rpc::RpcError, eth_rpc_client::providers::{ EthMainnetService, EthSepoliaService, L2MainnetService, RpcApi, RpcService, } }; here
  • use cketh_common::{ address::Address, eth_rpc::Hash, eth_rpc::{into_nat, FixedSizeData, ValidationError}, numeric::BlockNumber }; here
  • cketh_common::eth_rpc::BlockSpec here
  • cketh_common::eth_rpc::GetLogsParam here
  • cketh_common::eth_rpc::LogEntry here
  • cketh_common::eth_rpc_client::responses::TransactionReceipt here
  • cketh_common::eth_rpc::FeeHistoryParams here
  • cketh_common::eth_rpc_client::requests::GetTransactionCountParams here
  • use cketh_common::eth_rpc::ValidationError; here

Overview

  1. The main reason for using cketh_common is the usage of EthRpcClient, which contains all the logic for HTTPs outcalls (the EVM-RPC canisters method doing HTTPs outcalls, e.g., eth_fee_history, delegate to EthRpcClient).
  2. From there follows the usage of the various parameter or response types used by EthRpcClient

Rough Plan

  1. Copy over all data types from cketh_common (e.g., FeeHistory, LogEntry, but not EthRpcClient), into this repository. As part of this effort I think it would be useful
    1. to have a dedicated crate for the Candid types used in the API of the EVM-RPC canister. The easiest would be for this crate to live in the same repo as the evm-rpc canister so that it's always in sync.
    2. to have a dedicated crate for the type CheckedAmountOf (independent of the EVM-RPC canister) since this is also used by the ckETH minter (alternatively, integrate it into the phantom_newtype crate).
    3. We may want to have a clear separation between the types that are used for the candid interface (user/canister < > EVM-RPC) and the types that are used for the RPC communication (EVM-RPC <> Ethereum Json Provider), since the first are Candid-encoded and have an API character, while the second are JSON encoded and are an implementation detail (from the EVM-RPC canister point of view)
  2. Copy over the full code of EthRpcClient.
  3. Eliminate the dependency on cketh_common

@gregorydemay gregorydemay self-assigned this Aug 22, 2024
@gregorydemay gregorydemay linked a pull request Aug 23, 2024 that will close this issue
@gregorydemay gregorydemay reopened this Aug 27, 2024
@gregorydemay gregorydemay reopened this Aug 28, 2024
@gregorydemay gregorydemay linked a pull request Aug 28, 2024 that will close this issue
@gregorydemay gregorydemay reopened this Aug 29, 2024
@gregorydemay gregorydemay linked a pull request Sep 10, 2024 that will close this issue
@gregorydemay gregorydemay reopened this Sep 11, 2024
@gregorydemay gregorydemay reopened this Sep 12, 2024
@gregorydemay gregorydemay reopened this Sep 13, 2024
@gregorydemay gregorydemay reopened this Sep 18, 2024
@gregorydemay gregorydemay reopened this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment