Repositioning blob hash validation in newPayload of engine API (#2141)

hive_integration/nodocker/engine/cancun/customizer.nim

@@ -425,7 +425,7 @@ proc customizePayload*(cust: CustomPayloadData, data: ExecutableData): Executabl
   if cust.versionedHashesCustomizer.isNil.not:
     doAssert(data.versionedHashes.isSome)
     result.versionedHashes = cust.versionedHashesCustomizer.getVersionedHashes(data.versionedHashes.get)
-  
+
 # Base new payload directive call cust.
 # Used as base to other customizers.
 type
@@ -615,22 +615,22 @@ proc generateInvalidPayload*(sender: TxSender, data: ExecutableData, payloadFiel
       excessBlobGas: some(modExcessBlobGas),
     )
   of InvalidVersionedHashesVersion:
-    doAssert(data.versionedHashes.isNone, "no versioned hashes available for modification")
+    doAssert(data.versionedHashes.isSome, "no versioned hashes available for modification")
     customPayloadMod = CustomPayloadData(
       versionedHashesCustomizer: IncreaseVersionVersionedHashes(),
     )
   of InvalidVersionedHashes:
-    doAssert(data.versionedHashes.isNone, "no versioned hashes available for modification")
+    doAssert(data.versionedHashes.isSome, "no versioned hashes available for modification")
     customPayloadMod = CustomPayloadData(
       versionedHashesCustomizer: CorruptVersionedHashes(),
     )
   of IncompleteVersionedHashes:
-    doAssert(data.versionedHashes.isNone, "no versioned hashes available for modification")
+    doAssert(data.versionedHashes.isSome, "no versioned hashes available for modification")
     customPayloadMod = CustomPayloadData(
       versionedHashesCustomizer: RemoveVersionedHash(),
     )
   of ExtraVersionedHashes:
-    doAssert(data.versionedHashes.isNone, "no versioned hashes available for modification")
+    doAssert(data.versionedHashes.isSome, "no versioned hashes available for modification")
     customPayloadMod = CustomPayloadData(
       versionedHashesCustomizer: ExtraVersionedHash(),
     )

hive_integration/nodocker/engine/cancun/step_sendblobtx.nim

@@ -1,5 +1,5 @@
 # Nimbus
-# Copyright (c) 2023 Status Research & Development GmbH
+# Copyright (c) 2023-2024 Status Research & Development GmbH
 # Licensed under either of
 #  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
 #    http://www.apache.org/licenses/LICENSE-2.0)

hive_integration/nodocker/engine/cancun_tests.nim

@@ -1865,7 +1865,7 @@ proc makeCancunTest(): seq[EngineSpec] =
       mainFork: ForkCancun,
       fieldModification: t,
     )
-#[
+
   # Invalid Payload Tests
   const
     invalidFields = [
@@ -1911,7 +1911,6 @@ proc makeCancunTest(): seq[EngineSpec] =
         invalidDetectedOnSync: invalidDetectedOnSync,
         nilLatestValidHash   : nilLatestValidHash,
       )
