From dd570f4decd872a77758c1a9b56d92a7f06de30d Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 28 Sep 2021 14:21:48 +1000 Subject: [PATCH] Add is_valid_terminal_block to EL --- beacon_node/execution_layer/src/engine_api.rs | 9 +- .../execution_layer/src/engine_api/http.rs | 17 ++- beacon_node/execution_layer/src/lib.rs | 123 +++++++++++++++--- 3 files changed, 125 insertions(+), 24 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 2b0e89e00e..01a55e83a6 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -21,6 +21,8 @@ pub enum Error { Eip155Failure, NoErrorOrResult, IsSyncing, + ExecutionBlockNotFound(Hash256), + ExecutionHeadBlockNotFound, } impl From for Error { @@ -42,9 +44,12 @@ pub trait EngineApi { async fn get_block_by_number<'a>( &self, block_by_number: BlockByNumberQuery<'a>, - ) -> Result; + ) -> Result, Error>; - async fn get_block_by_hash<'a>(&self, block_hash: Hash256) -> Result; + async fn get_block_by_hash<'a>( + &self, + block_hash: Hash256, + ) -> Result, Error>; async fn prepare_payload( &self, diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index c40537e3e3..41b2cb848f 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -13,6 +13,8 @@ pub use reqwest::Client; const STATIC_ID: u32 = 1; const JSONRPC_VERSION: &str = "2.0"; +const RETURN_FULL_TRANSACTION_OBJECTS: bool = false; + const ETH_GET_BLOCK_BY_NUMBER: &str = "eth_getBlockByNumber"; const ETH_GET_BLOCK_BY_NUMBER_TIMEOUT: Duration = Duration::from_secs(1); @@ -112,8 +114,8 @@ impl EngineApi for HttpJsonRpc { async fn get_block_by_number<'a>( &self, query: BlockByNumberQuery<'a>, - ) -> Result { - let params = json!([query]); + ) -> Result, Error> { + let params = json!([query, RETURN_FULL_TRANSACTION_OBJECTS]); self.rpc_request( ETH_GET_BLOCK_BY_NUMBER, @@ -123,8 +125,11 @@ impl EngineApi for HttpJsonRpc { .await } - async fn get_block_by_hash<'a>(&self, block_hash: Hash256) -> Result { - let params = json!([block_hash]); + async fn get_block_by_hash<'a>( + &self, + block_hash: Hash256, + ) -> Result, Error> { + let params = json!([block_hash, RETURN_FULL_TRANSACTION_OBJECTS]); self.rpc_request(ETH_GET_BLOCK_BY_HASH, params, ETH_GET_BLOCK_BY_HASH_TIMEOUT) .await @@ -431,7 +436,7 @@ mod test { "id": STATIC_ID, "jsonrpc": JSONRPC_VERSION, "method": ETH_GET_BLOCK_BY_NUMBER, - "params": ["latest"] + "params": ["latest", false] }), ) .await; @@ -448,7 +453,7 @@ mod test { "id": STATIC_ID, "jsonrpc": JSONRPC_VERSION, "method": ETH_GET_BLOCK_BY_HASH, - "params": [HASH_01] + "params": [HASH_01, false] }), ) .await; diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index ad3d103f25..87c10f7ec7 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -36,6 +36,7 @@ impl From for Error { struct Inner { engines: Engines, terminal_total_difficulty: Uint256, + terminal_block_hash: Option, fee_recipient: Option
, execution_blocks: Mutex>, executor: TaskExecutor, @@ -51,6 +52,7 @@ impl ExecutionLayer { pub fn from_urls( urls: Vec, terminal_total_difficulty: Uint256, + terminal_block_hash: Option, fee_recipient: Option
, executor: TaskExecutor, log: Logger, @@ -70,6 +72,7 @@ impl ExecutionLayer { log: log.clone(), }, terminal_total_difficulty, + terminal_block_hash, fee_recipient, execution_blocks: Mutex::new(LruCache::new(EXECUTION_BLOCKS_LRU_CACHE_SIZE)), executor, @@ -95,6 +98,10 @@ impl ExecutionLayer { self.inner.terminal_total_difficulty } + fn terminal_block_hash(&self) -> Option { + self.inner.terminal_block_hash + } + fn fee_recipient(&self) -> Result { self.inner .fee_recipient @@ -198,6 +205,7 @@ impl ExecutionLayer { crit!( self.log(), "Consensus failure between execution nodes"; + "method" => "execute_payload" ); } @@ -268,6 +276,27 @@ impl ExecutionLayer { } } + async fn get_pow_block( + &self, + engine: &Engine, + hash: Hash256, + ) -> Result, ApiError> { + if let Some(cached) = self.execution_blocks().await.get(&hash).copied() { + // The block was in the cache, no need to request it from the execution + // engine. + return Ok(Some(cached)); + } + + // The block was *not* in the cache, request it from the execution + // engine and cache it for future reference. + if let Some(block) = engine.api.get_block_by_hash(hash).await? { + self.execution_blocks().await.put(hash, block); + Ok(Some(block)) + } else { + Ok(None) + } + } + pub async fn get_pow_block_hash_at_total_difficulty(&self) -> Result, Error> { self.engines() .first_success(|engine| async move { @@ -275,28 +304,19 @@ impl ExecutionLayer { let mut block = engine .api .get_block_by_number(BlockByNumberQuery::Tag(LATEST_TAG)) - .await?; + .await? + .ok_or(ApiError::ExecutionHeadBlockNotFound)?; + self.execution_blocks().await.put(block.parent_hash, block); loop { if block.total_difficulty >= self.terminal_total_difficulty() { ttd_exceeding_block = Some(block.block_hash); - let cached = self - .execution_blocks() - .await - .get(&block.parent_hash) - .copied(); - if let Some(cached_block) = cached { - // The block was in the cache, no need to request it from the execution - // engine. - block = cached_block; - } else { - // The block was *not* in the cache, request it from the execution - // engine and cache it for future reference. - block = engine.api.get_block_by_hash(block.parent_hash).await?; - self.execution_blocks().await.put(block.parent_hash, block); - } + block = self + .get_pow_block(engine, block.parent_hash) + .await? + .ok_or(ApiError::ExecutionBlockNotFound(block.parent_hash))?; } else { return Ok::<_, ApiError>(ttd_exceeding_block); } @@ -305,4 +325,75 @@ impl ExecutionLayer { .await .map_err(Error::EngineErrors) } + + /// Returns: + /// + /// - `Some(true)` if the given `block_hash` is the terminal proof-of-work block. + /// - `Some(false)` if the given `block_hash` is *not* the terminal proof-of-work block. + /// - `None` if the `block_hash` or its parent were not present on the execution engines. + /// - `Err(_)` if there was an error connecting to the execution engines. + pub async fn is_valid_terminal_pow_block_hash( + &self, + block_hash: Hash256, + ) -> Result, Error> { + let broadcast_results = self + .engines() + .broadcast(|engine| async move { + if let Some(pow_block) = self.get_pow_block(engine, block_hash).await? { + if let Some(pow_parent) = + self.get_pow_block(engine, pow_block.parent_hash).await? + { + return Ok(Some( + self.is_valid_terminal_pow_block(pow_block, pow_parent), + )); + } + } + + Ok(None) + }) + .await; + + let mut errors = vec![]; + let mut terminal = 0; + let mut not_terminal = 0; + let mut block_missing = 0; + for result in broadcast_results { + match result { + Ok(Some(true)) => terminal += 1, + Ok(Some(false)) => not_terminal += 1, + Ok(None) => block_missing += 1, + Err(e) => errors.push(e), + } + } + + if terminal > 0 && not_terminal > 0 { + crit!( + self.log(), + "Consensus failure between execution nodes"; + "method" => "is_valid_terminal_pow_block_hash" + ); + } + + if terminal > 0 { + Ok(Some(true)) + } else if not_terminal > 0 { + Ok(Some(false)) + } else if block_missing > 0 { + Ok(None) + } else { + Err(Error::EngineErrors(errors)) + } + } + + fn is_valid_terminal_pow_block(&self, block: ExecutionBlock, parent: ExecutionBlock) -> bool { + if Some(block.block_hash) == self.terminal_block_hash() { + return true; + } + + let is_total_difficulty_reached = + block.total_difficulty >= self.terminal_total_difficulty(); + let is_parent_total_difficulty_valid = + parent.total_difficulty < self.terminal_total_difficulty(); + is_total_difficulty_reached && is_parent_total_difficulty_valid + } }