From 461bda6e8569fbd11e8386b58ea9849b98748cc2 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 16 Feb 2023 16:54:05 +1100 Subject: [PATCH] Execution engine suggestions from code review Co-authored-by: Paul Hauner --- .../execution_layer/src/engine_api/http.rs | 18 ++++++++---------- beacon_node/execution_layer/src/engines.rs | 5 ++--- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 3871ca27af..8f88de79a2 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -570,8 +570,8 @@ impl CapabilitiesCacheEntry { } } - pub fn engine_capabilities(&self) -> &EngineCapabilities { - &self.engine_capabilities + pub fn engine_capabilities(&self) -> EngineCapabilities { + self.engine_capabilities } pub fn age(&self) -> Duration { @@ -817,7 +817,9 @@ impl HttpJsonRpc { Ok(GetPayloadResponse::Merge(GetPayloadResponseMerge { execution_payload: payload_v1.into(), - // Have to guess zero here as we don't know the value + // Set the V1 payload values from the EE to be zero. This simulates + // the pre-block-value functionality of always choosing the builder + // block. block_value: Uint256::zero(), })) } @@ -984,16 +986,12 @@ impl HttpJsonRpc { ) -> Result { let mut lock = self.engine_capabilities_cache.lock().await; - if lock - .as_ref() - .map_or(true, |entry| entry.older_than(age_limit)) - { + if let Some(lock) = lock.as_ref().filter(|entry| !entry.older_than(age_limit)) { + Ok(lock.engine_capabilities()) + } else { let engine_capabilities = self.exchange_capabilities().await?; *lock = Some(CapabilitiesCacheEntry::new(engine_capabilities)); Ok(engine_capabilities) - } else { - // here entry is guaranteed to exist so unwrap() is safe - Ok(*lock.as_ref().unwrap().engine_capabilities()) } } diff --git a/beacon_node/execution_layer/src/engines.rs b/beacon_node/execution_layer/src/engines.rs index fe4058af00..1ee355e477 100644 --- a/beacon_node/execution_layer/src/engines.rs +++ b/beacon_node/execution_layer/src/engines.rs @@ -17,8 +17,7 @@ use types::ExecutionBlockHash; /// The number of payload IDs that will be stored for each `Engine`. /// -/// Since the size of each value is small (~100 bytes) a large number is used for safety. -/// FIXME: check this assumption now that the key includes entire payload attributes which now includes withdrawals +/// Since the size of each value is small (~800 bytes) a large number is used for safety. const PAYLOAD_ID_LRU_CACHE_SIZE: usize = 512; const CACHED_ENGINE_CAPABILITIES_AGE_LIMIT: Duration = Duration::from_secs(900); // 15 minutes @@ -276,7 +275,7 @@ impl Engine { let mut state = self.state.write().await; state.update(EngineStateInternal::AuthFailed); - (**state, CapabilitiesCacheAction::None) + (**state, CapabilitiesCacheAction::Clear) } Err(e) => { error!(