Optimistic sync spec tests (v1.2.0) (sigp#3564)

## Issue Addressed

Implements new optimistic sync test format from ethereum/consensus-specs#2982.

## Proposed Changes

- Add parsing and runner support for the new test format.
- Extend the mock EL with a set of canned responses keyed by block hash. Although this doubles up on some of the existing functionality I think it's really nice to use compared to the `preloaded_responses` or static responses. I think we could write novel new opt sync tests using these primtives much more easily than the previous ones. Forks are natively supported, and different responses to `forkchoiceUpdated` and `newPayload` are also straight-forward.

## Additional Info

Blocked on merge of the spec PR and release of new test vectors.

beacon_node/execution_layer/src/engine_api/json_structures.rs

@@ -1,5 +1,6 @@
 use super::*;
 use serde::{Deserialize, Serialize};
+use strum::EnumString;
 use types::{EthSpec, ExecutionBlockHash, FixedVector, Transaction, Unsigned, VariableList};
 
 #[derive(Debug, PartialEq, Serialize, Deserialize)]
@@ -311,8 +312,9 @@ impl From<JsonForkChoiceStateV1> for ForkChoiceState {
     }
 }
 
-#[derive(Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, EnumString)]
 #[serde(rename_all = "SCREAMING_SNAKE_CASE")]
