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

Remove chain::Id completely #3173

Merged
merged 9 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions crates/chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ pub enum Chain {
Sepolia = 11155111,
ArbitrumOne = 42161,
Base = 8453,
Hardhat = 31337,
}

impl Chain {
/// Returns the chain's chain ID
pub fn id(&self) -> Id {
Id(*self as u64)
pub fn id(&self) -> u64 {
*self as u64
}

/// Returns the canonical name of the chain on CoW Protocol.
Expand All @@ -37,6 +38,7 @@ impl Chain {
Self::Sepolia => "Ethereum / Sepolia",
Self::ArbitrumOne => "Arbitrum One",
Self::Base => "Base",
Self::Hardhat => "Hardhat",
}
}

Expand All @@ -47,6 +49,9 @@ impl Chain {
10u128.pow(17).into()
}
Self::Gnosis => 10u128.pow(18).into(),
Self::Hardhat => {
panic!("unsupported chain for default amount to estimate native prices with")
}
}
}

Expand All @@ -59,6 +64,7 @@ impl Chain {
Self::Sepolia => Duration::from_millis(12_000),
Self::ArbitrumOne => Duration::from_millis(250),
Self::Base => Duration::from_millis(2_000),
Self::Hardhat => panic!("unsupported block time for Hardhat chain"),
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -82,30 +88,13 @@ impl TryFrom<u64> for Chain {
x if x == Self::Sepolia as u64 => Self::Sepolia,
x if x == Self::ArbitrumOne as u64 => Self::ArbitrumOne,
x if x == Self::Base as u64 => Self::Base,
x if x == Self::Hardhat as u64 => Self::Hardhat,
_ => Err(ChainIdNotSupported)?,
};
Ok(network)
}
}

/// Chain ID as defined by EIP-155.
///
/// https://eips.ethereum.org/EIPS/eip-155
#[derive(Clone, Copy, Debug, Eq, PartialEq, From, Into)]
pub struct Id(u64);

impl From<U256> for Id {
fn from(value: U256) -> Self {
Self(value.as_u64())
}
}

impl std::fmt::Display for Id {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
}
}

