From 75dd8780e0a7d8e095a7d92dba8aab30308b40de Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Wed, 14 Dec 2022 11:52:46 +1100
Subject: [PATCH] Use JsonPayload for payload reconstruction (#3797)

---
 beacon_node/execution_layer/src/engine_api.rs | 44 ++++++++++++-------
 beacon_node/execution_layer/src/lib.rs        | 20 +++++++--
 .../test_utils/execution_block_generator.rs   |  2 +-
 3 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs
index 988b04826..424ca30d1 100644
--- a/beacon_node/execution_layer/src/engine_api.rs
+++ b/beacon_node/execution_layer/src/engine_api.rs
@@ -1,10 +1,11 @@
 use crate::engines::ForkchoiceState;
 pub use ethers_core::types::Transaction;
-use ethers_core::utils::rlp::{Decodable, Rlp};
+use ethers_core::utils::rlp::{self, Decodable, Rlp};
 use http::deposit_methods::RpcError;
-pub use json_structures::TransitionConfigurationV1;
+pub use json_structures::{JsonWithdrawal, TransitionConfigurationV1};
 use reqwest::StatusCode;
 use serde::{Deserialize, Serialize};
+use std::convert::TryFrom;
 use strum::IntoStaticStr;
 use superstruct::superstruct;
 pub use types::{
@@ -46,6 +47,7 @@ pub enum Error {
     RequiredMethodUnsupported(&'static str),
     UnsupportedForkVariant(String),
     BadConversion(String),
+    RlpDecoderError(rlp::DecoderError),
 }
 
 impl From<reqwest::Error> for Error {
@@ -79,6 +81,12 @@ impl From<builder_client::Error> for Error {
     }
 }
 
+impl From<rlp::DecoderError> for Error {
+    fn from(e: rlp::DecoderError) -> Self {
+        Error::RlpDecoderError(e)
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, IntoStaticStr)]
 #[strum(serialize_all = "snake_case")]
 pub enum PayloadStatusV1Status {
@@ -159,12 +167,14 @@ pub struct ExecutionBlockWithTransactions<T: EthSpec> {
     pub transactions: Vec<Transaction>,
     #[cfg(feature = "withdrawals")]
     #[superstruct(only(Capella, Eip4844))]
-    pub withdrawals: Vec<Withdrawal>,
+    pub withdrawals: Vec<JsonWithdrawal>,
 }
 
-impl<T: EthSpec> From<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T> {
-    fn from(payload: ExecutionPayload<T>) -> Self {
-        match payload {
+impl<T: EthSpec> TryFrom<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T> {
+    type Error = Error;
+
+    fn try_from(payload: ExecutionPayload<T>) -> Result<Self, Error> {
+        let json_payload = match payload {
             ExecutionPayload::Merge(block) => Self::Merge(ExecutionBlockWithTransactionsMerge {
                 parent_hash: block.parent_hash,
                 fee_recipient: block.fee_recipient,
@@ -183,8 +193,7 @@ impl<T: EthSpec> From<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T>
                     .transactions
                     .iter()
                     .map(|tx| Transaction::decode(&Rlp::new(tx)))
-                    .collect::<Result<Vec<_>, _>>()
-                    .unwrap_or_else(|_| Vec::new()),
+                    .collect::<Result<Vec<_>, _>>()?,
             }),
             ExecutionPayload::Capella(block) => {
                 Self::Capella(ExecutionBlockWithTransactionsCapella {
@@ -205,10 +214,12 @@ impl<T: EthSpec> From<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T>
                         .transactions
                         .iter()
                         .map(|tx| Transaction::decode(&Rlp::new(tx)))
-                        .collect::<Result<Vec<_>, _>>()
-                        .unwrap_or_else(|_| Vec::new()),
+                        .collect::<Result<Vec<_>, _>>()?,
                     #[cfg(feature = "withdrawals")]
-                    withdrawals: block.withdrawals.into(),
+                    withdrawals: Vec::from(block.withdrawals)
+                        .into_iter()
+                        .map(|withdrawal| withdrawal.into())
+                        .collect(),
                 })
             }
             ExecutionPayload::Eip4844(block) => {
@@ -231,13 +242,16 @@ impl<T: EthSpec> From<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T>
                         .transactions
                         .iter()
                         .map(|tx| Transaction::decode(&Rlp::new(tx)))
-                        .collect::<Result<Vec<_>, _>>()
-                        .unwrap_or_else(|_| Vec::new()),
+                        .collect::<Result<Vec<_>, _>>()?,
                     #[cfg(feature = "withdrawals")]
-                    withdrawals: block.withdrawals.into(),
+                    withdrawals: Vec::from(block.withdrawals)
+                        .into_iter()
+                        .map(|withdrawal| withdrawal.into())
+                        .collect(),
                 })
             }
-        }
+        };
+        Ok(json_payload)
     }
 }
 
diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
index 2aaa7608e..1980e82ce 100644
--- a/beacon_node/execution_layer/src/lib.rs
+++ b/beacon_node/execution_layer/src/lib.rs
@@ -1622,8 +1622,14 @@ impl<T: EthSpec> ExecutionLayer<T> {
             }
             ExecutionBlockWithTransactions::Capella(capella_block) => {
                 #[cfg(feature = "withdrawals")]
-                let withdrawals = VariableList::new(capella_block.withdrawals.clone())
-                    .map_err(ApiError::DeserializeWithdrawals)?;
+                let withdrawals = VariableList::new(
+                    capella_block
+                        .withdrawals
+                        .into_iter()
+                        .map(|w| w.into())
+                        .collect(),
+                )
+                .map_err(ApiError::DeserializeWithdrawals)?;
 
                 ExecutionPayload::Capella(ExecutionPayloadCapella {
                     parent_hash: capella_block.parent_hash,
@@ -1646,8 +1652,14 @@ impl<T: EthSpec> ExecutionLayer<T> {
             }
             ExecutionBlockWithTransactions::Eip4844(eip4844_block) => {
                 #[cfg(feature = "withdrawals")]
-                let withdrawals = VariableList::new(eip4844_block.withdrawals.clone())
-                    .map_err(ApiError::DeserializeWithdrawals)?;
+                let withdrawals = VariableList::new(
+                    eip4844_block
+                        .withdrawals
+                        .into_iter()
+                        .map(|w| w.into())
+                        .collect(),
+                )
+                .map_err(ApiError::DeserializeWithdrawals)?;
 
                 ExecutionPayload::Eip4844(ExecutionPayloadEip4844 {
                     parent_hash: eip4844_block.parent_hash,
diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs
index f2282c603..a7ec429e4 100644
--- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs
+++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs
@@ -76,7 +76,7 @@ impl<T: EthSpec> Block<T> {
 
     pub fn as_execution_block_with_tx(&self) -> Option<ExecutionBlockWithTransactions<T>> {
         match self {
-            Block::PoS(payload) => Some(payload.clone().into()),
+            Block::PoS(payload) => Some(payload.clone().try_into().unwrap()),
             Block::PoW(_) => None,
         }
     }