Ensure difficulty/hash/epoch overrides change the ChainSpec (#2798)

* Unify loading of eth2_network_config

* Apply overrides at lighthouse binary level

* Remove duplicate override values

* Add merge values to existing net configs

* Make override flags global

* Add merge fields to testing config

* Add one to TTD

* Fix failing engine tests

* Fix test compile error

* Remove TTD flags

* Move get_eth2_network_config

* Fix warn

* Address review comments

Author: paulhauner

Cargo.lock

@@ -2893,7 +2893,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             let parent_hash;
             if !is_merge_complete(&state) {
                 let terminal_pow_block_hash = execution_layer
-                    .block_on(|execution_layer| execution_layer.get_terminal_pow_block_hash())
+                    .block_on(|execution_layer| {
+                        execution_layer.get_terminal_pow_block_hash(&self.spec)
+                    })
                     .map_err(BlockProductionError::TerminalPoWBlockLookupFailed)?;
 
                 if let Some(terminal_pow_block_hash) = terminal_pow_block_hash {

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1119,7 +1119,10 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
 
             let is_valid_terminal_pow_block = execution_layer
                 .block_on(|execution_layer| {
-                    execution_layer.is_valid_terminal_pow_block_hash(execution_payload.parent_hash)
+                    execution_layer.is_valid_terminal_pow_block_hash(
+                        execution_payload.parent_hash,
+                        &chain.spec,
+                    )
                 })
                 .map_err(ExecutionPayloadError::from)?;
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -330,7 +330,6 @@ where
     }
 
     pub fn execution_layer(mut self, urls: &[&str]) -> Self {
-        let spec = self.spec.clone().expect("cannot build without spec");
         assert!(
             self.execution_layer.is_none(),
             "execution layer already defined"
@@ -345,8 +344,6 @@ where
             .unwrap();
         let execution_layer = ExecutionLayer::from_urls(
             urls,
-            spec.terminal_total_difficulty,
-            spec.terminal_block_hash,
             Some(Address::repeat_byte(42)),
             el_runtime.task_executor.clone(),
             el_runtime.log.clone(),
@@ -364,6 +361,7 @@ where
             spec.terminal_total_difficulty,
             DEFAULT_TERMINAL_BLOCK,
             spec.terminal_block_hash,
+            spec.terminal_block_hash_activation_epoch,
         );
         self.execution_layer = Some(mock.el.clone());
         self.mock_execution_layer = Some(mock);

beacon_node/beacon_chain/src/test_utils.rs

@@ -148,19 +148,10 @@ where
             None
         };
 
-        let terminal_total_difficulty = config
-            .terminal_total_difficulty_override
-            .unwrap_or(spec.terminal_total_difficulty);
-        let terminal_block_hash = config
-            .terminal_block_hash_override
-            .unwrap_or(spec.terminal_block_hash);
-
         let execution_layer = if let Some(execution_endpoints) = config.execution_endpoints {
             let context = runtime_context.service_context("exec".into());
             let execution_layer = ExecutionLayer::from_urls(
                 execution_endpoints,
-                terminal_total_difficulty,
-                terminal_block_hash,
                 config.fee_recipient,
                 context.executor.clone(),
                 context.log().clone(),

beacon_node/client/src/builder.rs

@@ -1,11 +1,10 @@
-use beacon_chain::types::Epoch;
 use directory::DEFAULT_ROOT_DIR;
 use network::NetworkConfig;
 use sensitive_url::SensitiveUrl;
 use serde_derive::{Deserialize, Serialize};
 use std::fs;
 use std::path::PathBuf;
-use types::{Address, Graffiti, Hash256, PublicKeyBytes, Uint256};
+use types::{Address, Graffiti, PublicKeyBytes};
 
 /// Default directory name for the freezer database under the top-level data dir.
 const DEFAULT_FREEZER_DB_DIR: &str = "freezer_db";
@@ -76,9 +75,6 @@ pub struct Config {
     pub chain: beacon_chain::ChainConfig,
     pub eth1: eth1::Config,
     pub execution_endpoints: Option<Vec<SensitiveUrl>>,
-    pub terminal_total_difficulty_override: Option<Uint256>,
-    pub terminal_block_hash_override: Option<Hash256>,
-    pub terminal_block_hash_epoch_override: Option<Epoch>,
     pub fee_recipient: Option<Address>,
     pub http_api: http_api::Config,
     pub http_metrics: http_metrics::Config,
@@ -101,9 +97,6 @@ impl Default for Config {
             sync_eth1_chain: false,
             eth1: <_>::default(),
             execution_endpoints: None,
-            terminal_total_difficulty_override: None,
-            terminal_block_hash_override: None,
-            terminal_block_hash_epoch_override: None,
             fee_recipient: None,
             disabled_forks: Vec::new(),
             graffiti: Graffiti::default(),

beacon_node/client/src/config.rs

@@ -18,6 +18,7 @@ use tokio::{
     sync::{Mutex, MutexGuard},
     time::{sleep, sleep_until, Instant},
 };
+use types::ChainSpec;
 
 pub use engine_api::{http::HttpJsonRpc, ExecutePayloadResponseStatus};
 
@@ -47,8 +48,6 @@ impl From<ApiError> for Error {
 
 struct Inner {
     engines: Engines<HttpJsonRpc>,
-    terminal_total_difficulty: Uint256,
-    terminal_block_hash: Hash256,
     fee_recipient: Option<Address>,
     execution_blocks: Mutex<LruCache<Hash256, ExecutionBlock>>,
     executor: TaskExecutor,
@@ -73,8 +72,6 @@ impl ExecutionLayer {
     /// Instantiate `Self` with `urls.len()` engines, all using the JSON-RPC via HTTP.
     pub fn from_urls(
         urls: Vec<SensitiveUrl>,
-        terminal_total_difficulty: Uint256,
-        terminal_block_hash: Hash256,
         fee_recipient: Option<Address>,
         executor: TaskExecutor,
         log: Logger,
@@ -98,8 +95,6 @@ impl ExecutionLayer {
                 latest_forkchoice_state: <_>::default(),
                 log: log.clone(),
             },
-            terminal_total_difficulty,
-            terminal_block_hash,
             fee_recipient,
             execution_blocks: Mutex::new(LruCache::new(EXECUTION_BLOCKS_LRU_CACHE_SIZE)),
             executor,
@@ -121,14 +116,6 @@ impl ExecutionLayer {
         &self.inner.executor
     }
 
-    fn terminal_total_difficulty(&self) -> Uint256 {
-        self.inner.terminal_total_difficulty
-    }
-
-    fn terminal_block_hash(&self) -> Hash256 {
-        self.inner.terminal_block_hash
-    }
-
     fn fee_recipient(&self) -> Result<Address, Error> {
         self.inner
             .fee_recipient
@@ -455,11 +442,14 @@ impl ExecutionLayer {
     /// `get_terminal_pow_block_hash`
     ///
     /// https://github.com/ethereum/consensus-specs/blob/v1.1.0/specs/merge/validator.md
-    pub async fn get_terminal_pow_block_hash(&self) -> Result<Option<Hash256>, Error> {
+    pub async fn get_terminal_pow_block_hash(
+        &self,
+        spec: &ChainSpec,
+    ) -> Result<Option<Hash256>, Error> {
         let hash_opt = self
             .engines()
             .first_success(|engine| async move {
-                if self.terminal_block_hash() != Hash256::zero() {
+                if spec.terminal_block_hash != Hash256::zero() {
                     // Note: the specification is written such that if there are multiple blocks in
                     // the PoW chain with the terminal block hash, then to select 0'th one.
                     //
@@ -468,11 +458,12 @@ impl ExecutionLayer {
                     // hash. Such a scenario would be a devestating hash collision with external
                     // implications far outweighing those here.
                     Ok(self
-                        .get_pow_block(engine, self.terminal_block_hash())
+                        .get_pow_block(engine, spec.terminal_block_hash)
                         .await?
                         .map(|block| block.block_hash))
                 } else {
-                    self.get_pow_block_hash_at_total_difficulty(engine).await
+                    self.get_pow_block_hash_at_total_difficulty(engine, spec)
+                        .await
                 }
             })
             .await
@@ -482,8 +473,8 @@ impl ExecutionLayer {
             info!(
                 self.log(),
                 "Found terminal block hash";
-                "terminal_block_hash_override" => ?self.terminal_block_hash(),
-                "terminal_total_difficulty" => ?self.terminal_total_difficulty(),
+                "terminal_block_hash_override" => ?spec.terminal_block_hash,
+                "terminal_total_difficulty" => ?spec.terminal_total_difficulty,
                 "block_hash" => ?hash,
             );
         }
@@ -503,6 +494,7 @@ impl ExecutionLayer {
     async fn get_pow_block_hash_at_total_difficulty(
         &self,
         engine: &Engine<HttpJsonRpc>,
+        spec: &ChainSpec,
     ) -> Result<Option<Hash256>, ApiError> {
         let mut ttd_exceeding_block = None;
         let mut block = engine
@@ -518,7 +510,7 @@ impl ExecutionLayer {
         //
         // https://github.com/ethereum/consensus-specs/issues/2636
         loop {
-            if block.total_difficulty >= self.terminal_total_difficulty() {
+            if block.total_difficulty >= spec.terminal_total_difficulty {
                 ttd_exceeding_block = Some(block.block_hash);
 
                 // Try to prevent infinite loops.
@@ -565,6 +557,7 @@ impl ExecutionLayer {
     pub async fn is_valid_terminal_pow_block_hash(
         &self,
         block_hash: Hash256,
+        spec: &ChainSpec,
     ) -> Result<Option<bool>, Error> {
         let broadcast_results = self
             .engines()
@@ -574,7 +567,7 @@ impl ExecutionLayer {
                         self.get_pow_block(engine, pow_block.parent_hash).await?
                     {
                         return Ok(Some(
-                            self.is_valid_terminal_pow_block(pow_block, pow_parent),
+                            self.is_valid_terminal_pow_block(pow_block, pow_parent, spec),
                         ));
                     }
                 }
@@ -618,15 +611,19 @@ impl ExecutionLayer {
     /// This function should remain internal.
     ///
     /// External users should use `self.is_valid_terminal_pow_block_hash`.
-    fn is_valid_terminal_pow_block(&self, block: ExecutionBlock, parent: ExecutionBlock) -> bool {
-        if block.block_hash == self.terminal_block_hash() {
+    fn is_valid_terminal_pow_block(
+        &self,
+        block: ExecutionBlock,
+        parent: ExecutionBlock,
+        spec: &ChainSpec,
+    ) -> bool {
+        if block.block_hash == spec.terminal_block_hash {
             return true;
         }
 
-        let is_total_difficulty_reached =
-            block.total_difficulty >= self.terminal_total_difficulty();
+        let is_total_difficulty_reached = block.total_difficulty >= spec.terminal_total_difficulty;
         let is_parent_total_difficulty_valid =
-            parent.total_difficulty < self.terminal_total_difficulty();
+            parent.total_difficulty < spec.terminal_total_difficulty;
         is_total_difficulty_reached && is_parent_total_difficulty_valid
     }
 
@@ -685,14 +682,14 @@ mod test {
     async fn finds_valid_terminal_block_hash() {
         MockExecutionLayer::default_params()
             .move_to_block_prior_to_terminal_block()
-            .with_terminal_block(|el, _| async move {
-                assert_eq!(el.get_terminal_pow_block_hash().await.unwrap(), None)
+            .with_terminal_block(|spec, el, _| async move {
+                assert_eq!(el.get_terminal_pow_block_hash(&spec).await.unwrap(), None)
             })
             .await
             .move_to_terminal_block()
-            .with_terminal_block(|el, terminal_block| async move {
+            .with_terminal_block(|spec, el, terminal_block| async move {
                 assert_eq!(
-                    el.get_terminal_pow_block_hash().await.unwrap(),
+                    el.get_terminal_pow_block_hash(&spec).await.unwrap(),
                     Some(terminal_block.unwrap().block_hash)
                 )
             })
@@ -703,9 +700,9 @@ mod test {
     async fn verifies_valid_terminal_block_hash() {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
-            .with_terminal_block(|el, terminal_block| async move {
+            .with_terminal_block(|spec, el, terminal_block| async move {
                 assert_eq!(
-                    el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash)
+                    el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash, &spec)
                         .await
                         .unwrap(),
                     Some(true)
@@ -718,11 +715,11 @@ mod test {
     async fn rejects_invalid_terminal_block_hash() {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
-            .with_terminal_block(|el, terminal_block| async move {
+            .with_terminal_block(|spec, el, terminal_block| async move {
                 let invalid_terminal_block = terminal_block.unwrap().parent_hash;
 
                 assert_eq!(
-                    el.is_valid_terminal_pow_block_hash(invalid_terminal_block)
+                    el.is_valid_terminal_pow_block_hash(invalid_terminal_block, &spec)
                         .await
                         .unwrap(),
                     Some(false)
@@ -735,11 +732,11 @@ mod test {
     async fn rejects_unknown_terminal_block_hash() {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
-            .with_terminal_block(|el, _| async move {
+            .with_terminal_block(|spec, el, _| async move {
                 let missing_terminal_block = Hash256::repeat_byte(42);
 
                 assert_eq!(
-                    el.is_valid_terminal_pow_block_hash(missing_terminal_block)
+                    el.is_valid_terminal_pow_block_hash(missing_terminal_block, &spec)
                         .await
                         .unwrap(),
                     None