Remove withdrawals-processing feature (#3864)

* Use spec to Determine Supported Engine APIs

* Remove `withdrawals-processing` feature

* Fixed Tests

* Missed Some Spots

* Fixed Another Test

* Stupid Clippy

beacon_node/execution_layer/Cargo.toml

@@ -4,8 +4,6 @@ version = "0.1.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-[features]
-withdrawals-processing = ["state_processing/withdrawals-processing"]
 
 [dependencies]
 types = { path = "../../consensus/types"}

beacon_node/execution_layer/src/engine_api.rs

@@ -329,7 +329,7 @@ pub struct ProposeBlindedBlockResponse {
 // This name is work in progress, it could
 // change when this method is actually proposed
 // but I'm writing this as it has been described
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 pub struct SupportedApis {
     pub new_payload_v1: bool,
     pub new_payload_v2: bool,

beacon_node/execution_layer/src/engine_api/http.rs

@@ -10,7 +10,7 @@ use serde_json::json;
 use tokio::sync::RwLock;
 
 use std::time::Duration;
-use types::EthSpec;
+use types::{ChainSpec, EthSpec};
 
 pub use deposit_log::{DepositLog, Log};
 pub use reqwest::Client;
@@ -538,12 +538,27 @@ impl HttpJsonRpc {
     pub fn new(
         url: SensitiveUrl,
         execution_timeout_multiplier: Option<u32>,
+        spec: &ChainSpec,
     ) -> Result<Self, Error> {
+        // FIXME: remove this `cached_supported_apis` spec hack once the `engine_getCapabilities`
+        //        method is implemented in all execution clients:
+        //        https://github.com/ethereum/execution-apis/issues/321
+        let cached_supported_apis = RwLock::new(Some(SupportedApis {
+            new_payload_v1: true,
+            new_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(),
+            forkchoice_updated_v1: true,
+            forkchoice_updated_v2: spec.capella_fork_epoch.is_some()
+                || spec.eip4844_fork_epoch.is_some(),
+            get_payload_v1: true,
+            get_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(),
+            exchange_transition_configuration_v1: true,
+        }));
+
         Ok(Self {
             client: Client::builder().build()?,
             url,
             execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1),
-            cached_supported_apis: Default::default(),
+            cached_supported_apis,
             auth: None,
         })
     }
@@ -552,12 +567,27 @@ impl HttpJsonRpc {
         url: SensitiveUrl,
         auth: Auth,
         execution_timeout_multiplier: Option<u32>,
+        spec: &ChainSpec,
     ) -> Result<Self, Error> {
+        // FIXME: remove this `cached_supported_apis` spec hack once the `engine_getCapabilities`
+        //        method is implemented in all execution clients:
+        //        https://github.com/ethereum/execution-apis/issues/321
+        let cached_supported_apis = RwLock::new(Some(SupportedApis {
+            new_payload_v1: true,
+            new_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(),
+            forkchoice_updated_v1: true,
+            forkchoice_updated_v2: spec.capella_fork_epoch.is_some()
+                || spec.eip4844_fork_epoch.is_some(),
+            get_payload_v1: true,
+            get_payload_v2: spec.capella_fork_epoch.is_some() || spec.eip4844_fork_epoch.is_some(),
+            exchange_transition_configuration_v1: true,
+        }));
+
         Ok(Self {
             client: Client::builder().build()?,
             url,
             execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1),
-            cached_supported_apis: Default::default(),
+            cached_supported_apis,
             auth: Some(auth),
         })
     }
@@ -848,21 +878,25 @@ impl HttpJsonRpc {
         Ok(response)
     }
 
-    // this is a stub as this method hasn't been defined yet
-    pub async fn supported_apis_v1(&self) -> Result<SupportedApis, Error> {
+    // TODO: This is currently a stub for the `engine_getCapabilities`
+    //       method. This stub is unused because we set cached_supported_apis
+    //       in the constructor based on the `spec`
+    //       Implement this once the execution clients support it
+    //       https://github.com/ethereum/execution-apis/issues/321
+    pub async fn get_capabilities(&self) -> Result<SupportedApis, Error> {
         Ok(SupportedApis {
             new_payload_v1: true,
-            new_payload_v2: cfg!(any(feature = "withdrawals-processing", test)),
+            new_payload_v2: true,
             forkchoice_updated_v1: true,
-            forkchoice_updated_v2: cfg!(any(feature = "withdrawals-processing", test)),
+            forkchoice_updated_v2: true,
             get_payload_v1: true,
-            get_payload_v2: cfg!(any(feature = "withdrawals-processing", test)),
+            get_payload_v2: true,
             exchange_transition_configuration_v1: true,
         })
     }
 
-    pub async fn set_cached_supported_apis(&self, supported_apis: SupportedApis) {
-        *self.cached_supported_apis.write().await = Some(supported_apis);
+    pub async fn set_cached_supported_apis(&self, supported_apis: Option<SupportedApis>) {
+        *self.cached_supported_apis.write().await = supported_apis;
     }
 
     pub async fn get_cached_supported_apis(&self) -> Result<SupportedApis, Error> {
@@ -870,8 +904,8 @@ impl HttpJsonRpc {
         if let Some(supported_apis) = cached_opt {
             Ok(supported_apis)
         } else {
-            let supported_apis = self.supported_apis_v1().await?;
-            self.set_cached_supported_apis(supported_apis).await;
+            let supported_apis = self.get_capabilities().await?;
+            self.set_cached_supported_apis(Some(supported_apis)).await;
             Ok(supported_apis)
         }
     }
@@ -955,6 +989,7 @@ mod test {
     impl Tester {
         pub fn new(with_auth: bool) -> Self {
             let server = MockServer::unit_testing();
+            let spec = MainnetEthSpec::default_spec();
 
             let rpc_url = SensitiveUrl::parse(&server.url()).unwrap();
             let echo_url = SensitiveUrl::parse(&format!("{}/echo", server.url())).unwrap();
@@ -965,13 +1000,13 @@ mod test {
                 let echo_auth =
                     Auth::new(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap(), None, None);
                 (
-                    Arc::new(HttpJsonRpc::new_with_auth(rpc_url, rpc_auth, None).unwrap()),
-                    Arc::new(HttpJsonRpc::new_with_auth(echo_url, echo_auth, None).unwrap()),
+                    Arc::new(HttpJsonRpc::new_with_auth(rpc_url, rpc_auth, None, &spec).unwrap()),
+                    Arc::new(HttpJsonRpc::new_with_auth(echo_url, echo_auth, None, &spec).unwrap()),
                 )
             } else {
                 (
-                    Arc::new(HttpJsonRpc::new(rpc_url, None).unwrap()),
-                    Arc::new(HttpJsonRpc::new(echo_url, None).unwrap()),
+                    Arc::new(HttpJsonRpc::new(rpc_url, None, &spec).unwrap()),
+                    Arc::new(HttpJsonRpc::new(echo_url, None, &spec).unwrap()),
                 )
             };
 

beacon_node/execution_layer/src/lib.rs

@@ -258,7 +258,12 @@ pub struct ExecutionLayer<T: EthSpec> {
 
 impl<T: EthSpec> ExecutionLayer<T> {
     /// Instantiate `Self` with an Execution engine specified in `Config`, using JSON-RPC via HTTP.
-    pub fn from_config(config: Config, executor: TaskExecutor, log: Logger) -> Result<Self, Error> {
+    pub fn from_config(
+        config: Config,
+        executor: TaskExecutor,
+        log: Logger,
+        spec: &ChainSpec,
+    ) -> Result<Self, Error> {
         let Config {
             execution_endpoints: urls,
             builder_url,
@@ -313,8 +318,9 @@ impl<T: EthSpec> ExecutionLayer<T> {
         let engine: Engine = {
             let auth = Auth::new(jwt_key, jwt_id, jwt_version);
             debug!(log, "Loaded execution endpoint"; "endpoint" => %execution_url, "jwt_path" => ?secret_file.as_path());
-            let api = HttpJsonRpc::new_with_auth(execution_url, auth, execution_timeout_multiplier)
-                .map_err(Error::ApiError)?;
+            let api =
+                HttpJsonRpc::new_with_auth(execution_url, auth, execution_timeout_multiplier, spec)
+                    .map_err(Error::ApiError)?;
             Engine::new(api, executor.clone(), &log)
         };
 

beacon_node/execution_layer/src/test_utils/mock_builder.rs

@@ -84,7 +84,8 @@ impl<E: EthSpec> TestingBuilder<E> {
         };
 
         let el =
-            ExecutionLayer::from_config(config, executor.clone(), executor.log().clone()).unwrap();
+            ExecutionLayer::from_config(config, executor.clone(), executor.log().clone(), &spec)
+                .unwrap();
 
         // This should probably be done for all fields, we only update ones we are testing with so far.
         let mut context = Context::for_mainnet();

beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs

@@ -9,7 +9,7 @@ use sensitive_url::SensitiveUrl;
 use task_executor::TaskExecutor;
 use tempfile::NamedTempFile;
 use tree_hash::TreeHash;
-use types::{Address, ChainSpec, Epoch, EthSpec, FullPayload, Hash256, Uint256};
+use types::{Address, ChainSpec, Epoch, EthSpec, FullPayload, Hash256, MainnetEthSpec};
 
 pub struct MockExecutionLayer<T: EthSpec> {
     pub server: MockServer<T>,
@@ -20,15 +20,17 @@ pub struct MockExecutionLayer<T: EthSpec> {
 
 impl<T: EthSpec> MockExecutionLayer<T> {
     pub fn default_params(executor: TaskExecutor) -> Self {
+        let mut spec = MainnetEthSpec::default_spec();
+        spec.terminal_total_difficulty = DEFAULT_TERMINAL_DIFFICULTY.into();
+        spec.terminal_block_hash = ExecutionBlockHash::zero();
+        spec.terminal_block_hash_activation_epoch = Epoch::new(0);
         Self::new(
             executor,
-            DEFAULT_TERMINAL_DIFFICULTY.into(),
             DEFAULT_TERMINAL_BLOCK,
-            ExecutionBlockHash::zero(),
-            Epoch::new(0),
             None,
             None,
             Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()),
+            spec,
             None,
         )
     }
@@ -36,29 +38,22 @@ impl<T: EthSpec> MockExecutionLayer<T> {
     #[allow(clippy::too_many_arguments)]
     pub fn new(
         executor: TaskExecutor,
-        terminal_total_difficulty: Uint256,
         terminal_block: u64,
-        terminal_block_hash: ExecutionBlockHash,
-        terminal_block_hash_activation_epoch: Epoch,
         shanghai_time: Option<u64>,
         eip4844_time: Option<u64>,
         jwt_key: Option<JwtKey>,
+        spec: ChainSpec,
         builder_url: Option<SensitiveUrl>,
     ) -> Self {
         let handle = executor.handle().unwrap();
 
-        let mut spec = T::default_spec();
-        spec.terminal_total_difficulty = terminal_total_difficulty;
-        spec.terminal_block_hash = terminal_block_hash;
-        spec.terminal_block_hash_activation_epoch = terminal_block_hash_activation_epoch;
-
         let jwt_key = jwt_key.unwrap_or_else(JwtKey::random);
         let server = MockServer::new(
             &handle,
             jwt_key,
-            terminal_total_difficulty,
+            spec.terminal_total_difficulty,
             terminal_block,
-            terminal_block_hash,
+            spec.terminal_block_hash,
             shanghai_time,
             eip4844_time,
         );
@@ -78,7 +73,8 @@ impl<T: EthSpec> MockExecutionLayer<T> {
             ..Default::default()
         };
         let el =
-            ExecutionLayer::from_config(config, executor.clone(), executor.log().clone()).unwrap();
+            ExecutionLayer::from_config(config, executor.clone(), executor.log().clone(), &spec)
+                .unwrap();
 
         Self {
             server,