-  ]#
 
   # Invalid Transaction ChainID Tests
   result.add InvalidTxChainIDTest(

hive_integration/nodocker/engine/tx_sender.nim

@@ -22,11 +22,16 @@ import
 
 type
   BaseTx* = object of RootObj
-    recipient*: Option[EthAddress]
+    recipient* : Option[EthAddress]
-    gasLimit* : GasInt
+    gasLimit*  : GasInt
-    amount*   : UInt256
+    amount*    : UInt256
-    payload*  : seq[byte]
+    payload*   : seq[byte]
-    txType*   : Option[TxType]
+    txType*    : Option[TxType]
+    gasTip*    : GasInt
+    gasFee*    : GasInt
+    blobGasFee*: UInt256
+    blobCount* : int
+    blobID*    : BlobID
 
   BigInitcodeTx* = object of BaseTx
     initcodeLength*: int
@@ -35,11 +40,6 @@ type
 
   # Blob transaction creator
   BlobTx* = object of BaseTx
-    gasFee*    : GasInt
-    gasTip*    : GasInt
-    blobGasFee*: UInt256
-    blobID*    : BlobID
-    blobCount* : int
 
   TestAccount* = object
     key*    : PrivateKey
@@ -77,6 +77,7 @@ const
   TestAccountCount = 1000
   gasPrice* = 30.gwei
   gasTipPrice* = 1.gwei
+  blobGasPrice* = 1.gwei
 
 func toAddress(key: PrivateKey): EthAddress =
   toKeyPair(key).pubkey.toCanonicalAddress()
@@ -129,37 +130,75 @@ proc getTxType(tc: BaseTx, nonce: uint64): TxType =
   else:
     tc.txType.get
 
-proc makeTx(params: MakeTxParams, tc: BaseTx): Transaction =
+proc makeTxOfType(params: MakeTxParams, tc: BaseTx): Transaction =
-  const
+  let
-    gasFeeCap = gasPrice
+    gasFeeCap = if tc.gasFee != 0.GasInt: tc.gasFee
-    gasTipCap = gasTipPrice
+                else: gasPrice
+    gasTipCap = if tc.gasTip != 0.GasInt: tc.gasTip
+                else: gasTipPrice
 
   let txType = tc.getTxType(params.nonce)
+  case txType
+  of TxLegacy:
+    Transaction(
+      txType  : TxLegacy,
+      nonce   : params.nonce,
+      to      : tc.recipient,
+      value   : tc.amount,
+      gasLimit: tc.gasLimit,
+      gasPrice: gasPrice,
+      payload : tc.payload
+    )
+  of TxEip1559:
+    Transaction(
+      txType  : TxEIP1559,
+      nonce   : params.nonce,
+      gasLimit: tc.gasLimit,
+      maxFee  : gasFeeCap,
+      maxPriorityFee: gasTipCap,
+      to      : tc.recipient,
+      value   : tc.amount,
+      payload : tc.payload,
+      chainId : params.chainId
+    )
+  of TxEip4844:
+    doAssert(tc.recipient.isSome, "recipient must be some")
+    let
+      blobCount  = if tc.blobCount != 0: tc.blobCount
+                   else: MAX_BLOBS_PER_BLOCK
+      blobFeeCap = if tc.blobGasFee != 0.u256: tc.blobGasFee
+                   else: blobGasPrice.u256
 
+    # Need tx wrap data that will pass blob verification
+    var blobData = blobDataGenerator(tc.blobID, blobCount)
+    #tc.blobID += BlobID(blobCount)
+
+    Transaction(
+      txType  : TxEIP4844,
+      nonce   : params.nonce,
+      chainId : params.chainId,
+      maxFee  : gasFeeCap,
+      maxPriorityFee: gasTipCap,
+      gasLimit: tc.gasLimit,
+      to      : tc.recipient,
+      value   : tc.amount,
+      payload : tc.payload,
+      #AccessList: tc.AccessList,
+      maxFeePerBlobGas: blobFeeCap,
+      versionedHashes: system.move(blobData.hashes),
+      networkPayload: NetworkPayload(
+        blobs: system.move(blobData.blobs),
+        commitments: system.move(blobData.commitments),
+        proofs: system.move(blobData.proofs),
+      )
+    )
+  else:
+    doAssert(false, "unsupported tx type")
+    Transaction()
+
+proc makeTx(params: MakeTxParams, tc: BaseTx): Transaction =
   # Build the transaction depending on the specified type
-  let tx = if txType == TxLegacy:
+  let tx = makeTxOfType(params, tc)
-             Transaction(
-               txType  : TxLegacy,
-               nonce   : params.nonce,
-               to      : tc.recipient,
-               value   : tc.amount,
-               gasLimit: tc.gasLimit,
-               gasPrice: gasPrice,
-               payload : tc.payload
-             )
-           else:
-             Transaction(
-               txType  : TxEIP1559,
-               nonce   : params.nonce,
-               gasLimit: tc.gasLimit,
-               maxFee  : gasFeeCap,
-               maxPriorityFee: gasTipCap,
-               to      : tc.recipient,
-               value   : tc.amount,
-               payload : tc.payload,
-               chainId : params.chainId
-             )
-
   signTransaction(tx, params.key, params.chainId, eip155 = true)
 
 proc makeTx(params: MakeTxParams, tc: BigInitcodeTx): Transaction =
@@ -263,13 +302,13 @@ proc makeTx*(params: MakeTxParams, tc: BlobTx): Transaction =
   let data = blobDataGenerator(tc.blobID, tc.blobCount)
   doAssert(tc.recipient.isSome, "nil recipient address")
 
-  # Collect fields for transaction
   let
     gasFeeCap = if tc.gasFee != 0.GasInt: tc.gasFee
                 else: gasPrice
     gasTipCap = if tc.gasTip != 0.GasInt: tc.gasTip
                 else: gasTipPrice
 
+  # Collect fields for transaction
   let unsignedTx = Transaction(
     txType    : TxEip4844,
     chainId   : params.chainId,

nimbus/beacon/api_handler.nim

@@ -8,11 +8,6 @@
 # those terms.
 
 import
-  std/[options, typetraits],
-  eth/common,
-  ./web3_eth_conv,
-  ./beacon_engine,
-  web3/execution_types,
   ./api_handler/api_utils,
   ./api_handler/api_getpayload,
   ./api_handler/api_getbodies,
@@ -24,25 +19,6 @@ import
 # Public functions
 # ------------------------------------------------------------------------------
 
-{.push gcsafe, raises:[CatchableError].}
-
-func validateVersionedHashed*(payload: ExecutionPayload,
-                              expected: openArray[Web3Hash]): bool  =
-  var versionedHashes: seq[common.Hash256]
-  for x in payload.transactions:
-    let tx = rlp.decode(distinctBase(x), Transaction)
-    versionedHashes.add tx.versionedHashes
-
-  if versionedHashes.len != expected.len:
-    return false
-
-  for i, x in expected:
-    if distinctBase(x) != versionedHashes[i].data:
-      return false
-  true
-
-{.pop.}
-
 export
   invalidStatus,
   getPayload,

nimbus/beacon/api_handler/api_newpayload.nim

@@ -19,6 +19,21 @@ import
 
 {.push gcsafe, raises:[CatchableError].}
 
+func validateVersionedHashed(payload: ExecutionPayload,
+                              expected: openArray[Web3Hash]): bool  =
+  var versionedHashes: seq[common.Hash256]
+  for x in payload.transactions:
+    let tx = rlp.decode(distinctBase(x), Transaction)
+    versionedHashes.add tx.versionedHashes
+
+  if versionedHashes.len != expected.len:
+    return false
+
+  for i, x in expected:
+    if distinctBase(x) != versionedHashes[i].data:
+      return false
+  true
+
 template validateVersion(com, timestamp, version, apiVersion) =
   if apiVersion == Version.V4:
     if not com.isPragueOrLater(timestamp):
@@ -83,6 +98,7 @@ template validatePayload(apiVersion, version, payload) =
 proc newPayload*(ben: BeaconEngineRef,
                  apiVersion: Version,
                  payload: ExecutionPayload,
+                 versionedHashes = none(seq[Web3Hash]),
                  beaconRoot = none(Web3Hash)): PayloadStatusV1 =
 
   trace "Engine API request received",
@@ -90,7 +106,7 @@ proc newPayload*(ben: BeaconEngineRef,
     number = payload.blockNumber,
     hash = payload.blockHash
 
-  if apiVersion == Version.V3:
+  if apiVersion >= Version.V3:
     if beaconRoot.isNone:
       raise invalidParams("newPayloadV3 expect beaconRoot but got none")
 
@@ -108,6 +124,12 @@ proc newPayload*(ben: BeaconEngineRef,
   header.validateBlockHash(blockHash, version).isOkOr:
     return error
 
+  if apiVersion >= Version.V3:
+    if versionedHashes.isNone:
+      raise invalidParams("newPayload" & $apiVersion &
+        " expect blobVersionedHashes but got none")
+    if not validateVersionedHashed(payload, versionedHashes.get):
+      return invalidStatus(header.parentHash, "invalid blob versionedHashes")
 
   # If we already have the block locally, ignore the entire execution and just
   # return a fake success.

nimbus/rpc/engine_api.nim

@@ -13,8 +13,7 @@ import
   web3/[conversions, execution_types],
   ../beacon/api_handler,
   ../beacon/beacon_engine,
-  ../beacon/web3_eth_conv,
+  ../beacon/web3_eth_conv
-  ../beacon/api_handler/api_utils
 
 {.push raises: [].}
 
@@ -53,20 +52,12 @@ proc setupEngineAPI*(engine: BeaconEngineRef, server: RpcServer) =
   server.rpc("engine_newPayloadV3") do(payload: ExecutionPayload,
                                        expectedBlobVersionedHashes: Option[seq[Web3Hash]],
                                        parentBeaconBlockRoot: Option[Web3Hash]) -> PayloadStatusV1:
-    if expectedBlobVersionedHashes.isNone:
+    return engine.newPayload(Version.V3, payload, expectedBlobVersionedHashes, parentBeaconBlockRoot)
-      raise invalidParams("newPayloadV3 expect blobVersionedHashes but got none")
-    if not validateVersionedHashed(payload, expectedBlobVersionedHashes.get):
-      return invalidStatus()
-    return engine.newPayload(Version.V3, payload, parentBeaconBlockRoot)
 
   server.rpc("engine_newPayloadV4") do(payload: ExecutionPayload,
                                        expectedBlobVersionedHashes: Option[seq[Web3Hash]],
                                        parentBeaconBlockRoot: Option[Web3Hash]) -> PayloadStatusV1:
-    if expectedBlobVersionedHashes.isNone:
+    return engine.newPayload(Version.V4, payload, expectedBlobVersionedHashes, parentBeaconBlockRoot)
-      raise invalidParams("newPayloadV4 expect blobVersionedHashes but got none")
-    if not validateVersionedHashed(payload, expectedBlobVersionedHashes.get):
-      return invalidStatus()
-    return engine.newPayload(Version.V4, payload, parentBeaconBlockRoot)
 
   server.rpc("engine_getPayloadV1") do(payloadId: PayloadID) -> ExecutionPayloadV1:
     return engine.getPayload(Version.V1, payloadId).executionPayload.V1