Unexpected handling of undefined in bytesToHex

[gabrocheleau]

Our bytesToHex helper returns '0x' when its input is undefined. This seems odd and prone to errors. @jochem-brouwer and I have discussed this and we feel like this should be removed.

I have attempted removing it, but noticed that this led to many test errors in the monorepo that rely on undefined being handled in this way, so I thought it'd be better to leave this for a standalone PR.

export const bytesToHex = (bytes: Uint8Array): PrefixedHexString => {
  if (bytes === undefined || bytes.length === 0) return '0x'
  const unprefixedHex = bytesToUnprefixedHex(bytes)
  return `0x${unprefixedHex}`
}

(Src:

ethereumjs-monorepo/packages/util/src/bytes.ts

Line 38 in 7e3d163

if (bytes === undefined || bytes.length === 0) return '0x'

)

[jochem-brouwer]

Yes, sounds good 😄 👍

I've slightly edited the issue description, if you permalink the code line shows up in the GH issue :) To permalink see the attached image. (You can also do this from VSCode)

[holgerd77]

Fan of this 🙂

[gabrocheleau]

Began working on this, and stumbled into this in the toBytes helper, where it seems like we explicitly support undefined and null:

ethereumjs-monorepo/packages/util/src/bytes.ts

Line 234 in c859537

export const toBytes = (v: ToBytesInputTypes): Uint8Array => {

Doesn't necessarily mean we shouldn't remove the handling from bytesToHex, just pointing it out.

[jochem-brouwer]

I think it is worth a short discussion or seeing if there are any strong preferences. I'd initially say that I'd like those methods more strict. Especially in the case of calling with undefined while it should have been some object to convert sounds error-prone, also at production level. Or am I now seeing dangers which are not there?