Remove Transaction enum wrapper

beacon_node/execution_layer/src/engine_api/http.rs

@@ -433,11 +433,10 @@ pub mod serde_transactions {
 
             while let Some(val) = seq.next_element::<String>()? {
                 let inner_vec = hex::decode(&val).map_err(de::Error::custom)?;
-                let opaque_transaction = VariableList::new(inner_vec).map_err(|e| {
+                let transaction = VariableList::new(inner_vec).map_err(|e| {
                     serde::de::Error::custom(format!("transaction too large: {:?}", e))
                 })?;
-                let transaction = Transaction::OpaqueTransaction(opaque_transaction);
-                outer.push(transaction).map_err(|e| {
+                outer.push(Transaction::new(transaction)).map_err(|e| {
                     serde::de::Error::custom(format!("too many transactions: {:?}", e))
                 })?;
             }
@@ -458,9 +457,7 @@ pub mod serde_transactions {
             // It's important to match on the inner values of the transaction. Serializing the
             // entire `Transaction` will result in appending the SSZ union prefix byte. The
             // execution node does not want that.
-            let hex = match transaction {
-                Transaction::OpaqueTransaction(val) => hex::encode(&val[..]),
-            };
+            let hex = hex::encode(transaction.as_bytes());
             seq.serialize_element(&hex)?;
         }
         seq.end()
@@ -611,7 +608,7 @@ mod test {
     }
 
     /// Example: if `spec == &[1, 1]`, then two one-byte transactions will be created.
-    fn generate_opaque_transactions<E: EthSpec>(
+    fn generate_transactions<E: EthSpec>(
         spec: &[usize],
     ) -> VariableList<Transaction<E>, E::MaxTransactionsPerPayload> {
         let mut txs = VariableList::default();
@@ -621,42 +618,38 @@ mod test {
             for _ in 0..num_bytes {
                 tx.push(0).unwrap();
             }
-            txs.push(Transaction::OpaqueTransaction(tx)).unwrap();
+            txs.push(Transaction::new(tx)).unwrap();
         }
 
         txs
     }
 
     #[test]
     fn transaction_serde() {
-        assert_transactions_serde::<MainnetEthSpec>(
-            "empty",
-            generate_opaque_transactions(&[]),
-            json!([]),
-        );
+        assert_transactions_serde::<MainnetEthSpec>("empty", generate_transactions(&[]), json!([]));
         assert_transactions_serde::<MainnetEthSpec>(
             "one empty tx",
-            generate_opaque_transactions(&[0]),
+            generate_transactions(&[0]),
             json!(["0x"]),
         );
         assert_transactions_serde::<MainnetEthSpec>(
             "two empty txs",
-            generate_opaque_transactions(&[0, 0]),
+            generate_transactions(&[0, 0]),
             json!(["0x", "0x"]),
         );
         assert_transactions_serde::<MainnetEthSpec>(
             "one one-byte tx",
-            generate_opaque_transactions(&[1]),
+            generate_transactions(&[1]),
             json!(["0x00"]),
         );
         assert_transactions_serde::<MainnetEthSpec>(
             "two one-byte txs",
-            generate_opaque_transactions(&[1, 1]),
+            generate_transactions(&[1, 1]),
             json!(["0x00", "0x00"]),
         );
         assert_transactions_serde::<MainnetEthSpec>(
             "mixed bag",
-            generate_opaque_transactions(&[0, 1, 3, 0]),
+            generate_transactions(&[0, 1, 3, 0]),
             json!(["0x", "0x00", "0x000000", "0x"]),
         );
 
@@ -680,7 +673,7 @@ mod test {
 
         use eth2_serde_utils::hex;
 
-        let num_max_bytes = <MainnetEthSpec as EthSpec>::MaxBytesPerOpaqueTransaction::to_usize();
+        let num_max_bytes = <MainnetEthSpec as EthSpec>::MaxBytesPerTransaction::to_usize();
         let max_bytes = (0..num_max_bytes).map(|_| 0_u8).collect::<Vec<_>>();
         let too_many_bytes = (0..=num_max_bytes).map(|_| 0_u8).collect::<Vec<_>>();
         decode_transactions::<MainnetEthSpec>(

consensus/types/src/eth_spec.rs

@@ -3,13 +3,13 @@ use crate::*;
 use safe_arith::SafeArith;
 use serde_derive::{Deserialize, Serialize};
 use ssz_types::typenum::{
-    Unsigned, U0, U1024, U1099511627776, U128, U16, U16777216, U2, U2048, U32, U4, U4096, U512,
-    U64, U65536, U8, U8192,
+    Unsigned, U0, U1024, U1073741824, U1099511627776, U128, U16, U16777216, U2, U2048, U32, U4,
+    U4096, U512, U64, U65536, U8, U8192,
 };
 use std::fmt::{self, Debug};
 use std::str::FromStr;
 
-use ssz_types::typenum::{bit::B0, UInt, U1048576, U16384, U256, U625};
+use ssz_types::typenum::{bit::B0, UInt, U1048576, U256, U625};
 pub type U5000 = UInt<UInt<UInt<U625, B0>, B0>, B0>; // 625 * 8 = 5000
 
 const MAINNET: &str = "mainnet";
@@ -86,7 +86,7 @@ pub trait EthSpec: 'static + Default + Sync + Send + Clone + Debug + PartialEq +
     /*
      * New in Merge
      */
-    type MaxBytesPerOpaqueTransaction: Unsigned + Clone + Sync + Send + Debug + PartialEq;
+    type MaxBytesPerTransaction: Unsigned + Clone + Sync + Send + Debug + PartialEq;
     type MaxTransactionsPerPayload: Unsigned + Clone + Sync + Send + Debug + PartialEq;
     type BytesPerLogsBloom: Unsigned + Clone + Sync + Send + Debug + PartialEq;
     type GasLimitDenominator: Unsigned + Clone + Sync + Send + Debug + PartialEq;
@@ -200,9 +200,9 @@ pub trait EthSpec: 'static + Default + Sync + Send + Clone + Debug + PartialEq +
         Self::SyncSubcommitteeSize::to_usize()
     }
 
-    /// Returns the `MAX_BYTES_PER_OPAQUE_TRANSACTION` constant for this specification.
-    fn max_bytes_per_opaque_transaction() -> usize {
-        Self::MaxBytesPerOpaqueTransaction::to_usize()
+    /// Returns the `MAX_BYTES_PER_TRANSACTION` constant for this specification.
+    fn max_bytes_per_transaction() -> usize {
+        Self::MaxBytesPerTransaction::to_usize()
     }
 
     /// Returns the `MAX_TRANSACTIONS_PER_PAYLOAD` constant for this specification.
@@ -258,8 +258,8 @@ impl EthSpec for MainnetEthSpec {
     type MaxVoluntaryExits = U16;
     type SyncCommitteeSize = U512;
     type SyncCommitteeSubnetCount = U4;
-    type MaxBytesPerOpaqueTransaction = U1048576;
-    type MaxTransactionsPerPayload = U16384;
+    type MaxBytesPerTransaction = U1073741824; // 1,073,741,824
+    type MaxTransactionsPerPayload = U1048576; // 1,048,576
     type BytesPerLogsBloom = U256;
     type GasLimitDenominator = U1024;
     type MinGasLimit = U5000;
@@ -306,7 +306,7 @@ impl EthSpec for MinimalEthSpec {
         MaxAttestations,
         MaxDeposits,
         MaxVoluntaryExits,
-        MaxBytesPerOpaqueTransaction,
+        MaxBytesPerTransaction,
         MaxTransactionsPerPayload,
         BytesPerLogsBloom,
         GasLimitDenominator,

consensus/types/src/execution_payload.rs

@@ -1,39 +1,85 @@
 use crate::{test_utils::TestRandom, *};
+use rand::RngCore;
 use serde_derive::{Deserialize, Serialize};
+use ssz::{Decode, DecodeError, Encode};
 use ssz_derive::{Decode, Encode};
-use std::{ops::Index, slice::SliceIndex};
 use test_random_derive::TestRandom;
 use tree_hash_derive::TreeHash;
 
 #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Encode, Decode, TreeHash)]
-#[ssz(enum_behaviour = "union")]
-#[tree_hash(enum_behaviour = "union")]
-#[serde(tag = "selector", content = "value")]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(transparent)]
 #[serde(bound = "T: EthSpec")]
-pub enum Transaction<T: EthSpec> {
-    // FIXME(merge): renaming this enum variant to 0 is a bit of a hack...
-    #[serde(rename = "0")]
-    OpaqueTransaction(
-        #[serde(with = "ssz_types::serde_utils::hex_var_list")]
-        VariableList<u8, T::MaxBytesPerOpaqueTransaction>,
-    ),
+pub struct Transaction<T: EthSpec>(
+    #[serde(with = "ssz_types::serde_utils::hex_var_list")]
+    VariableList<u8, T::MaxBytesPerTransaction>,
+);
+
+impl<T: EthSpec> Transaction<T> {
+    pub fn new(tx: VariableList<u8, T::MaxBytesPerTransaction>) -> Transaction<T> {
+        Self(tx)
+    }
+
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0[..]
+    }
 }
 
-impl<T: EthSpec, I: SliceIndex<[u8]>> Index<I> for Transaction<T> {
-    type Output = I::Output;
+impl<T: EthSpec> Encode for Transaction<T> {
+    fn is_ssz_fixed_len() -> bool {
+        <VariableList<u8, T::MaxBytesPerTransaction> as Encode>::is_ssz_fixed_len()
+    }
 
-    #[inline]
-    fn index(&self, index: I) -> &Self::Output {
-        match self {
-            Self::OpaqueTransaction(v) => Index::index(v, index),
-        }
+    fn ssz_append(&self, buf: &mut Vec<u8>) {
+        self.0.ssz_append(buf)
+    }
+
+    fn ssz_fixed_len() -> usize {
+        <VariableList<u8, T::MaxBytesPerTransaction> as Encode>::ssz_fixed_len()
+    }
+
+    fn ssz_bytes_len(&self) -> usize {
+        self.0.ssz_bytes_len()
+    }
+}
+
+impl<T: EthSpec> Decode for Transaction<T> {
+    fn is_ssz_fixed_len() -> bool {
+        <VariableList<u8, T::MaxBytesPerTransaction> as Decode>::is_ssz_fixed_len()
+    }
+
+    fn ssz_fixed_len() -> usize {
+        <VariableList<u8, T::MaxBytesPerTransaction> as Decode>::ssz_fixed_len()
+    }
+
+    fn from_ssz_bytes(bytes: &[u8]) -> Result<Self, DecodeError> {
+        Ok(Transaction(VariableList::from_ssz_bytes(bytes)?))
+    }
+}
+
+impl<T: EthSpec> tree_hash::TreeHash for Transaction<T> {
+    fn tree_hash_type() -> tree_hash::TreeHashType {
+        tree_hash::TreeHashType::Container
+    }
+
+    fn tree_hash_packed_encoding(&self) -> Vec<u8> {
+        unreachable!("List should never be packed.")
+    }
+
+    fn tree_hash_packing_factor() -> usize {
+        unreachable!("List should never be packed.")
+    }
+
+    fn tree_hash_root(&self) -> tree_hash::Hash256 {
+        self.0.tree_hash_root()
     }
 }
 
-impl<T: EthSpec> From<VariableList<u8, T::MaxBytesPerOpaqueTransaction>> for Transaction<T> {
-    fn from(list: VariableList<u8, <T as EthSpec>::MaxBytesPerOpaqueTransaction>) -> Self {
-        Self::OpaqueTransaction(list)
+impl<T: EthSpec> SignedRoot for Transaction<T> {}
+
+impl<T: EthSpec> TestRandom for Transaction<T> {
+    fn random_for_test(rng: &mut impl RngCore) -> Self {
+        Transaction(VariableList::random_for_test(rng))
     }
 }