impl TryFrom<U256> for Chain {
type Error = ChainIdNotSupported;

Expand Down
22 changes: 0 additions & 22 deletions crates/driver/src/domain/eth/eip712.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,3 @@
/// https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator
#[derive(Debug, Clone, Copy)]
pub struct DomainSeparator(pub [u8; 32]);

#[derive(Debug)]
pub struct DomainFields {
pub type_hash: &'static [u8],
pub name: &'static [u8],
pub version: &'static [u8],
pub chain_id: chain::Id,
pub verifying_contract: super::ContractAddress,
}

impl DomainSeparator {
pub fn new(fields: &DomainFields) -> Self {
let abi_string = ethabi::encode(&[
ethabi::Token::Uint(web3::signing::keccak256(fields.type_hash).into()),
ethabi::Token::Uint(web3::signing::keccak256(fields.name).into()),
ethabi::Token::Uint(web3::signing::keccak256(fields.version).into()),
ethabi::Token::Uint(ethereum_types::U256::from(u64::from(fields.chain_id))),
ethabi::Token::Address(fields.verifying_contract.into()),
]);
Self(web3::signing::keccak256(abi_string.as_slice()))
}
}
squadgazzz marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion crates/driver/src/domain/eth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ mod gas;

pub use {
allowance::Allowance,
eip712::{DomainFields, DomainSeparator},
eip712::DomainSeparator,
gas::{EffectiveGasPrice, FeePerGas, Gas, GasPrice},
number::nonzero::U256 as NonZeroU256,
primitive_types::{H160, H256, U256},
Expand Down
7 changes: 4 additions & 3 deletions crates/driver/src/infra/blockchain/contracts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
crate::{boundary, domain::eth, infra::blockchain::Ethereum},
chain::Chain,
ethcontract::dyns::DynWeb3,
ethrpc::block_stream::CurrentBlockWatcher,
thiserror::Error,
Expand Down Expand Up @@ -28,7 +29,7 @@ pub struct Addresses {
impl Contracts {
pub(super) async fn new(
web3: &DynWeb3,
chain: chain::Id,
chain: Chain,
addresses: Addresses,
block_stream: CurrentBlockWatcher,
archive_node_url: Option<&Url>,
Expand Down Expand Up @@ -130,12 +131,12 @@ pub struct CowAmmConfig {
/// there is no known deployment for the contract on that network.
pub fn deployment_address(
contract: &ethcontract::Contract,
network_id: chain::Id,
chain: Chain,
) -> Option<eth::ContractAddress> {
Some(
contract
.networks
.get(&network_id.to_string())?
.get(&chain.id().to_string())?
.address
.into(),
)
Expand Down
17 changes: 11 additions & 6 deletions crates/driver/src/infra/blockchain/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use {
self::contracts::ContractAt,
crate::{boundary, domain::eth},
chain::Chain,
ethcontract::{dyns::DynWeb3, errors::ExecutionError},
ethrpc::block_stream::CurrentBlockWatcher,
std::{fmt, sync::Arc},
Expand All @@ -18,7 +19,7 @@ pub use self::{contracts::Contracts, gas::GasPriceEstimator};
/// An Ethereum RPC connection.
pub struct Rpc {
web3: DynWeb3,
chain: chain::Id,
chain: Chain,
url: Url,
}

Expand All @@ -27,7 +28,8 @@ impl Rpc {
/// at the specifed URL.
pub async fn new(url: &url::Url) -> Result<Self, Error> {
sunce86 marked this conversation as resolved.
Show resolved Hide resolved
let web3 = boundary::buffered_web3_client(url);
let chain = web3.eth().chain_id().await?.into();
let chain =
Chain::try_from(web3.eth().chain_id().await?).map_err(|_| Error::UnsupportedChain)?;

Ok(Self {
web3,
Expand All @@ -36,8 +38,8 @@ impl Rpc {
})
}

/// Returns the chain id for the RPC connection.
pub fn chain(&self) -> chain::Id {
/// Returns the chain for the RPC connection.
pub fn chain(&self) -> Chain {
self.chain
}

Expand All @@ -55,7 +57,7 @@ pub struct Ethereum {
}

struct Inner {
chain: chain::Id,
chain: Chain,
contracts: Contracts,
gas: Arc<GasPriceEstimator>,
current_block: CurrentBlockWatcher,
Expand Down Expand Up @@ -102,7 +104,7 @@ impl Ethereum {
}
}

pub fn network(&self) -> chain::Id {
pub fn chain(&self) -> Chain {
self.inner.chain
}

Expand Down Expand Up @@ -275,6 +277,8 @@ pub enum Error {
GasPrice(boundary::Error),
#[error("access list estimation error: {0:?}")]
AccessList(serde_json::Value),
#[error("unsupported chain")]
UnsupportedChain,
}

impl Error {
Expand All @@ -290,6 +294,7 @@ impl Error {
}
Error::GasPrice(_) => false,
Error::AccessList(_) => true,
Error::UnsupportedChain => false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new error variant gets added which can never appear here and yet we have to handle this case. This is a code smell and indicates that these functions should return different errors.
Something for a future refactor, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can drop UnsupportedChain error, as it can appear only during application startup when wrong configuration is used (url parameter) and instead just panic with meaningful message in Rpc::new() function when Chain::try_from() fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion here. If you look at how the same constructor is used in autopilot, you can see that .expect() is used when the Chain::try_from is called in run.rs (so when we are aware of our surrounding and we explicitly want to exit), and error type is used inside Rpc::new because we don't know when and how this function will be called.

But I also agree that this error is rarely hit. If we agree to panic everywhere, then we can remove ChainIdNotSupported error from chain crate altogether. This is dangerous since we might want to try to build the Chain object from unreliable source that could give garbage, but it's not the case now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So another option would be to introduce new error type dedicated for Rpc object, for instance RpcError, which will contain only one variant ChainIdNotSupported, and caller of the constructor will decide if want to panic or map error to other type.
Alternatively constructor can return boolean type error as there is only one function which can fail with one error code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So another option would be to introduce new error type dedicated for Rpc object, for instance RpcError, which will contain only one variant ChainIdNotSupported, and caller of the constructor will decide if want to panic or map error to other type.

That seems reasonable. @sunce86 do you mind implementing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I created a new error type

}
}
}
Expand Down
10 changes: 7 additions & 3 deletions crates/driver/src/infra/config/file/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use {
solver::{self, SolutionMerging},
},
},
chain::Chain,
futures::future::join_all,
number::conversions::big_decimal_to_big_rational,
std::path::Path,
Expand All @@ -23,7 +24,7 @@ use {
/// # Panics
///
/// This method panics if the config is invalid or on I/O errors.
pub async fn load(chain: chain::Id, path: &Path) -> infra::Config {
pub async fn load(chain: Chain, path: &Path) -> infra::Config {
let data = fs::read_to_string(path)
.await
.unwrap_or_else(|e| panic!("I/O error while reading {path:?}: {e:?}"));
Expand All @@ -40,9 +41,12 @@ pub async fn load(chain: chain::Id, path: &Path) -> infra::Config {
});

assert_eq!(
config.chain_id.map(chain::Id::from).unwrap_or(chain),
config
.chain_id
.map(|id| Chain::try_from(id).expect("unsupported chain ID"))
.unwrap_or(chain),
chain,
"The configured chain ID does not match connected Ethereum node"
"The configured chain ID does not match the connected Ethereum node"
);
infra::Config {
solvers: join_all(config.solvers.into_iter().map(|config| async move {
Expand Down
19 changes: 10 additions & 9 deletions crates/driver/src/infra/liquidity/config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
crate::{domain::eth, infra::blockchain::contracts::deployment_address},
chain::Chain,
derive_more::Debug,
hex_literal::hex,
reqwest::Url,
Expand Down Expand Up @@ -49,7 +50,7 @@ pub struct UniswapV2 {
impl UniswapV2 {
/// Returns the liquidity configuration for Uniswap V2.
#[allow(clippy::self_named_constructors)]
pub fn uniswap_v2(chain: chain::Id) -> Option<Self> {
pub fn uniswap_v2(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::UniswapV2Router02::raw_contract(), chain)?,
pool_code: hex!("96e8ac4277198ff8b6f785478aa9a39f403cb768dd02cbee326c3e7da348845f")
Expand All @@ -59,7 +60,7 @@ impl UniswapV2 {
}

/// Returns the liquidity configuration for SushiSwap.
pub fn sushi_swap(chain: chain::Id) -> Option<Self> {
pub fn sushi_swap(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::SushiSwapRouter::raw_contract(), chain)?,
pool_code: hex!("e18a34eb0e04b04f7a0ac29a6e80748dca96319b42c54d679cb821dca90c6303")
Expand All @@ -69,7 +70,7 @@ impl UniswapV2 {
}

/// Returns the liquidity configuration for Honeyswap.
pub fn honeyswap(chain: chain::Id) -> Option<Self> {
pub fn honeyswap(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::HoneyswapRouter::raw_contract(), chain)?,
pool_code: hex!("3f88503e8580ab941773b59034fb4b2a63e86dbc031b3633a925533ad3ed2b93")
Expand All @@ -79,7 +80,7 @@ impl UniswapV2 {
}

/// Returns the liquidity configuration for Baoswap.
pub fn baoswap(chain: chain::Id) -> Option<Self> {
pub fn baoswap(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::BaoswapRouter::raw_contract(), chain)?,
pool_code: hex!("0bae3ead48c325ce433426d2e8e6b07dac10835baec21e163760682ea3d3520d")
Expand All @@ -89,7 +90,7 @@ impl UniswapV2 {
}

/// Returns the liquidity configuration for PancakeSwap.
pub fn pancake_swap(chain: chain::Id) -> Option<Self> {
pub fn pancake_swap(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::PancakeRouter::raw_contract(), chain)?,
pool_code: hex!("57224589c67f3f30a6b0d7a1b54cf3153ab84563bc609ef41dfb34f8b2974d2d")
Expand All @@ -100,7 +101,7 @@ impl UniswapV2 {

/// Returns the liquidity configuration for liquidity sources only used on
/// test networks.
pub fn testnet_uniswapv2(chain: chain::Id) -> Option<Self> {
pub fn testnet_uniswapv2(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::TestnetUniswapV2Router02::raw_contract(), chain)?,
pool_code: hex!("0efd7612822d579e24a8851501d8c2ad854264a1050e3dfcee8afcca08f80a86")
Expand All @@ -126,7 +127,7 @@ pub struct Swapr {
impl Swapr {
/// Returns the liquidity configuration for Swapr.
#[allow(clippy::self_named_constructors)]
pub fn swapr(chain: chain::Id) -> Option<Self> {
pub fn swapr(chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::SwaprRouter::raw_contract(), chain)?,
pool_code: hex!("d306a548755b9295ee49cc729e13ca4a45e00199bbd890fa146da43a50571776")
Expand All @@ -152,7 +153,7 @@ pub struct UniswapV3 {
impl UniswapV3 {
/// Returns the liquidity configuration for Uniswap V3.
#[allow(clippy::self_named_constructors)]
pub fn uniswap_v3(graph_url: &Url, chain: chain::Id) -> Option<Self> {
pub fn uniswap_v3(graph_url: &Url, chain: Chain) -> Option<Self> {
Some(Self {
router: deployment_address(contracts::UniswapV3SwapRouter::raw_contract(), chain)?,
max_pools_to_initialize: 100,
Expand Down Expand Up @@ -196,7 +197,7 @@ pub struct BalancerV2 {
impl BalancerV2 {
/// Returns the liquidity configuration for Balancer V2.
#[allow(clippy::self_named_constructors)]
pub fn balancer_v2(graph_url: &Url, chain: chain::Id) -> Option<Self> {
pub fn balancer_v2(graph_url: &Url, chain: Chain) -> Option<Self> {
let factory_addresses =
|contracts: &[&ethcontract::Contract]| -> Vec<eth::ContractAddress> {
contracts
Expand Down
13 changes: 5 additions & 8 deletions crates/driver/src/infra/simulator/enso/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
crate::domain::eth,
chain::Chain,
ethrpc::block_stream::CurrentBlockWatcher,
reqwest::ClientBuilder,
std::time::Duration,
Expand All @@ -13,7 +14,7 @@ const GAS_LIMIT: u64 = 30_000_000;
#[derive(Debug, Clone)]
pub(super) struct Enso {
url: reqwest::Url,
chain_id: chain::Id,
chain: Chain,
current_block: CurrentBlockWatcher,
network_block_interval: Option<Duration>,
}
Expand All @@ -27,14 +28,10 @@ pub struct Config {
}

impl Enso {
pub(super) fn new(
config: Config,
chain_id: chain::Id,
current_block: CurrentBlockWatcher,
) -> Self {
pub(super) fn new(config: Config, chain: Chain, current_block: CurrentBlockWatcher) -> Self {
Self {
url: reqwest::Url::parse(&format!("{}api/v1/simulate", config.url)).unwrap(),
chain_id,
chain,
current_block,
network_block_interval: config.network_block_interval,
}
Expand All @@ -61,7 +58,7 @@ impl Enso {
.unwrap()
.post(self.url.clone())
.json(&dto::Request {
chain_id: self.chain_id.into(),
chain_id: self.chain.id(),
from: tx.from.into(),
to: tx.to.into(),
data: tx.input.into(),
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/simulator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Simulator {
Self {
inner: Inner::Enso(enso::Enso::new(
config,
eth.network(),
eth.chain(),
eth.current_block().clone(),
)),
eth,
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/infra/simulator/tenderly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Tenderly {
.client
.post(self.endpoint.clone())
.json(&dto::Request {
network_id: self.eth.network().to_string(),
network_id: self.eth.chain().id().to_string(),
from: tx.from.into(),
to: tx.to.into(),
input: tx.input.clone().into(),
Expand Down
Loading
Loading