Address First Round Comments

beacon_node/execution_layer/src/engine_api/new_payload_request.rs

@@ -172,7 +172,7 @@ impl<'block, E: EthSpec> NewPayloadRequest<'block, E> {
     }
 }
 
-//TODO(EIP7732): Consider implmenting these as methods on the NewPayloadRequest struct
+//TODO(EIP7732): Consider implementing these as methods on the NewPayloadRequest struct
 impl<'a, E: EthSpec> TryFrom<BeaconBlockRef<'a, E>> for NewPayloadRequest<'a, E> {
     type Error = BeaconStateError;
 

consensus/types/src/execution_payload_envelope.rs

@@ -2,103 +2,32 @@ use crate::test_utils::TestRandom;
 use crate::*;
 use beacon_block_body::KzgCommitments;
 use educe::Educe;
-use serde::de::{Deserializer, Error as _};
 use serde::{Deserialize, Serialize};
 use ssz_derive::{Decode, Encode};
-use superstruct::superstruct;
 use test_random_derive::TestRandom;
 use tree_hash_derive::TreeHash;
 
-// in all likelihood, this will be superstructed so might as well start early eh?
+#[derive(Debug, Clone, Serialize, Encode, Decode, Deserialize, TestRandom, TreeHash, Educe)]
-#[superstruct(
-    variants(Gloas, NextFork),
-    variant_attributes(
-        derive(
-            Debug,
-            Clone,
-            Serialize,
-            Deserialize,
-            Encode,
-            Decode,
-            TreeHash,
-            TestRandom,
-            Educe
-        ),
-        cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary)),
-        educe(PartialEq, Hash(bound(E: EthSpec))),
-        serde(bound = "E: EthSpec", deny_unknown_fields),
-        cfg_attr(feature = "arbitrary", arbitrary(bound = "E: EthSpec"))
-    ),
-    ref_attributes(
-        derive(Debug, PartialEq, TreeHash),
-        tree_hash(enum_behaviour = "transparent")
-    ),
-    cast_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant"),
-    partial_getter_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant")
-)]
-#[derive(Debug, Clone, Serialize, Encode, Deserialize, TreeHash, Educe)]
 #[educe(PartialEq, Hash(bound(E: EthSpec)))]
-#[serde(bound = "E: EthSpec", untagged)]
+#[context_deserialize(ForkName)]
-#[ssz(enum_behaviour = "transparent")]
+#[serde(bound = "E: EthSpec")]
-#[tree_hash(enum_behaviour = "transparent")]
 pub struct ExecutionPayloadEnvelope<E: EthSpec> {
-    #[superstruct(only(Gloas), partial_getter(rename = "payload_gloas"))]
-    pub payload: ExecutionPayloadGloas<E>,
-    #[superstruct(only(NextFork), partial_getter(rename = "payload_next_fork"))]
     pub payload: ExecutionPayloadGloas<E>,
     pub execution_requests: ExecutionRequests<E>,
     #[serde(with = "serde_utils::quoted_u64")]
-    #[superstruct(getter(copy))]
     pub builder_index: u64,
-    #[superstruct(getter(copy))]
     pub beacon_block_root: Hash256,
-    #[superstruct(getter(copy))]
     pub slot: Slot,
     pub blob_kzg_commitments: KzgCommitments<E>,
-    #[superstruct(getter(copy))]
     pub state_root: Hash256,
 }
 
 impl<E: EthSpec> SignedRoot for ExecutionPayloadEnvelope<E> {}
-impl<'a, E: EthSpec> SignedRoot for ExecutionPayloadEnvelopeRef<'a, E> {}
-
-impl<'a, E: EthSpec> ExecutionPayloadEnvelopeRef<'a, E> {
-    pub fn payload(&self) -> ExecutionPayloadRef<'a, E> {
-        match self {
-            Self::Gloas(envelope) => ExecutionPayloadRef::Gloas(&envelope.payload),
-            Self::NextFork(envelope) => ExecutionPayloadRef::Gloas(&envelope.payload),
-        }
-    }
-}
-
-impl<'de, E: EthSpec> ContextDeserialize<'de, ForkName> for ExecutionPayloadEnvelope<E> {
-    fn context_deserialize<D>(deserializer: D, context: ForkName) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let value: Self = serde::Deserialize::deserialize(deserializer)?;
-
-        match (context, &value) {
-            (ForkName::Gloas, Self::Gloas { .. }) => Ok(value),
-            _ => Err(D::Error::custom(format!(
-                "ExecutionPayloadEnvelope does not support fork {context:?}"
-            ))),
-        }
-    }
-}
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::MainnetEthSpec;
 
-    mod gloas {
+    ssz_and_tree_hash_tests!(ExecutionPayloadEnvelope<MainnetEthSpec>);
-        use super::*;
-        ssz_and_tree_hash_tests!(ExecutionPayloadEnvelopeGloas<MainnetEthSpec>);
-    }
-
-    mod next_fork {
-        use super::*;
-        ssz_and_tree_hash_tests!(ExecutionPayloadEnvelopeNextFork<MainnetEthSpec>);
-    }
 }

consensus/types/src/lib.rs

@@ -186,10 +186,7 @@ pub use crate::execution_payload::{
     Transaction, Transactions, Withdrawals,
 };
 pub use crate::execution_payload_bid::ExecutionPayloadBid;
