Remove doing advanced state transition in importBlock function

ChainSafe · Jun 28, 2022 · 1e403c6 · 1e403c6
1 parent 54402ee
commit 1e403c6
Showing 1 changed file with 26 additions and 63 deletions.
diff --git a/packages/lodestar/src/chain/blocks/importBlock.ts b/packages/lodestar/src/chain/blocks/importBlock.ts
@@ -7,23 +7,20 @@ import {
   computeStartSlotAtEpoch,
   getEffectiveBalanceIncrementsZeroInactive,
   computeEpochAtSlot,
-  isBellatrixStateType,
   RootCache,
-  processSlots,
 } from "@chainsafe/lodestar-beacon-state-transition";
 import {IForkChoice, OnBlockPrecachedData, ForkChoiceError, ForkChoiceErrorCode} from "@chainsafe/lodestar-fork-choice";
 import {ILogger} from "@chainsafe/lodestar-utils";
 import {IChainForkConfig} from "@chainsafe/lodestar-config";
 import {IMetrics} from "../../metrics/index.js";
-import {IExecutionEngine, PayloadId} from "../../executionEngine/interface.js";
+import {IExecutionEngine} from "../../executionEngine/interface.js";
 import {IBeaconDb} from "../../db/index.js";
 import {ZERO_HASH_HEX} from "../../constants/index.js";
 import {CheckpointStateCache, StateContextCache, toCheckpointHex} from "../stateCache/index.js";
 import {ChainEvent} from "../emitter.js";
 import {ChainEventEmitter} from "../emitter.js";
 import {LightClientServer} from "../lightClient/index.js";
 import {SeenAggregatedAttestations} from "../seenCache/seenAggregateAndProof.js";
-import {prepareExecutionPayload} from "../factory/block/body.js";
 import {IEth1ForBlockProduction} from "../../eth1/index.js";
 import {BeaconProposerCache} from "../beaconProposerCache.js";
 import {IBeaconClock} from "../clock/index.js";
@@ -219,34 +216,32 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
     }
   }
 
-  void maybeIssueNextProposerEngineFcU(chain, postState).then((payloadId) => {
-    // NOTE: forkChoice.fsStore.finalizedCheckpoint MUST only change is response to an onBlock event
-    // Notify execution layer of head and finalized updates only if has already
-    // not been done via payloadId generation. But even if this fcU follows the
-    // payloadId one, there is no harm as the ELs will just ignore it.
-    if (payloadId === null && (newHead.blockRoot !== oldHead.blockRoot || currFinalizedEpoch !== prevFinalizedEpoch)) {
-      /**
-       * On post BELLATRIX_EPOCH but pre TTD, blocks include empty execution payload with a zero block hash.
-       * The consensus clients must not send notifyForkchoiceUpdate before TTD since the execution client will error.
-       * So we must check that:
-       * - `headBlockHash !== null` -> Pre BELLATRIX_EPOCH
-       * - `headBlockHash !== ZERO_HASH` -> Pre TTD
-       */
-      const headBlockHash = chain.forkChoice.getHead().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-      /**
-       * After BELLATRIX_EPOCH and TTD it's okay to send a zero hash block hash for the finalized block. This will happen if
-       * the current finalized block does not contain any execution payload at all (pre MERGE_EPOCH) or if it contains a
-       * zero block hash (pre TTD)
-       */
-      const safeBlockHash = chain.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-      const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-      if (headBlockHash !== ZERO_HASH_HEX) {
-        chain.executionEngine.notifyForkchoiceUpdate(headBlockHash, safeBlockHash, finalizedBlockHash).catch((e) => {
-          chain.logger.error("Error pushing notifyForkchoiceUpdate()", {headBlockHash, finalizedBlockHash}, e);
-        });
-      }
+  // NOTE: forkChoice.fsStore.finalizedCheckpoint MUST only change in response to an onBlock event
+  // Notifying EL of head and finalized updates as below is usually done within the 1st 4s of the slot.
+  // If there is an advanced payload generation in the next slot, we'll notify EL again 4s before next
+  // slot via PrepareNextSlotScheduler. There is no harm updating the ELs with same data, it will just ignore it.
+  if (newHead.blockRoot !== oldHead.blockRoot || currFinalizedEpoch !== prevFinalizedEpoch) {
+    /**
+     * On post BELLATRIX_EPOCH but pre TTD, blocks include empty execution payload with a zero block hash.
+     * The consensus clients must not send notifyForkchoiceUpdate before TTD since the execution client will error.
+     * So we must check that:
+     * - `headBlockHash !== null` -> Pre BELLATRIX_EPOCH
+     * - `headBlockHash !== ZERO_HASH` -> Pre TTD
+     */
+    const headBlockHash = chain.forkChoice.getHead().executionPayloadBlockHash ?? ZERO_HASH_HEX;
+    /**
+     * After BELLATRIX_EPOCH and TTD it's okay to send a zero hash block hash for the finalized block. This will happen if
+     * the current finalized block does not contain any execution payload at all (pre MERGE_EPOCH) or if it contains a
+     * zero block hash (pre TTD)
+     */
+    const safeBlockHash = chain.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
+    const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
+    if (headBlockHash !== ZERO_HASH_HEX) {
+      chain.executionEngine.notifyForkchoiceUpdate(headBlockHash, safeBlockHash, finalizedBlockHash).catch((e) => {
+        chain.logger.error("Error pushing notifyForkchoiceUpdate()", {headBlockHash, finalizedBlockHash}, e);
+      });
     }
-  });
+  }
 
   // Emit ChainEvent.block event
   //
@@ -268,38 +263,6 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
   chain.metrics?.parentBlockDistance.observe(block.message.slot - parentBlock.slot);
 }
 
-async function maybeIssueNextProposerEngineFcU(
-  chain: ImportBlockModules,
-  state: CachedBeaconStateAllForks
-): Promise<PayloadId | null> {
-  const prepareSlot = state.slot + 1;
-  const prepareEpoch = computeEpochAtSlot(prepareSlot);
-  // No need to try building block if we are not synced
-  if (prepareSlot !== chain.clock.currentSlot + 1 || prepareEpoch < chain.config.BELLATRIX_FORK_EPOCH) {
-    return null;
-  }
-  const prepareState = processSlots(state, prepareSlot);
-  // TODO wait till third/last interval of the slot to actual send an fcU
-  // so that any head change is accomodated before that. However this could
-  // be optimized if the last block receieved is already head. This will be
-  // especially meaningful for mev boost which might have more delays
-  // because of how protocol is designed
-  if (isBellatrixStateType(prepareState)) {
-    try {
-      const proposerIndex = prepareState.epochCtx.getBeaconProposer(prepareSlot);
-      const feeRecipient = chain.beaconProposerCache.get(proposerIndex);
-      if (feeRecipient) {
-        const safeBlockHash = chain.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-        const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-        return prepareExecutionPayload(chain, safeBlockHash, finalizedBlockHash, prepareState, feeRecipient);
-      }
-    } catch (e) {
-      chain.logger.error("Error on issuing next proposer engine fcU", {}, e as Error);
-    }
-  }
-  return null;
-}
-
 /**
  * Returns the closest state to postState.currentJustifiedCheckpoint in the same fork as postState
  *