Use justified blocks execution payload hash as safeBlockHash in engin…

…e fcU (#4080)

* Use justified blocks execution payload hash as safeBlockHash in engine fcU

* fix test

* safe block hash fix

* run merge ci on latest

.github/workflows/test-sim-merge.yml

@@ -3,8 +3,8 @@ name: Sim merge tests
 on: [pull_request, push]
 
 env:
-  GETH_COMMIT: bb5633c5ee3975ce016636066ec790054ec469e4
-  NETHERMIND_COMMIT: 00b50532543824dbac65e8b7ab09484e44992c27
+  GETH_COMMIT: be9742721f56eb8bb7ebf4f6a03fb01b13a05408
+  NETHERMIND_COMMIT: 82f331a3e7ff21712a5f839e0a62ba7c16110e44
 
 jobs:
   sim-merge-tests:
@@ -59,7 +59,7 @@ jobs:
         with:
           dotnet-version: "6.0.x"
       - name: Clone Nethermind merge interop branch
-        run: git clone -b kiln https://github.com/g11tech/nethermind --recursive && cd nethermind && git reset --hard $NETHERMIND_COMMIT && git submodule update --init --recursive
+        run: git clone -b kiln-rebased https://github.com/g11tech/nethermind --recursive && cd nethermind && git reset --hard $NETHERMIND_COMMIT && git submodule update --init --recursive
       - name: Build Nethermind
         run: cd nethermind/src/Nethermind && dotnet build Nethermind.sln -c Release
 

packages/lodestar/src/chain/blocks/importBlock.ts

@@ -237,9 +237,10 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
        * 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, finalizedBlockHash).catch((e) => {
+        chain.executionEngine.notifyForkchoiceUpdate(headBlockHash, safeBlockHash, finalizedBlockHash).catch((e) => {
           chain.logger.error("Error pushing notifyForkchoiceUpdate()", {headBlockHash, finalizedBlockHash}, e);
         });
       }
@@ -284,8 +285,9 @@ async function maybeIssueNextProposerEngineFcU(
       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, finalizedBlockHash, prepareState, feeRecipient);
+        return prepareExecutionPayload(chain, safeBlockHash, finalizedBlockHash, prepareState, feeRecipient);
       }
     } catch (e) {
       chain.logger.error("Error on issuing next proposer engine fcU", {}, e as Error);

packages/lodestar/src/chain/factory/block/body.ts

@@ -88,29 +88,20 @@ export async function assembleBody(
   }
 
   if (blockEpoch >= chain.config.BELLATRIX_FORK_EPOCH) {
-    // TODO: Optimize this flow
-    // - Call prepareExecutionPayload as early as possible
-    // - Call prepareExecutionPayload again if parameters change
-
-    const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash;
+    const safeBlockHash = chain.forkChoice.getJustifiedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
+    const finalizedBlockHash = chain.forkChoice.getFinalizedBlock().executionPayloadBlockHash ?? ZERO_HASH_HEX;
     const feeRecipient = chain.beaconProposerCache.getOrDefault(proposerIndex);
 
     // prepareExecutionPayload will throw error via notifyForkchoiceUpdate if
     // the EL returns Syncing on this request to prepare a payload
     //
-    // TODO:
-    // The payloadId should be extracted from the ones cached in the execution engine
-    // by the advance firing of the fcU. If no entry in the cache is available then
-    // continue with the usual firing, but this will most likely not generate a full
-    // block. However some timing consideration can be done here to bundle some time
-    // for the same.
-    //
-    // For MeV boost integration as well, this is where the execution header will be
+    // For MeV boost integration, this is where the execution header will be
     // fetched from the payload id and a blinded block will be produced instead of
     // fullblock for the validator to sign
     const payloadId = await prepareExecutionPayload(
       chain,
-      finalizedBlockHash ?? ZERO_HASH_HEX,
+      safeBlockHash,
+      finalizedBlockHash,
       currentState as CachedBeaconStateBellatrix,
       feeRecipient
     );
@@ -139,6 +130,7 @@ export async function prepareExecutionPayload(
     executionEngine: IExecutionEngine;
     config: IChainForkConfig;
   },
+  safeBlockHash: RootHex,
   finalizedBlockHash: RootHex,
   state: CachedBeaconStateBellatrix,
   suggestedFeeRecipient: string
@@ -180,7 +172,7 @@ export async function prepareExecutionPayload(
       prevRandao: toHex(prevRandao),
       suggestedFeeRecipient,
     }) ??
-    (await chain.executionEngine.notifyForkchoiceUpdate(parentHash, finalizedBlockHash, {
+    (await chain.executionEngine.notifyForkchoiceUpdate(parentHash, safeBlockHash, finalizedBlockHash, {
       timestamp,
       prevRandao,
       suggestedFeeRecipient,

packages/lodestar/src/executionEngine/http.ts

@@ -191,6 +191,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
    */
   async notifyForkchoiceUpdate(
     headBlockHash: Root | RootHex,
+    safeBlockHash: RootHex,
     finalizedBlockHash: RootHex,
     payloadAttributes?: PayloadAttributes
   ): Promise<PayloadId | null> {
@@ -211,10 +212,7 @@ export class ExecutionEngineHttp implements IExecutionEngine {
       payloadId,
     } = await this.rpc.fetch<EngineApiRpcReturnTypes[typeof method], EngineApiRpcParamTypes[typeof method]>({
       method,
-      params: [
-        {headBlockHash: headBlockHashData, safeBlockHash: headBlockHashData, finalizedBlockHash},
-        apiPayloadAttributes,
-      ],
+      params: [{headBlockHash: headBlockHashData, safeBlockHash, finalizedBlockHash}, apiPayloadAttributes],
     });
 
     switch (status) {

packages/lodestar/src/executionEngine/interface.ts

@@ -81,6 +81,7 @@ export interface IExecutionEngine {
    */
   notifyForkchoiceUpdate(
     headBlockHash: Root | RootHex,
+    safeBlockHash: RootHex,
     finalizedBlockHash: RootHex,
     payloadAttributes?: PayloadAttributes
   ): Promise<PayloadId | null>;

packages/lodestar/src/executionEngine/mock.ts

@@ -82,6 +82,7 @@ export class ExecutionEngineMock implements IExecutionEngine {
    */
   async notifyForkchoiceUpdate(
     headBlockHash: Root,
+    safeBlockHash: RootHex,
     finalizedBlockHash: RootHex,
     payloadAttributes?: PayloadAttributes
   ): Promise<PayloadId> {

packages/lodestar/test/sim/merge-interop.test.ts

@@ -176,6 +176,8 @@ describe("executionEngine / ExecutionEngineHttp", function () {
 
     const payloadId = await executionEngine.notifyForkchoiceUpdate(
       genesisBlockHash,
+      //use finalizedBlockHash as safeBlockHash
+      finalizedBlockHash,
       finalizedBlockHash,
       preparePayloadParams
     );
@@ -211,7 +213,7 @@ describe("executionEngine / ExecutionEngineHttp", function () {
      * curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"engine_forkchoiceUpdatedV1","params":[{"headBlockHash":"0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858", "safeBlockHash":"0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858", "finalizedBlockHash":"0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a"}, null],"id":67}' http://localhost:8550
      **/
 
-    await executionEngine.notifyForkchoiceUpdate(bytesToData(payload.blockHash), genesisBlockHash);
+    await executionEngine.notifyForkchoiceUpdate(bytesToData(payload.blockHash), genesisBlockHash, genesisBlockHash);
 
     if (TX_SCENARIOS.includes("simple")) {
       const balance = await getBalance(jsonRpcUrl, "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");

packages/lodestar/test/unit/executionEngine/http.test.ts

@@ -140,6 +140,7 @@ describe("ExecutionEngine / http", () => {
 
     await executionEngine.notifyForkchoiceUpdate(
       forkChoiceHeadData.headBlockHash,
+      forkChoiceHeadData.safeBlockHash,
       forkChoiceHeadData.finalizedBlockHash
     );