From fa8e2bd7b78a6c8340483aa74687a68c2dd2f5bb Mon Sep 17 00:00:00 2001
From: acolytec3 <17355484+acolytec3@users.noreply.github.com>
Date: Thu, 13 Oct 2022 10:09:50 -0400
Subject: [PATCH] Add invalid scenario checks and tests

---
 package-lock.json                     |  2 +-
 packages/tx/src/eip4844Transaction.ts | 37 ++++++++++++++++++---
 packages/tx/src/types.ts              |  3 ++
 packages/tx/test/eip4844.spec.ts      | 46 +++++++++++++++++++++++++--
 4 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 0a8d7b5c82..c418bb4197 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -20408,7 +20408,7 @@
     "@ethereumjs/tx": {
       "version": "file:packages/tx",
       "requires": {
-        "@chainsafe/ssz": "*",
+        "@chainsafe/ssz": "^0.9.2",
         "@ethereumjs/common": "^3.0.0",
         "@ethereumjs/rlp": "^4.0.0",
         "@ethereumjs/util": "^8.0.0",
diff --git a/packages/tx/src/eip4844Transaction.ts b/packages/tx/src/eip4844Transaction.ts
index 35224f7712..dada471e2b 100644
--- a/packages/tx/src/eip4844Transaction.ts
+++ b/packages/tx/src/eip4844Transaction.ts
@@ -2,7 +2,7 @@ import { toHexString } from '@chainsafe/ssz'
 import { Address, MAX_INTEGER, bufferToBigInt, toBuffer } from '@ethereumjs/util'
 
 import { BaseTransaction } from './baseTransaction'
-import { BlobTransactionType } from './types'
+import { BLOB_COMMITMENT_VERSION_KZG, BlobTransactionType, MAX_BLOBS_PER_TX } from './types'
 import { AccessLists, checkMaxInitCodeSize } from './util'
 
 import type {
@@ -79,6 +79,21 @@ export class BlobEIP4844Transaction extends BaseTransaction<BlobEIP4844Transacti
       checkMaxInitCodeSize(this.common, this.data.length)
     }
 
+    for (const hash of txData.versionedHashes) {
+      if (hash.length !== 32) {
+        const msg = this._errorMsg('versioned hash is invalid length')
+        throw new Error(msg)
+      }
+      if (hash[0] !== BLOB_COMMITMENT_VERSION_KZG) {
+        const msg = this._errorMsg('versioned hash does not start with KZG commitment version')
+        throw new Error(msg)
+      }
+    }
+    if (txData.versionedHashes.length > MAX_BLOBS_PER_TX) {
+      const msg = this._errorMsg(`tx can contain at most ${MAX_BLOBS_PER_TX} blobs`)
+      throw new Error(msg)
+    }
+
     this.versionedHashes = txData.versionedHashes
 
     const freeze = opts?.freeze ?? true
@@ -161,10 +176,22 @@ export class BlobEIP4844Transaction extends BaseTransaction<BlobEIP4844Transacti
   _processSignature(_v: bigint, _r: Buffer, _s: Buffer): BlobEIP4844Transaction {
     throw new Error('Method not implemented.')
   }
-  public errorStr(): string {
-    throw new Error('Method not implemented.')
+  /**
+   * Return a compact error string representation of the object
+   */
+  public errorStr() {
+    let errorStr = this._getSharedErrorPostfix()
+    errorStr += ` maxFeePerGas=${this.maxFeePerGas} maxPriorityFeePerGas=${this.maxPriorityFeePerGas}`
+    return errorStr
   }
-  _errorMsg(_msg: string): string {
-    throw new Error('Method not implemented.')
+
+  /**
+   * Internal helper function to create an annotated error message
+   *
+   * @param msg Base error message
+   * @hidden
+   */
+  protected _errorMsg(msg: string) {
+    return `${msg} (${this.errorStr()})`
   }
 }
diff --git a/packages/tx/src/types.ts b/packages/tx/src/types.ts
index 728de25ef3..396988723a 100644
--- a/packages/tx/src/types.ts
+++ b/packages/tx/src/types.ts
@@ -314,9 +314,12 @@ export interface JsonRpcTx {
 
 /** EIP4844 constants */
 
+// TODO: Decide which of these should be in the hardfork params instead of hardcoded as constants here
+export const BLOB_COMMITMENT_VERSION_KZG = 0x01
 export const MAX_CALLDATA_SIZE = 2 ** 24
 export const MAX_ACCESS_LIST_SIZE = 2 ** 24
 export const MAX_VERSIONED_HASHES_LIST_SIZE = 2 ** 24
+export const MAX_BLOBS_PER_TX = 2
 
 /** EIP4844 types */
 export const AddressType = new ByteVectorType(20) // SSZ encoded address
diff --git a/packages/tx/test/eip4844.spec.ts b/packages/tx/test/eip4844.spec.ts
index 165d35c7dc..d699decfba 100644
--- a/packages/tx/test/eip4844.spec.ts
+++ b/packages/tx/test/eip4844.spec.ts
@@ -1,11 +1,12 @@
+import { randomBytes } from 'crypto'
 import * as tape from 'tape'
 
 import { BlobEIP4844Transaction, TransactionFactory } from '../src'
 
-tape('EIP4844 constructor tests', (t) => {
+tape('EIP4844 constructor tests - valid scenarios', (t) => {
   const txData = {
     type: 0x05,
-    versionedHashes: [Buffer.from([])],
+    versionedHashes: [Buffer.concat([Buffer.from([1]), randomBytes(31)])],
   }
   const tx = BlobEIP4844Transaction.fromTxData(txData)
   t.equal(tx.type, 5, 'successfully instantiated a blob transaction from txData')
@@ -18,3 +19,44 @@ tape('EIP4844 constructor tests', (t) => {
   t.equal(deserializedTx.type, 5, 'deserialized a blob tx')
   t.end()
 })
+
+tape('EIP4844 constructor tests - invalid scenarios', (t) => {
+  const baseTxData = {
+    type: 0x05,
+  }
+  const shortVersionHash = {
+    versionedHashes: [Buffer.concat([Buffer.from([3]), randomBytes(3)])],
+  }
+  const invalidVersionHash = {
+    versionedHashes: [Buffer.concat([Buffer.from([3]), randomBytes(31)])],
+  }
+  const tooManyBlobs = {
+    versionedHashes: [
+      Buffer.concat([Buffer.from([1]), randomBytes(31)]),
+      Buffer.concat([Buffer.from([1]), randomBytes(31)]),
+      Buffer.concat([Buffer.from([1]), randomBytes(31)]),
+    ],
+  }
+  try {
+    BlobEIP4844Transaction.fromTxData({ ...baseTxData, ...shortVersionHash })
+  } catch (err: any) {
+    t.ok(
+      err.message.includes('versioned hash is invalid length'),
+      'throws on invalid versioned hash length'
+    )
+  }
+  try {
+    BlobEIP4844Transaction.fromTxData({ ...baseTxData, ...invalidVersionHash })
+  } catch (err: any) {
+    t.ok(
+      err.message.includes('does not start with KZG commitment'),
+      'throws on invalid commitment version'
+    )
+  }
+  try {
+    BlobEIP4844Transaction.fromTxData({ ...baseTxData, ...tooManyBlobs })
+  } catch (err: any) {
+    t.ok(err.message.includes('tx can contain at most'), 'throws on too many versioned hashes')
+  }
+  t.end()
+})