fix the payload generation,fetch/caching

ChainSafe · May 3, 2022 · 8381e8f · 8381e8f
1 parent b73f5bf
commit 8381e8f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 15 deletions.
diff --git a/packages/lodestar/src/chain/blocks/importBlock.ts b/packages/lodestar/src/chain/blocks/importBlock.ts
@@ -1,6 +1,5 @@
 import {SLOTS_PER_EPOCH} from "@chainsafe/lodestar-params";
 import {toHexString} from "@chainsafe/ssz";
-import {allForks} from "@chainsafe/lodestar-types";
 import {
   CachedBeaconStateAllForks,
   CachedBeaconStateAltair,
@@ -9,6 +8,7 @@ import {
   bellatrix,
   altair,
   computeEpochAtSlot,
+  allForks,
 } from "@chainsafe/lodestar-beacon-state-transition";
 import {
   IForkChoice,
@@ -217,8 +217,10 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
 
   void issueNextProposerEngineFcU(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
-    if (payloadId === null || newHead.blockRoot !== oldHead.blockRoot || currFinalizedEpoch !== prevFinalizedEpoch) {
+    // 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.
@@ -278,15 +280,19 @@ export async function issueNextProposerEngineFcU(
   state: CachedBeaconStateAllForks
 ): Promise<PayloadId | null> {
   const prepareSlot = state.slot + 1;
-  // TODO: if prepareSlot is on boundary do the epoch processing
-  if (bellatrix.isBellatrixStateType(state) && prepareSlot% SLOTS_PER_EPOCH !== 0) {
+  const prepareState = allForks.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 (bellatrix.isBellatrixStateType(prepareState)) {
     try {
-      const proposerIndex = state.epochCtx.getBeaconProposer(prepareSlot);
+      const proposerIndex = prepareState.epochCtx.getBeaconProposer(prepareSlot);
       const feeRecipient = chain.executionEngine.proposers.get(proposerIndex)?.feeRecipient;
       if (feeRecipient) {
         const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
-        const payloadId = await prepareExecutionPayload(chain, finalizedBlockHash, state, feeRecipient);
-        return payloadId;
+        return prepareExecutionPayload(chain, finalizedBlockHash, prepareState, feeRecipient);
       }
     } catch (e) {
       chain.logger.error("Error on issuing next proposer engine fcU", {}, e as Error);

diff --git a/packages/lodestar/src/chain/factory/block/body.ts b/packages/lodestar/src/chain/factory/block/body.ts
@@ -31,6 +31,7 @@ import {IBeaconChain} from "../../interface";
 import {PayloadId, IExecutionEngine} from "../../../executionEngine/interface";
 import {ZERO_HASH, ZERO_HASH_HEX} from "../../../constants";
 import {IEth1ForBlockProduction} from "../../../eth1";
+import {numToQuantity} from "../../../eth1/provider/utils";
 
 export async function assembleBody(
   chain: IBeaconChain,
@@ -172,10 +173,11 @@ export async function prepareExecutionPayload(
   const timestamp = computeTimeAtSlot(chain.config, state.slot, state.genesisTime);
   const prevRandao = getRandaoMix(state, state.epochCtx.epoch);
 
+  const payloadIdKey = `${toHex(parentHash)}-${finalizedBlockHash}-${numToQuantity(timestamp)}-${toHex(
+    prevRandao
+  )}-${toHex(suggestedFeeRecipient)}`;
   const payloadId =
-    chain.executionEngine.payloadIdCache.get(
-      `${toHex(parentHash)}-${finalizedBlockHash}-${toHex(prevRandao)}-${toHex(suggestedFeeRecipient)}`
-    ) ??
+    chain.executionEngine.payloadIdCache.get(payloadIdKey) ??
     (await chain.executionEngine.notifyForkchoiceUpdate(parentHash, finalizedBlockHash, {
       timestamp,
       prevRandao,

diff --git a/packages/lodestar/src/executionEngine/http.ts b/packages/lodestar/src/executionEngine/http.ts
@@ -236,10 +236,10 @@ export class ExecutionEngineHttp implements IExecutionEngine {
           if (!payloadId || payloadId === "0x") {
             throw Error(`Received invalid payloadId=${payloadId}`);
           }
-          this.payloadIdCache.set(
-            `${headBlockHashData}-${finalizedBlockHash}-${apiPayloadAttributes.prevRandao}-${apiPayloadAttributes.suggestedFeeRecipient}`,
-            payloadId
-          );
+
+          const payloadIdKey = `${headBlockHashData}-${finalizedBlockHash}-${apiPayloadAttributes.timestamp}-${apiPayloadAttributes.prevRandao}-${apiPayloadAttributes.suggestedFeeRecipient}`;
+
+          this.payloadIdCache.set(payloadIdKey, payloadId);
         }
         return payloadId !== "0x" ? payloadId : null;