client: fixes for new engine api method validations for hive pr-834 (#…

…2973) * client: fixes for new engine api method validations for hive pr-834 * fix spec * handle null values hive sends * fix remaining errors * return blobs of already build payload * fix client spec * small fix --------- Co-authored-by: Holger Drewes <[email protected]>
ethereumjs · Aug 22, 2023 · 26c5179 · 26c5179
1 parent 6f8ad22
commit 26c5179
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 23 deletions.
diff --git a/packages/client/src/miner/pendingBlock.ts b/packages/client/src/miner/pendingBlock.ts
@@ -225,7 +225,12 @@ export class PendingBlock {
     }
     const blockStatus = builder.getStatus()
     if (blockStatus.status === BuildStatus.Build) {
-      return [blockStatus.block, builder.transactionReceipts, builder.minerValue]
+      return [
+        blockStatus.block,
+        builder.transactionReceipts,
+        builder.minerValue,
+        this.blobsBundles.get(payloadId),
+      ]
     }
     const { vm, headerData } = builder as unknown as { vm: VM; headerData: HeaderData }
 

diff --git a/packages/client/src/rpc/error-code.ts b/packages/client/src/rpc/error-code.ts
@@ -7,3 +7,15 @@ export const INVALID_PARAMS = -32602
 export const INTERNAL_ERROR = -32603
 export const TOO_LARGE_REQUEST = -38004
 export const UNSUPPORTED_FORK = -38005
