From 280334b1b00255de526ea6982153dcbeaf6df15b Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Mon, 16 Nov 2020 23:10:42 +0000 Subject: [PATCH] Validate eth1 chain id (#1877) ## Issue Addressed Resolves #1815 ## Proposed Changes Adds extra validation for eth1 chain id apart from the existing check for eth1 network id. --- beacon_node/eth1/src/http.rs | 41 +++++++++++++++++--------- beacon_node/eth1/src/service.rs | 44 ++++++++++++++++++++-------- beacon_node/src/config.rs | 1 + testing/eth1_test_rig/src/ganache.rs | 8 +++++ testing/simulator/src/eth1_sim.rs | 6 ++-- 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/beacon_node/eth1/src/http.rs b/beacon_node/eth1/src/http.rs index 9fa14a7b7a..f6ed2e8344 100644 --- a/beacon_node/eth1/src/http.rs +++ b/beacon_node/eth1/src/http.rs @@ -32,8 +32,9 @@ pub const DEPOSIT_COUNT_RESPONSE_BYTES: usize = 96; /// Number of bytes in deposit contract deposit root (value only). pub const DEPOSIT_ROOT_BYTES: usize = 32; +/// Represents an eth1 chain/network id. #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] -pub enum Eth1NetworkId { +pub enum Eth1Id { Goerli, Mainnet, Custom(u64), @@ -46,28 +47,28 @@ pub enum BlockQuery { Latest, } -impl Into for Eth1NetworkId { +impl Into for Eth1Id { fn into(self) -> u64 { match self { - Eth1NetworkId::Mainnet => 1, - Eth1NetworkId::Goerli => 5, - Eth1NetworkId::Custom(id) => id, + Eth1Id::Mainnet => 1, + Eth1Id::Goerli => 5, + Eth1Id::Custom(id) => id, } } } -impl From for Eth1NetworkId { +impl From for Eth1Id { fn from(id: u64) -> Self { - let into = |x: Eth1NetworkId| -> u64 { x.into() }; + let into = |x: Eth1Id| -> u64 { x.into() }; match id { - id if id == into(Eth1NetworkId::Mainnet) => Eth1NetworkId::Mainnet, - id if id == into(Eth1NetworkId::Goerli) => Eth1NetworkId::Goerli, - id => Eth1NetworkId::Custom(id), + id if id == into(Eth1Id::Mainnet) => Eth1Id::Mainnet, + id if id == into(Eth1Id::Goerli) => Eth1Id::Goerli, + id => Eth1Id::Custom(id), } } } -impl FromStr for Eth1NetworkId { +impl FromStr for Eth1Id { type Err = String; fn from_str(s: &str) -> Result { @@ -78,16 +79,28 @@ impl FromStr for Eth1NetworkId { } /// Get the eth1 network id of the given endpoint. -pub async fn get_network_id(endpoint: &str, timeout: Duration) -> Result { +pub async fn get_network_id(endpoint: &str, timeout: Duration) -> Result { let response_body = send_rpc_request(endpoint, "net_version", json!([]), timeout).await?; - Eth1NetworkId::from_str( + Eth1Id::from_str( response_result(&response_body)? - .ok_or_else(|| "No result was returned for block number".to_string())? + .ok_or_else(|| "No result was returned for network id".to_string())? .as_str() .ok_or_else(|| "Data was not string")?, ) } +/// Get the eth1 chain id of the given endpoint. +pub async fn get_chain_id(endpoint: &str, timeout: Duration) -> Result { + let response_body = send_rpc_request(endpoint, "eth_chainId", json!([]), timeout).await?; + hex_to_u64_be( + response_result(&response_body)? + .ok_or_else(|| "No result was returned for chain id".to_string())? + .as_str() + .ok_or_else(|| "Data was not string")?, + ) + .map(Into::into) +} + #[derive(Debug, PartialEq, Clone)] pub struct Block { pub hash: Hash256, diff --git a/beacon_node/eth1/src/service.rs b/beacon_node/eth1/src/service.rs index 47e8c38610..4fd1199bd6 100644 --- a/beacon_node/eth1/src/service.rs +++ b/beacon_node/eth1/src/service.rs @@ -3,8 +3,8 @@ use crate::{ block_cache::{BlockCache, Error as BlockCacheError, Eth1Block}, deposit_cache::Error as DepositCacheError, http::{ - get_block, get_block_number, get_deposit_logs_in_range, get_network_id, BlockQuery, - Eth1NetworkId, Log, + get_block, get_block_number, get_chain_id, get_deposit_logs_in_range, get_network_id, + BlockQuery, Eth1Id, Log, }, inner::{DepositUpdater, Inner}, }; @@ -18,8 +18,10 @@ use std::time::{SystemTime, UNIX_EPOCH}; use tokio::time::{interval_at, Duration, Instant}; use types::ChainSpec; -/// Indicates the default eth1 network we use for the deposit contract. -pub const DEFAULT_NETWORK_ID: Eth1NetworkId = Eth1NetworkId::Goerli; +/// Indicates the default eth1 network id we use for the deposit contract. +pub const DEFAULT_NETWORK_ID: Eth1Id = Eth1Id::Goerli; +/// Indicates the default eth1 chain id we use for the deposit contract. +pub const DEFAULT_CHAIN_ID: Eth1Id = Eth1Id::Goerli; const STANDARD_TIMEOUT_MILLIS: u64 = 15_000; @@ -84,7 +86,9 @@ pub struct Config { /// The address the `BlockCache` and `DepositCache` should assume is the canonical deposit contract. pub deposit_contract_address: String, /// The eth1 network id where the deposit contract is deployed (Goerli/Mainnet). - pub network_id: Eth1NetworkId, + pub network_id: Eth1Id, + /// The eth1 chain id where the deposit contract is deployed (Goerli/Mainnet). + pub chain_id: Eth1Id, /// Defines the first block that the `DepositCache` will start searching for deposit logs. /// /// Setting too high can result in missed logs. Setting too low will result in unnecessary @@ -115,6 +119,7 @@ impl Default for Config { endpoint: "http://localhost:8545".into(), deposit_contract_address: "0x0000000000000000000000000000000000000000".into(), network_id: DEFAULT_NETWORK_ID, + chain_id: DEFAULT_CHAIN_ID, deposit_contract_deploy_block: 1, lowest_cached_block_number: 1, follow_distance: 128, @@ -387,23 +392,36 @@ impl Service { async fn do_update(&self, update_interval: Duration) -> Result<(), ()> { let endpoint = self.config().endpoint.clone(); - let config_network = self.config().network_id.clone(); - let result = + let config_network_id = self.config().network_id.clone(); + let config_chain_id = self.config().chain_id.clone(); + let network_id_result = get_network_id(&endpoint, Duration::from_millis(STANDARD_TIMEOUT_MILLIS)).await; - match result { - Ok(network_id) => { - if network_id != config_network { + let chain_id_result = + get_chain_id(&endpoint, Duration::from_millis(STANDARD_TIMEOUT_MILLIS)).await; + match (network_id_result, chain_id_result) { + (Ok(network_id), Ok(chain_id)) => { + if network_id != config_network_id { crit!( self.log, - "Invalid eth1 network. Please switch to correct network"; - "expected" => format!("{:?}",config_network), + "Invalid eth1 network id. Please switch to correct network id"; + "expected" => format!("{:?}",config_network_id), "received" => format!("{:?}",network_id), "warning" => WARNING_MSG, ); return Ok(()); } + if chain_id != config_chain_id { + crit!( + self.log, + "Invalid eth1 chain id. Please switch to correct chain id"; + "expected" => format!("{:?}",config_chain_id), + "received" => format!("{:?}", chain_id), + "warning" => WARNING_MSG, + ); + return Ok(()); + } } - Err(_) => { + _ => { crit!( self.log, "Error connecting to eth1 node. Please ensure that you have an eth1 http server running locally on http://localhost:8545 or \ diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 9d8d668741..4ae124a8e5 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -258,6 +258,7 @@ pub fn get_config( client_config.eth1.deposit_contract_deploy_block; client_config.eth1.follow_distance = spec.eth1_follow_distance; client_config.eth1.network_id = spec.deposit_network_id.into(); + client_config.eth1.chain_id = spec.deposit_chain_id.into(); if let Some(mut boot_nodes) = eth2_testnet_config.boot_enr { client_config.network.boot_nodes_enr.append(&mut boot_nodes) diff --git a/testing/eth1_test_rig/src/ganache.rs b/testing/eth1_test_rig/src/ganache.rs index 9452d524f2..d9f15f4b48 100644 --- a/testing/eth1_test_rig/src/ganache.rs +++ b/testing/eth1_test_rig/src/ganache.rs @@ -15,6 +15,7 @@ use web3::{ const GANACHE_STARTUP_TIMEOUT_MILLIS: u64 = 10_000; const NETWORK_ID: u64 = 42; +const CHAIN_ID: u64 = 42; /// Provides a dedicated `ganachi-cli` instance with a connected `Web3` instance. /// @@ -46,6 +47,8 @@ impl GanacheInstance { .arg("\"vast thought differ pull jewel broom cook wrist tribe word before omit\"") .arg("--networkId") .arg(format!("{}", NETWORK_ID)) + .arg("--chainId") + .arg(format!("{}", CHAIN_ID)) .spawn() .map_err(|e| { format!( @@ -106,6 +109,11 @@ impl GanacheInstance { NETWORK_ID } + /// Returns the chain id of the ganache instance + pub fn chain_id(&self) -> u64 { + CHAIN_ID + } + /// Increase the timestamp on future blocks by `increase_by` seconds. pub async fn increase_time(&self, increase_by: u64) -> Result<(), String> { self.web3 diff --git a/testing/simulator/src/eth1_sim.rs b/testing/simulator/src/eth1_sim.rs index 28e1f5f0d6..33fb3a2b6c 100644 --- a/testing/simulator/src/eth1_sim.rs +++ b/testing/simulator/src/eth1_sim.rs @@ -1,6 +1,6 @@ use crate::{checks, LocalNetwork, E}; use clap::ArgMatches; -use eth1::http::Eth1NetworkId; +use eth1::http::Eth1Id; use eth1_test_rig::GanacheEth1Instance; use futures::prelude::*; use node_test_rig::{ @@ -76,6 +76,7 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> { let ganache_eth1_instance = GanacheEth1Instance::new().await?; let deposit_contract = ganache_eth1_instance.deposit_contract; let network_id = ganache_eth1_instance.ganache.network_id(); + let chain_id = ganache_eth1_instance.ganache.chain_id(); let ganache = ganache_eth1_instance.ganache; let eth1_endpoint = ganache.endpoint(); let deposit_contract_address = deposit_contract.address(); @@ -108,7 +109,8 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> { beacon_config.eth1.follow_distance = 1; beacon_config.dummy_eth1_backend = false; beacon_config.sync_eth1_chain = true; - beacon_config.eth1.network_id = Eth1NetworkId::Custom(network_id); + beacon_config.eth1.network_id = Eth1Id::Custom(network_id); + beacon_config.eth1.chain_id = Eth1Id::Custom(chain_id); beacon_config.network.enr_address = Some(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)));