Apply suggestions from review

sigp · Mar 7, 2022 · 2e1cc71 · 2e1cc71
1 parent 2a72690
commit 2e1cc71
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 7 deletions.
diff --git a/beacon_node/execution_layer/src/engine_api/auth.rs b/beacon_node/execution_layer/src/engine_api/auth.rs
@@ -101,7 +101,6 @@ impl Auth {
     ) -> Result<jsonwebtoken::TokenData<Claims>, Error> {
         let mut validation = jsonwebtoken::Validation::new(DEFAULT_ALGORITHM);
         validation.validate_exp = false;
-        // Really weird that we have to do this to get the validation working
         validation.required_spec_claims.remove("exp");
 
         jsonwebtoken::decode::<Claims>(

diff --git a/beacon_node/execution_layer/src/engines.rs b/beacon_node/execution_layer/src/engines.rs
@@ -265,7 +265,7 @@ impl<T: EngineApi> Engines<T> {
             }
         }
 
-        if num_synced == 0 && num_auth_failed != 0 && logging.is_enabled() {
+        if num_synced == 0 && num_auth_failed > 0 && logging.is_enabled() {
             crit!(
                 self.log,
                 "No synced execution engines";
@@ -313,8 +313,13 @@ impl<T: EngineApi> Engines<T> {
         let mut errors = vec![];
 
         for engine in &self.engines {
-            let engine_synced = *engine.state.read().await == EngineState::Synced;
-            let engine_auth_failed = *engine.state.read().await == EngineState::AuthFailed;
+            let (engine_synced, engine_auth_failed) = {
+                let state = engine.state.read().await;
+                (
+                    *state == EngineState::Synced,
+                    *state == EngineState::AuthFailed,
+                )
+            };
             if engine_synced {
                 match func(engine).await {
                     Ok(result) => return Ok(result),

diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
@@ -35,7 +35,7 @@ mod payload_status;
 pub mod test_utils;
 
 /// Name for the default file used for the jwt secret.
-pub const DEFAULT_JWT_FILE: &str = "jwtsecret";
+pub const DEFAULT_JWT_FILE: &str = "jwt.hex";
 
 /// Each time the `ExecutionLayer` retrieves a block from an execution node, it stores that block
 /// in an LRU cache to avoid redundant lookups. This is the size of that cache.
@@ -877,6 +877,7 @@ mod test {
         MockExecutionLayer::default_params()
             .move_to_block_prior_to_terminal_block()
             .with_terminal_block(|spec, el, _| async move {
+                el.engines().upcheck_not_synced(Logging::Disabled).await;
                 assert_eq!(el.get_terminal_pow_block_hash(&spec).await.unwrap(), None)
             })
             .await
@@ -895,6 +896,7 @@ mod test {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
             .with_terminal_block(|spec, el, terminal_block| async move {
+                el.engines().upcheck_not_synced(Logging::Disabled).await;
                 assert_eq!(
                     el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash, &spec)
                         .await
@@ -910,6 +912,7 @@ mod test {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
             .with_terminal_block(|spec, el, terminal_block| async move {
+                el.engines().upcheck_not_synced(Logging::Disabled).await;
                 let invalid_terminal_block = terminal_block.unwrap().parent_hash;
 
                 assert_eq!(
@@ -927,6 +930,7 @@ mod test {
         MockExecutionLayer::default_params()
             .move_to_terminal_block()
             .with_terminal_block(|spec, el, _| async move {
+                el.engines().upcheck_not_synced(Logging::Disabled).await;
                 let missing_terminal_block = ExecutionBlockHash::repeat_byte(42);
 
                 assert_eq!(

diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs
@@ -440,7 +440,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .value_name("JWT-VERSION")
                 .help("Used by the beacon node to communicate a client version to execution nodes \
                        during JWT authentication. It corresponds to the 'clv' field in the JWT claims object.\
-                       Set to the lighthouse version by deafult")
+                       Set to empty by deafult")
                 .takes_value(true)
         )
         .arg(

diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs
@@ -251,7 +251,12 @@ pub fn get_config<E: EthSpec>(
         }
 
         if let Some(secrets) = cli_args.value_of("jwt-secrets") {
-            el_config.secret_files = secrets.split(',').map(PathBuf::from).collect();
+            let secret_files: Vec<_> = secrets.split(',').map(PathBuf::from).collect();
+            if !secret_files.is_empty() && secret_files.len() != el_config.execution_endpoints.len()
+            {
+                return Err("length of jwt-secrets must be equal to number of execution-endpoints or 0 to use the default secret".to_string());
+            }
+            el_config.secret_files = secret_files;
         }
 
         el_config.suggested_fee_recipient =