+export const UNKNOWN_PAYLOAD = -32001
+
+export const validEngineCodes = [
+  PARSE_ERROR,
+  INVALID_REQUEST,
+  METHOD_NOT_FOUND,
+  INVALID_PARAMS,
+  INTERNAL_ERROR,
+  TOO_LARGE_REQUEST,
+  UNSUPPORTED_FORK,
+  UNKNOWN_PAYLOAD,
+]
diff --git a/packages/client/src/rpc/modules/engine.ts b/packages/client/src/rpc/modules/engine.ts
@@ -13,7 +13,14 @@ import {
 
 import { PendingBlock } from '../../miner'
 import { short } from '../../util'
-import { INTERNAL_ERROR, INVALID_PARAMS, TOO_LARGE_REQUEST, UNSUPPORTED_FORK } from '../error-code'
+import {
+  INTERNAL_ERROR,
+  INVALID_PARAMS,
+  TOO_LARGE_REQUEST,
+  UNKNOWN_PAYLOAD,
+  UNSUPPORTED_FORK,
+  validEngineCodes,
+} from '../error-code'
 import { CLConnectionManager, middleware as cmMiddleware } from '../util/CLConnectionManager'
 import { middleware, validators } from '../validation'
 
@@ -24,6 +31,7 @@ import type { VMExecution } from '../../execution'
 import type { BlobsBundle } from '../../miner'
 import type { FullEthereumService } from '../../service'
 import type { ExecutionPayload } from '@ethereumjs/block'
+import type { Common } from '@ethereumjs/common'
 import type { VM } from '@ethereumjs/vm'
 
 const zeroBlockHash = zeros(32)
@@ -103,7 +111,7 @@ type ExecutionPayloadBodyV1 = {
 
 const EngineError = {
   UnknownPayload: {
-    code: -32001,
+    code: UNKNOWN_PAYLOAD,
     message: 'Unknown payload',
   },
 }
@@ -360,6 +368,34 @@ const getPayloadBody = (block: Block): ExecutionPayloadBodyV1 => {
   }
 }
 
+function validateHardforkRange(
+  chainCommon: Common,
+  methodVersion: number,
+  checkNotBeforeHf: Hardfork | null,
+  checkNotAfterHf: Hardfork | null,
+  timestamp: bigint
+) {
+  if (checkNotBeforeHf !== null) {
+    const hfTimeStamp = chainCommon.hardforkTimestamp(checkNotBeforeHf)
+    if (hfTimeStamp !== null && timestamp < hfTimeStamp) {
+      throw {
+        code: UNSUPPORTED_FORK,
+        message: `V${methodVersion} cannot be called pre-${checkNotBeforeHf}`,
+      }
+    }
+  }
+
+  if (checkNotAfterHf !== null) {
+    const nextHFTimestamp = chainCommon.nextHardforkBlockOrTimestamp(checkNotAfterHf)
+    if (nextHFTimestamp !== null && timestamp >= nextHFTimestamp) {
+      throw {
+        code: UNSUPPORTED_FORK,
+        message: `V${methodVersion + 1} MUST be called post-${checkNotAfterHf}`,
+      }
+    }
+  }
+}
+
 /**
  * engine_* RPC module
  * @memberof module:rpc/modules
@@ -1090,6 +1126,26 @@ export class Engine {
   private async forkchoiceUpdatedV1(
     params: [forkchoiceState: ForkchoiceStateV1, payloadAttributes: PayloadAttributesV1 | undefined]
   ): Promise<ForkchoiceResponseV1 & { headBlock?: Block }> {
+    const payloadAttributes = params[1]
+    if (payloadAttributes !== undefined && payloadAttributes !== null) {
+      if (
+        Object.values(payloadAttributes).filter((attr) => attr !== null && attr !== undefined)
+          .length > 3
+      ) {
+        throw {
+          code: INVALID_PARAMS,
+          message: 'PayloadAttributesV1 MUST be used for forkchoiceUpdatedV2',
+        }
+      }
+      validateHardforkRange(
+        this.chain.config.chainCommon,
+        1,
+        null,
+        Hardfork.Paris,
+        BigInt(payloadAttributes.timestamp)
+      )
+    }
+
     return this.forkchoiceUpdated(params)
   }
 
@@ -1101,6 +1157,24 @@ export class Engine {
   ): Promise<ForkchoiceResponseV1 & { headBlock?: Block }> {
     const payloadAttributes = params[1]
     if (payloadAttributes !== undefined && payloadAttributes !== null) {
+      if (
+        Object.values(payloadAttributes).filter((attr) => attr !== null && attr !== undefined)
+          .length > 4
+      ) {
+        throw {
+          code: INVALID_PARAMS,
+          message: 'PayloadAttributesV{1|2} MUST be used for forkchoiceUpdatedV2',
+        }
+      }
+
+      validateHardforkRange(
+        this.chain.config.chainCommon,
+        2,
+        null,
+        Hardfork.Shanghai,
+        BigInt(payloadAttributes.timestamp)
+      )
+
       const shanghaiTimestamp = this.chain.config.chainCommon.hardforkTimestamp(Hardfork.Shanghai)
       const ts = BigInt(payloadAttributes.timestamp)
       const withdrawals = (payloadAttributes as PayloadAttributesV2).withdrawals
@@ -1137,14 +1211,24 @@ export class Engine {
   ): Promise<ForkchoiceResponseV1 & { headBlock?: Block }> {
     const payloadAttributes = params[1]
     if (payloadAttributes !== undefined && payloadAttributes !== null) {
-      const cancunTimestamp = this.chain.config.chainCommon.hardforkTimestamp(Hardfork.Cancun)
-      const ts = BigInt(payloadAttributes.timestamp)
-      if (ts < cancunTimestamp!) {
+      if (
+        Object.values(payloadAttributes).filter((attr) => attr !== null && attr !== undefined)
+          .length > 5
+      ) {
         throw {
-          code: UNSUPPORTED_FORK,
-          message: 'PayloadAttributesV{1|2} MUST be used before Cancun is activated',
+          code: INVALID_PARAMS,
+          message: 'PayloadAttributesV3 MUST be used for forkchoiceUpdatedV3',
         }
       }
+
+      validateHardforkRange(
+        this.chain.config.chainCommon,
+        3,
+        Hardfork.Cancun,
+        // this could be valid post cancun as well, if not then update the valid till hf here
+        null,
+        BigInt(payloadAttributes.timestamp)
+      )
     }
 
     return this.forkchoiceUpdated(params)
@@ -1158,7 +1242,7 @@ export class Engine {
    *   1. payloadId: DATA, 8 bytes - identifier of the payload building process
    * @returns Instance of {@link ExecutionPayloadV1} or an error
    */
-  private async getPayload(params: [Bytes8], payloadVersion?: number) {
+  private async getPayload(params: [Bytes8], payloadVersion: number) {
     const payloadId = params[0]
     try {
       const built = await this.pendingBlock.build(payloadId)
@@ -1178,22 +1262,40 @@ export class Engine {
       this.executedBlocks.set(bytesToUnprefixedHex(block.hash()), block)
       const executionPayload = blockToExecutionPayload(block, value, blobs)
 
-      if (payloadVersion === 3) {
-        const cancunTimestamp = this.chain.config.chainCommon.hardforkTimestamp(Hardfork.Cancun)
-        if (
-          cancunTimestamp !== null &&
-          BigInt(executionPayload.executionPayload.timestamp) < cancunTimestamp
-        ) {
-          throw {
-            code: UNSUPPORTED_FORK,
-            message: 'getPayloadV3 cannot be called on payloads pre-Cancun',
-          }
-        }
+      let checkNotBeforeHf: Hardfork | null
+      let checkNotAfterHf: Hardfork | null
+
+      switch (payloadVersion) {
+        case 3:
+          checkNotBeforeHf = Hardfork.Cancun
+          checkNotAfterHf = Hardfork.Cancun
+          break
+
+        case 2:
+          // no checks to be done for before as valid till paris
+          checkNotBeforeHf = null
+          checkNotAfterHf = Hardfork.Shanghai
+          break
+
+        case 1:
+          checkNotBeforeHf = null
+          checkNotAfterHf = Hardfork.Paris
+          break
+
+        default:
+          throw Error(`Invalid payloadVersion=${payloadVersion}`)
       }
 
+      validateHardforkRange(
+        this.chain.config.chainCommon,
+        payloadVersion,
+        checkNotBeforeHf,
+        checkNotAfterHf,
+        BigInt(executionPayload.executionPayload.timestamp)
+      )
       return executionPayload
     } catch (error: any) {
-      if (error === EngineError.UnknownPayload) throw error
+      if (validEngineCodes.includes(error.code)) throw error
       throw {
         code: INTERNAL_ERROR,
         message: error.message ?? error,
@@ -1202,12 +1304,12 @@ export class Engine {
   }
 
   async getPayloadV1(params: [Bytes8]) {
-    const { executionPayload } = await this.getPayload(params)
+    const { executionPayload } = await this.getPayload(params, 1)
     return executionPayload
   }
 
   async getPayloadV2(params: [Bytes8]) {
-    const { executionPayload, blockValue } = await this.getPayload(params)
+    const { executionPayload, blockValue } = await this.getPayload(params, 2)
     return { executionPayload, blockValue }
   }