Fixed Payload Reconstruction Bug (#3796)

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -939,6 +939,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             Some(DatabaseBlock::Blinded(block)) => block,
             None => return Ok(None),
         };
+        let fork = blinded_block.fork_name(&self.spec)?;
 
         // If we only have a blinded block, load the execution payload from the EL.
         let block_message = blinded_block.message();
@@ -953,7 +954,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             .execution_layer
             .as_ref()
             .ok_or(Error::ExecutionLayerMissing)?
-            .get_payload_by_block_hash(exec_block_hash)
+            .get_payload_by_block_hash(exec_block_hash, fork)
             .await
             .map_err(|e| Error::ExecutionLayerErrorPayloadReconstruction(exec_block_hash, e))?
             .ok_or(Error::BlockHashMissingFromExecutionLayer(exec_block_hash))?;

beacon_node/beacon_chain/src/errors.rs

@@ -207,6 +207,7 @@ pub enum BeaconChainError {
     CommitteePromiseFailed(oneshot_broadcast::Error),
     MaxCommitteePromises(usize),
     BlsToExecutionChangeBadFork(ForkName),
+    InconsistentFork(InconsistentFork),
 }
 
 easy_from_to!(SlotProcessingError, BeaconChainError);
@@ -230,6 +231,7 @@ easy_from_to!(ForkChoiceStoreError, BeaconChainError);
 easy_from_to!(HistoricalBlockError, BeaconChainError);
 easy_from_to!(StateAdvanceError, BeaconChainError);
 easy_from_to!(BlockReplayError, BeaconChainError);
+easy_from_to!(InconsistentFork, BeaconChainError);
 
 #[derive(Debug)]
 pub enum BlockProductionError {

beacon_node/execution_layer/src/engine_api/http.rs

@@ -664,14 +664,41 @@ impl HttpJsonRpc {
     pub async fn get_block_by_hash_with_txns<T: EthSpec>(
         &self,
         block_hash: ExecutionBlockHash,
+        fork: ForkName,
     ) -> Result<Option<ExecutionBlockWithTransactions<T>>, Error> {
         let params = json!([block_hash, true]);
-        self.rpc_request(
-            ETH_GET_BLOCK_BY_HASH,
-            params,
-            ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
-        )
-        .await
+        Ok(Some(match fork {
+            ForkName::Merge => ExecutionBlockWithTransactions::Merge(
+                self.rpc_request(
+                    ETH_GET_BLOCK_BY_HASH,
+                    params,
+                    ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
+                )
+                .await?,
+            ),
+            ForkName::Capella => ExecutionBlockWithTransactions::Capella(
+                self.rpc_request(
+                    ETH_GET_BLOCK_BY_HASH,
+                    params,
+                    ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
+                )
+                .await?,
+            ),
+            ForkName::Eip4844 => ExecutionBlockWithTransactions::Eip4844(
+                self.rpc_request(
+                    ETH_GET_BLOCK_BY_HASH,
+                    params,
+                    ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
+                )
+                .await?,
+            ),
+            ForkName::Base | ForkName::Altair => {
+                return Err(Error::UnsupportedForkVariant(format!(
+                    "called get_block_by_hash_with_txns with fork {:?}",
+                    fork
+                )))
+            }
+        }))
     }
 
     pub async fn new_payload_v1<T: EthSpec>(

beacon_node/execution_layer/src/lib.rs

@@ -1550,10 +1550,11 @@ impl<T: EthSpec> ExecutionLayer<T> {
     pub async fn get_payload_by_block_hash(
         &self,
         hash: ExecutionBlockHash,
+        fork: ForkName,
     ) -> Result<Option<ExecutionPayload<T>>, Error> {
         self.engine()
             .request(|engine| async move {
-                self.get_payload_by_block_hash_from_engine(engine, hash)
+                self.get_payload_by_block_hash_from_engine(engine, hash, fork)
                     .await
             })
             .await
@@ -1565,15 +1566,26 @@ impl<T: EthSpec> ExecutionLayer<T> {
         &self,
         engine: &Engine,
         hash: ExecutionBlockHash,
+        fork: ForkName,
     ) -> Result<Option<ExecutionPayload<T>>, ApiError> {
         let _timer = metrics::start_timer(&metrics::EXECUTION_LAYER_GET_PAYLOAD_BY_BLOCK_HASH);
 
         if hash == ExecutionBlockHash::zero() {
-            // FIXME: how to handle forks properly here?
-            return Ok(Some(ExecutionPayloadMerge::default().into()));
+            return match fork {
+                ForkName::Merge => Ok(Some(ExecutionPayloadMerge::default().into())),
+                ForkName::Capella => Ok(Some(ExecutionPayloadCapella::default().into())),
+                ForkName::Eip4844 => Ok(Some(ExecutionPayloadEip4844::default().into())),
+                ForkName::Base | ForkName::Altair => Err(ApiError::UnsupportedForkVariant(
+                    format!("called get_payload_by_block_hash_from_engine with {}", fork),
+                )),
+            };
         }
 
-        let block = if let Some(block) = engine.api.get_block_by_hash_with_txns::<T>(hash).await? {
+        let block = if let Some(block) = engine
+            .api
+            .get_block_by_hash_with_txns::<T>(hash, fork)
+            .await?
+        {
             block
         } else {
             return Ok(None);

testing/execution_engine_integration/src/test_rig.rs

@@ -616,7 +616,8 @@ async fn check_payload_reconstruction<E: GenericExecutionEngine>(
 ) {
     let reconstructed = ee
         .execution_layer
-        .get_payload_by_block_hash(payload.block_hash())
+        // FIXME: handle other forks here?
+        .get_payload_by_block_hash(payload.block_hash(), ForkName::Merge)
         .await
         .unwrap()
         .unwrap();