+#[strum(serialize_all = "SCREAMING_SNAKE_CASE")]
 pub enum JsonPayloadStatusV1Status {
     Valid,
     Invalid,

beacon_node/execution_layer/src/test_utils/handle_rpc.rs

@@ -77,6 +77,11 @@ pub async fn handle_rpc<T: EthSpec>(
         ENGINE_NEW_PAYLOAD_V1 => {
             let request: JsonExecutionPayloadV1<T> = get_param(params, 0)?;
 
+            // Canned responses set by block hash take priority.
+            if let Some(status) = ctx.get_new_payload_status(&request.block_hash) {
+                return Ok(serde_json::to_value(JsonPayloadStatusV1::from(status)).unwrap());
+            }
+
             let (static_response, should_import) =
                 if let Some(mut response) = ctx.static_new_payload_response.lock().clone() {
                     if response.status.status == PayloadStatusV1Status::Valid {
@@ -120,6 +125,15 @@ pub async fn handle_rpc<T: EthSpec>(
 
             let head_block_hash = forkchoice_state.head_block_hash;
 
+            // Canned responses set by block hash take priority.
+            if let Some(status) = ctx.get_fcu_payload_status(&head_block_hash) {
+                let response = JsonForkchoiceUpdatedV1Response {
+                    payload_status: JsonPayloadStatusV1::from(status),
+                    payload_id: None,
+                };
+                return Ok(serde_json::to_value(response).unwrap());
+            }
+
             let mut response = ctx
                 .execution_block_generator
                 .write()

beacon_node/execution_layer/src/test_utils/mod.rs

@@ -12,6 +12,7 @@ use parking_lot::{Mutex, RwLock, RwLockWriteGuard};
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 use slog::{info, Logger};
+use std::collections::HashMap;
 use std::convert::Infallible;
 use std::future::Future;
 use std::marker::PhantomData;
@@ -98,6 +99,8 @@ impl<T: EthSpec> MockServer<T> {
             static_new_payload_response: <_>::default(),
             static_forkchoice_updated_response: <_>::default(),
             static_get_block_by_hash_response: <_>::default(),
+            new_payload_statuses: <_>::default(),
+            fcu_payload_statuses: <_>::default(),
             _phantom: PhantomData,
         });
 
@@ -370,6 +373,25 @@ impl<T: EthSpec> MockServer<T> {
     pub fn drop_all_blocks(&self) {
         self.ctx.execution_block_generator.write().drop_all_blocks()
     }
+
+    pub fn set_payload_statuses(&self, block_hash: ExecutionBlockHash, status: PayloadStatusV1) {
+        self.set_new_payload_status(block_hash, status.clone());
+        self.set_fcu_payload_status(block_hash, status);
+    }
+
+    pub fn set_new_payload_status(&self, block_hash: ExecutionBlockHash, status: PayloadStatusV1) {
+        self.ctx
+            .new_payload_statuses
+            .lock()
+            .insert(block_hash, status);
+    }
+
+    pub fn set_fcu_payload_status(&self, block_hash: ExecutionBlockHash, status: PayloadStatusV1) {
+        self.ctx
+            .fcu_payload_statuses
+            .lock()
+            .insert(block_hash, status);
+    }
 }
 
 #[derive(Debug)]
@@ -419,9 +441,33 @@ pub struct Context<T: EthSpec> {
     pub static_new_payload_response: Arc<Mutex<Option<StaticNewPayloadResponse>>>,
     pub static_forkchoice_updated_response: Arc<Mutex<Option<PayloadStatusV1>>>,
     pub static_get_block_by_hash_response: Arc<Mutex<Option<Option<ExecutionBlock>>>>,
+
+    // Canned responses by block hash.
+    //
+    // This is a more flexible and less stateful alternative to `static_new_payload_response`
+    // and `preloaded_responses`.
+    pub new_payload_statuses: Arc<Mutex<HashMap<ExecutionBlockHash, PayloadStatusV1>>>,
+    pub fcu_payload_statuses: Arc<Mutex<HashMap<ExecutionBlockHash, PayloadStatusV1>>>,
+
     pub _phantom: PhantomData<T>,
 }
 
+impl<T: EthSpec> Context<T> {
+    pub fn get_new_payload_status(
+        &self,
+        block_hash: &ExecutionBlockHash,
+    ) -> Option<PayloadStatusV1> {
+        self.new_payload_statuses.lock().get(block_hash).cloned()
+    }
+
+    pub fn get_fcu_payload_status(
+        &self,
+        block_hash: &ExecutionBlockHash,
+    ) -> Option<PayloadStatusV1> {
+        self.fcu_payload_statuses.lock().get(block_hash).cloned()
+    }
+}
+
 /// Configuration for the HTTP server.
 #[derive(PartialEq, Debug, Clone, Serialize, Deserialize)]
 pub struct Config {

testing/ef_tests/Cargo.toml

@@ -35,3 +35,4 @@ fs2 = "0.4.3"
 beacon_chain = { path = "../../beacon_node/beacon_chain" }
 store = { path = "../../beacon_node/store" }
 fork_choice = { path = "../../consensus/fork_choice" }
+execution_layer = { path = "../../beacon_node/execution_layer" }

testing/ef_tests/Makefile

@@ -1,4 +1,4 @@
-TESTS_TAG := v1.2.0-rc.3
+TESTS_TAG := v1.2.0
 TESTS = general minimal mainnet
 TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS))
 

testing/ef_tests/src/cases/bls_aggregate_sigs.rs

@@ -7,7 +7,7 @@ use serde_derive::Deserialize;
 #[derive(Debug, Clone, Deserialize)]
 pub struct BlsAggregateSigs {
     pub input: Vec<String>,
-    pub output: String,
+    pub output: Option<String>,
 }
 
 impl_bls_load_case!(BlsAggregateSigs);
@@ -25,14 +25,13 @@ impl Case for BlsAggregateSigs {
             aggregate_signature.add_assign(&sig);
         }
 
-        // Check for YAML null value, indicating invalid input. This is a bit of a hack,
-        // as our mutating `aggregate_signature.add` API doesn't play nicely with aggregating 0
-        // inputs.
-        let output_bytes = if self.output == "~" {
-            AggregateSignature::infinity().serialize().to_vec()
-        } else {
-            hex::decode(&self.output[2..])
-                .map_err(|e| Error::FailedToParseTest(format!("{:?}", e)))?
+        let output_bytes = match self.output.as_deref() {
+            // Check for YAML null value, indicating invalid input. This is a bit of a hack,
+            // as our mutating `aggregate_signature.add` API doesn't play nicely with aggregating 0
+            // inputs.
+            Some("~") | None => AggregateSignature::infinity().serialize().to_vec(),
+            Some(output) => hex::decode(&output[2..])
+                .map_err(|e| Error::FailedToParseTest(format!("{:?}", e)))?,
         };
         let aggregate_signature = Ok(aggregate_signature.serialize().to_vec());
 

testing/ef_tests/src/cases/fork_choice.rs

@@ -9,7 +9,8 @@ use beacon_chain::{
     test_utils::{BeaconChainHarness, EphemeralHarnessType},
     BeaconChainTypes, CachedHead, CountUnrealized,
 };
-use serde_derive::Deserialize;
+use execution_layer::{json_structures::JsonPayloadStatusV1Status, PayloadStatusV1};
+use serde::Deserialize;
 use ssz_derive::Decode;
 use state_processing::state_advance::complete_state_advance;
 use std::future::Future;
@@ -50,16 +51,53 @@ pub struct Checks {
     proposer_boost_root: Option<Hash256>,
 }
 
+#[derive(Debug, Clone, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct PayloadStatus {
+    status: JsonPayloadStatusV1Status,
+    latest_valid_hash: Option<ExecutionBlockHash>,
+    validation_error: Option<String>,
+}
+
+impl From<PayloadStatus> for PayloadStatusV1 {
+    fn from(status: PayloadStatus) -> Self {
+        PayloadStatusV1 {
+            status: status.status.into(),
+            latest_valid_hash: status.latest_valid_hash,
+            validation_error: status.validation_error,
+        }
+    }
+}
+
 #[derive(Debug, Clone, Deserialize)]
 #[serde(untagged, deny_unknown_fields)]
 pub enum Step<B, A, AS, P> {
-    Tick { tick: u64 },
-    ValidBlock { block: B },
-    MaybeValidBlock { block: B, valid: bool },
-    Attestation { attestation: A },
-    AttesterSlashing { attester_slashing: AS },
-    PowBlock { pow_block: P },
-    Checks { checks: Box<Checks> },
+    Tick {
+        tick: u64,
+    },
+    ValidBlock {
+        block: B,
+    },
+    MaybeValidBlock {
+        block: B,
+        valid: bool,
+    },
+    Attestation {
+        attestation: A,
+    },
+    AttesterSlashing {
+        attester_slashing: AS,
+    },
+    PowBlock {
+        pow_block: P,
+    },
+    OnPayloadInfo {
+        block_hash: ExecutionBlockHash,
+        payload_status: PayloadStatus,
+    },
+    Checks {
+        checks: Box<Checks>,
+    },
 }
 
 #[derive(Debug, Clone, Deserialize)]
@@ -119,6 +157,13 @@ impl<E: EthSpec> LoadCase for ForkChoiceTest<E> {
                     ssz_decode_file(&path.join(format!("{}.ssz_snappy", pow_block)))
                         .map(|pow_block| Step::PowBlock { pow_block })
                 }
+                Step::OnPayloadInfo {
+                    block_hash,
+                    payload_status,
+                } => Ok(Step::OnPayloadInfo {
+                    block_hash,
+                    payload_status,
+                }),
                 Step::Checks { checks } => Ok(Step::Checks { checks }),
             })
             .collect::<Result<_, _>>()?;
@@ -168,6 +213,14 @@ impl<E: EthSpec> Case for ForkChoiceTest<E> {
                     tester.process_attester_slashing(attester_slashing)
                 }
                 Step::PowBlock { pow_block } => tester.process_pow_block(pow_block),
+                Step::OnPayloadInfo {
+                    block_hash,
+                    payload_status,
+                } => {
+                    let el = tester.harness.mock_execution_layer.as_ref().unwrap();
+                    el.server
+                        .set_payload_statuses(*block_hash, payload_status.clone().into());
+                }
                 Step::Checks { checks } => {
                     let Checks {
                         head,

testing/ef_tests/src/cases/operations.rs

@@ -117,6 +117,11 @@ impl<E: EthSpec> Operation<E> for Deposit {
         ssz_decode_file(path)
     }
 
+    fn is_enabled_for_fork(_: ForkName) -> bool {
+        // Some deposit tests require signature verification but are not marked as such.
+        cfg!(not(feature = "fake_crypto"))
+    }
+
     fn apply_to(
         &self,
         state: &mut BeaconState<E>,

testing/ef_tests/src/handler.rs

@@ -546,6 +546,37 @@ impl<E: EthSpec + TypeName> Handler for ForkChoiceHandler<E> {
     }
 }
 
+#[derive(Derivative)]
+#[derivative(Default(bound = ""))]
+pub struct OptimisticSyncHandler<E>(PhantomData<E>);
+
+impl<E: EthSpec + TypeName> Handler for OptimisticSyncHandler<E> {
+    type Case = cases::ForkChoiceTest<E>;
+
+    fn config_name() -> &'static str {
+        E::name()
+    }
+
+    fn runner_name() -> &'static str {
+        "sync"
+    }
+
+    fn handler_name(&self) -> String {
+        "optimistic".into()
+    }
+
+    fn use_rayon() -> bool {
+        // The opt sync tests use `block_on` which can cause panics with rayon.
+        false
+    }
+
+    fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool {
+        fork_name != ForkName::Base
+            && fork_name != ForkName::Altair
+            && cfg!(not(feature = "fake_crypto"))
+    }
+}
+
 #[derive(Derivative)]
 #[derivative(Default(bound = ""))]
 pub struct GenesisValidityHandler<E>(PhantomData<E>);

testing/ef_tests/tests/tests.rs

@@ -448,6 +448,12 @@ fn fork_choice_ex_ante() {
     ForkChoiceHandler::<MainnetEthSpec>::new("ex_ante").run();
 }
 
+#[test]
+fn optimistic_sync() {
+    OptimisticSyncHandler::<MinimalEthSpec>::default().run();
+    OptimisticSyncHandler::<MainnetEthSpec>::default().run();
+}
+
 #[test]
 fn genesis_initialization() {
     GenesisInitializationHandler::<MinimalEthSpec>::default().run();