util: remove undefined handling from bytesToHex (#4004)

* util: document and remove undefined handling from bytesToHex

* util: remove typecasting

* util: refactor account handling

* chore: simplify handling

* util: deprecate account constructor and update docs

* vm: fix event test

* client: fix client tests

* lint: remove console olgs

* client: fix more client tests

* lint: remove empty block

* client: remove unnecessary optional chaining

* client: remove it.only

Author: gabrocheleau

packages/client/src/net/peerpool.ts

@@ -213,7 +213,7 @@ export class PeerPool {
    * @emits {@link Event.POOL_PEER_ADDED}
    */
   add(peer?: Peer) {
-    if (peer && peer.id && !this.pool.get(peer.id)) {
+    if (peer?.id !== undefined && !this.pool.get(peer.id)) {
       this.pool.set(peer.id, peer)
       peer.pooled = true
       this.config.events.emit(Event.POOL_PEER_ADDED, peer)

packages/client/src/rpc/modules/admin.ts

@@ -95,9 +95,10 @@ export class Admin {
         name: peer.rlpxPeer?.['_hello']?.clientId ?? null,
         protocols: {
           eth: {
-            head: peer.eth?.updatedBestHeader
-              ? bytesToHex(peer.eth.updatedBestHeader?.hash())
-              : bytesToHex(peer.eth?.status.bestHash),
+            head:
+              peer.eth?.updatedBestHeader !== undefined
+                ? bytesToHex(peer.eth.updatedBestHeader.hash())
+                : bytesToHex(peer.eth?.status.bestHash ?? new Uint8Array()),
             difficulty: peer.eth?.status.td.toString(10),
             version: peer.eth?.['versions'].slice(-1)[0] ?? null,
           },

packages/client/src/sync/fetcher/accountfetcher.ts

@@ -337,9 +337,10 @@ export class AccountFetcher extends Fetcher<JobTask, AccountData[], AccountData>
     const { task, partialResult } = job
     const { first } = task
     // Snap protocol will automatically pad it with 32 bytes left, so we don't need to worry
-    const origin = partialResult
-      ? bigIntToBytes(bytesToBigInt(partialResult[partialResult.length - 1].hash) + BIGINT_1)
-      : bigIntToBytes(first)
+    const origin =
+      partialResult !== undefined
+        ? bigIntToBytes(bytesToBigInt(partialResult[partialResult.length - 1].hash) + BIGINT_1)
+        : bigIntToBytes(first)
     return setLengthLeft(origin, 32)
   }
 
@@ -470,9 +471,12 @@ export class AccountFetcher extends Fetcher<JobTask, AccountData[], AccountData>
     const fullResult = (job.partialResult ?? []).concat(result)
 
     // update highest known hash
-    const highestReceivedhash = result.at(-1)?.hash as Uint8Array
+    const highestReceivedhash = result.at(-1)?.hash
     if (this.highestKnownHash) {
-      if (compareBytes(highestReceivedhash, this.highestKnownHash) > 0) {
+      if (
+        highestReceivedhash !== undefined &&
+        compareBytes(highestReceivedhash, this.highestKnownHash) > 0
+      ) {
         this.highestKnownHash = highestReceivedhash
       }
     } else {

packages/client/test/sync/fetcher/accountfetcher.spec.ts

@@ -188,20 +188,19 @@ describe('[AccountFetcher]', async () => {
       address: 'random',
       latest: vi.fn(),
     }
-    const partialResult: any = [
-      [
-        {
-          hash: new Uint8Array(0),
-          body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
-        },
-        {
-          hash: new Uint8Array(0),
-          body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
-        },
-      ],
+    const partialResult = [
+      {
+        hash: new Uint8Array(0),
+        body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
+      },
+      {
+        hash: new Uint8Array(0),
+        body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
+      },
     ]
     const job = { peer, partialResult, task }
-    const result = (await fetcher.request(job as any)) as any
+    const result = await fetcher.request(job as any)
+    assert.isDefined(result)
     assert.equal(
       JSON.stringify(result[0]),
       JSON.stringify({ skipped: true }),
@@ -219,17 +218,15 @@ describe('[AccountFetcher]', async () => {
       first: BigInt(1),
       count: BigInt(3),
     })
-    const partialResult: any = [
-      [
-        {
-          hash: new Uint8Array(0),
-          body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
-        },
-        {
-          hash: new Uint8Array(0),
-          body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
-        },
-      ],
+    const partialResult = [
+      {
+        hash: new Uint8Array(0),
+        body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
+      },
+      {
+        hash: new Uint8Array(0),
+        body: [new Uint8Array(0), new Uint8Array(0), new Uint8Array(0), new Uint8Array(0)],
+      },
     ]
 
     const task = { count: 3, first: BigInt(1) }

packages/client/test/sync/fetcher/storagefetcher.spec.ts

@@ -255,10 +255,10 @@ describe('[StorageFetcher]', async () => {
       pool,
       root: utf8ToBytes(''),
     } as StorageFetcherOptions)
-    const partialResult: any = [
+    const partialResult = [
       [
-        [{ hash: utf8ToBytes(''), body: utf8ToBytes('') }],
-        [{ hash: utf8ToBytes(''), body: utf8ToBytes('') }],
+        { hash: utf8ToBytes(''), body: utf8ToBytes('') },
+        { hash: utf8ToBytes(''), body: utf8ToBytes('') },
       ],
     ]
 
@@ -276,7 +276,7 @@ describe('[StorageFetcher]', async () => {
         },
       ],
     }
-    const resData = RLP.decode(hexToBytes(_storageRangesRLP)) as unknown
+    const resData = RLP.decode(hexToBytes(_storageRangesRLP))
     const res = p.decode(
       p.messages.filter((message) => message.name === 'StorageRanges')[0],
       resData,

packages/util/README.md

@@ -29,7 +29,7 @@ import { hexToBytes, isValidChecksumAddress } from '@ethereumjs/util'
 
 ### Module: [account](src/account.ts)
 
-Class representing an `Account` and providing private/public key and address-related functionality (creation, validation, conversion).
+Class representing an `Account` and providing private/public key and address-related functionality (creation, validation, conversion). It is not recommended to use this constructor directly. Instead use the static factory methods to assist in creating an Account from varying data types.
 
 ```ts
 // ./examples/account.ts

packages/util/src/account.ts

@@ -19,7 +19,7 @@ import { EthereumJSErrorWithoutCode } from './errors.ts'
 import { assertIsBytes, assertIsHexString, assertIsString } from './helpers.ts'
 import { stripHexPrefix } from './internal.ts'
 
-import type { BigIntLike, BytesLike, PrefixedHexString } from './types.ts'
+import type { BigIntLike, BytesLike, NestedUint8Array, PrefixedHexString } from './types.ts'
 
 export interface AccountData {
   nonce?: BigIntLike
@@ -39,6 +39,35 @@ export interface PartialAccountData {
 
 export type AccountBodyBytes = [Uint8Array, Uint8Array, Uint8Array, Uint8Array]
 
+/**
+ * Handles the null indicator for RLP encoded accounts
+ * @returns {null} is the null indicator is 0
+ * @returns The unchanged value is the null indicator is 1
+ * @throws if the null indicator is > 1
+ * @throws if the length of values is < 2
+ * @param value The value to convert
+ * @returns The converted value
+ */
+function handleNullIndicator(values: NestedUint8Array | Uint8Array): Uint8Array | null {
+  // Needed if some values are not provided to the array (e.g. partial account RLP)
+  if (values[0] === undefined) {
+    return null
+  }
+
+  const nullIndicator = bytesToInt(values[0] as Uint8Array)
+
+  if (nullIndicator === 0) {
+    return null
+  }
+  if (nullIndicator > 1) {
+    throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${nullIndicator}`)
+  }
+  if (values.length < 2) {
+    throw EthereumJSErrorWithoutCode(`Invalid values length=${values.length}`)
+  }
+  return values[1] as Uint8Array
+}
+
 /**
  * Account class to load and maintain the  basic account objects.
  * Supports partial loading and access required for verkle with null
@@ -126,8 +155,10 @@ export class Account {
 
   /**
    * This constructor assigns and validates the values.
-   * Use the static factory methods to assist in creating an Account from varying data types.
-   * undefined get assigned with the defaults present, but null args are retained as is
+   * It is not recommended to use this constructor directly. Instead use the static
+   * factory methods to assist in creating an Account from varying data types.
+   * undefined get assigned with the defaults, but null args are retained as is
+   * @deprecated
    */
   constructor(
     nonce: bigint | null = BIGINT_0,
@@ -325,91 +356,26 @@ export function createAccountFromRLP(serialized: Uint8Array) {
 }
 
 export function createPartialAccountFromRLP(serialized: Uint8Array) {
-  const values = RLP.decode(serialized) as Uint8Array[][]
+  const values = RLP.decode(serialized)
 
   if (!Array.isArray(values)) {
     throw EthereumJSErrorWithoutCode('Invalid serialized account input. Must be array')
   }
 
-  let nonce = null
-  if (!Array.isArray(values[0])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial nonce encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[0][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${isNotNullIndicator} for nonce`)
-    }
-    if (isNotNullIndicator === 1) {
-      nonce = bytesToBigInt(values[0][1])
+  for (const value of values) {
+    // Ensure that each array item is an array
+    if (!Array.isArray(value)) {
+      throw EthereumJSErrorWithoutCode('Invalid partial encoding. Each item must be an array')
     }
   }
 
-  let balance = null
-  if (!Array.isArray(values[1])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial balance encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[1][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${isNotNullIndicator} for balance`)
-    }
-    if (isNotNullIndicator === 1) {
-      balance = bytesToBigInt(values[1][1])
-    }
-  }
+  const [nonceRaw, balanceRaw, storageRoot, codeHash, codeSizeRaw, versionRaw] =
+    values.map(handleNullIndicator)
 
-  let storageRoot = null
-  if (!Array.isArray(values[2])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial storageRoot encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[2][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(
-        `Invalid isNullIndicator=${isNotNullIndicator} for storageRoot`,
-      )
-    }
-    if (isNotNullIndicator === 1) {
-      storageRoot = values[2][1]
-    }
-  }
-
-  let codeHash = null
-  if (!Array.isArray(values[3])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial codeHash encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[3][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${isNotNullIndicator} for codeHash`)
-    }
-    if (isNotNullIndicator === 1) {
-      codeHash = values[3][1]
-    }
-  }
-
-  let codeSize = null
-  if (!Array.isArray(values[4])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial codeSize encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[4][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${isNotNullIndicator} for codeSize`)
-    }
-    if (isNotNullIndicator === 1) {
-      codeSize = bytesToInt(values[4][1])
-    }
-  }
-
-  let version = null
-  if (!Array.isArray(values[5])) {
-    throw EthereumJSErrorWithoutCode('Invalid partial version encoding. Must be array')
-  } else {
-    const isNotNullIndicator = bytesToInt(values[5][0])
-    if (isNotNullIndicator !== 0 && isNotNullIndicator !== 1) {
-      throw EthereumJSErrorWithoutCode(`Invalid isNullIndicator=${isNotNullIndicator} for version`)
-    }
-    if (isNotNullIndicator === 1) {
-      version = bytesToInt(values[5][1])
-    }
-  }
+  const nonce = nonceRaw === null ? null : bytesToBigInt(nonceRaw)
+  const balance = balanceRaw === null ? null : bytesToBigInt(balanceRaw)
+  const codeSize = codeSizeRaw === null ? null : bytesToInt(codeSizeRaw)
+  const version = versionRaw === null ? null : bytesToInt(versionRaw)
 
   return createPartialAccount({ balance, nonce, storageRoot, codeHash, codeSize, version })
 }

packages/util/src/bytes.ts

@@ -34,8 +34,13 @@ export const unprefixedHexToBytes = (hex: string): Uint8Array => {
   return nobleH2B(padToEven(hex))
 }
 
+/**
+ * Converts a {@link Uint8Array} to a {@link PrefixedHexString}
+ * @param {Uint8Array} bytes the bytes to convert
+ * @returns {PrefixedHexString} the hex string
+ * @dev Returns `0x` if provided an empty Uint8Array
+ */
 export const bytesToHex = (bytes: Uint8Array): PrefixedHexString => {
-  if (bytes === undefined || bytes.length === 0) return '0x'
   const unprefixedHex = bytesToUnprefixedHex(bytes)
   return `0x${unprefixedHex}`
 }
@@ -92,7 +97,7 @@ export const intToHex = (i: number): PrefixedHexString => {
   if (!Number.isSafeInteger(i) || i < 0) {
     throw EthereumJSErrorWithoutCode(`Received an invalid integer type: ${i}`)
   }
-  return ('0x' + i.toString(16)) as PrefixedHexString
+  return `0x${i.toString(16)}`
 }
 
 /**