-pub use crate::execution_payload_envelope::{
+pub use crate::execution_payload_envelope::ExecutionPayloadEnvelope;
-    ExecutionPayloadEnvelope, ExecutionPayloadEnvelopeGloas, ExecutionPayloadEnvelopeNextFork,
-    ExecutionPayloadEnvelopeRef,
-};
 pub use crate::execution_payload_header::{
     ExecutionPayloadHeader, ExecutionPayloadHeaderBellatrix, ExecutionPayloadHeaderCapella,
     ExecutionPayloadHeaderDeneb, ExecutionPayloadHeaderElectra, ExecutionPayloadHeaderFulu,

consensus/types/src/signed_execution_payload_envelope.rs

@@ -1,109 +1,23 @@
 use crate::test_utils::TestRandom;
 use crate::*;
 use educe::Educe;
-use serde::de::{Deserializer, Error as _};
 use serde::{Deserialize, Serialize};
 use ssz_derive::{Decode, Encode};
-use superstruct::superstruct;
 use test_random_derive::TestRandom;
 use tree_hash_derive::TreeHash;
 
-#[superstruct(
+#[derive(Debug, Clone, Serialize, Encode, Decode, Deserialize, TestRandom, TreeHash, Educe)]
-    variants(Gloas, NextFork),
-    variant_attributes(
-        derive(
-            Debug,
-            Clone,
-            Serialize,
-            Deserialize,
-            Encode,
-            Decode,
-            TreeHash,
-            TestRandom,
-            Educe
-        ),
-        cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary)),
-        educe(PartialEq, Hash(bound(E: EthSpec))),
-        serde(bound = "E: EthSpec", deny_unknown_fields),
-        cfg_attr(feature = "arbitrary", arbitrary(bound = "E: EthSpec"))
-    ),
-    ref_attributes(
-        derive(Debug, PartialEq, TreeHash),
-        tree_hash(enum_behaviour = "transparent")
-    ),
-    cast_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant"),
-    partial_getter_error(ty = "Error", expr = "BeaconStateError::IncorrectStateVariant")
-)]
-#[derive(Debug, Clone, Serialize, Encode, Deserialize, TreeHash, Educe)]
 #[educe(PartialEq, Hash(bound(E: EthSpec)))]
-#[serde(bound = "E: EthSpec", untagged)]
+#[serde(bound = "E: EthSpec")]
-#[ssz(enum_behaviour = "transparent")]
-#[tree_hash(enum_behaviour = "transparent")]
 pub struct SignedExecutionPayloadEnvelope<E: EthSpec> {
-    #[superstruct(only(Gloas), partial_getter(rename = "message_gloas"))]
+    pub message: ExecutionPayloadEnvelope<E>,
-    pub message: ExecutionPayloadEnvelopeGloas<E>,
-    #[superstruct(only(NextFork), partial_getter(rename = "message_next_fork"))]
-    pub message: crate::execution_payload_envelope::ExecutionPayloadEnvelopeNextFork<E>,
     pub signature: Signature,
 }
 
-impl<E: EthSpec> SignedExecutionPayloadEnvelope<E> {
-    /// Create a new `SignedExecutionPayloadEnvelope` from an `ExecutionPayloadEnvelope` and `Signature`.
-    pub fn from_envelope(envelope: ExecutionPayloadEnvelope<E>, signature: Signature) -> Self {
-        match envelope {
-            ExecutionPayloadEnvelope::Gloas(message) => SignedExecutionPayloadEnvelope::Gloas(
-                signed_execution_payload_envelope::SignedExecutionPayloadEnvelopeGloas {
-                    message,
-                    signature,
-                },
-            ),
-            ExecutionPayloadEnvelope::NextFork(message) => {
-                SignedExecutionPayloadEnvelope::NextFork(
-                    signed_execution_payload_envelope::SignedExecutionPayloadEnvelopeNextFork {
-                        message,
-                        signature,
-                    },
-                )
-            }
-        }
-    }
-
-    pub fn message(&self) -> ExecutionPayloadEnvelopeRef<'_, E> {
-        match self {
-            Self::Gloas(signed) => ExecutionPayloadEnvelopeRef::Gloas(&signed.message),
-            Self::NextFork(signed) => ExecutionPayloadEnvelopeRef::NextFork(&signed.message),
-        }
-    }
-}
-
-impl<'de, E: EthSpec> ContextDeserialize<'de, ForkName> for SignedExecutionPayloadEnvelope<E> {
-    fn context_deserialize<D>(deserializer: D, context: ForkName) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let value: Self = Deserialize::deserialize(deserializer)?;
-
-        match (context, &value) {
-            (ForkName::Gloas, Self::Gloas { .. }) => Ok(value),
-            _ => Err(D::Error::custom(format!(
-                "SignedExecutionPayloadEnvelope does not support fork {context:?}"
-            ))),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::MainnetEthSpec;
 
-    mod gloas {
+    ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelope<MainnetEthSpec>);
-        use super::*;
-        ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelopeGloas<MainnetEthSpec>);
-    }
-
-    mod next_fork {
-        use super::*;
-        ssz_and_tree_hash_tests!(SignedExecutionPayloadEnvelopeNextFork<MainnetEthSpec>);
-    }
 }