New Byte Type Aliases with Byte Length Indicators

[holgerd77]

This is on my plate for a very long time, PR adds simple byte aliases for both Uint8Array and PrefixedHexString with byte length indication.

As discussed already quite some time back ago, this implementation has none of the eventual backlashes as stricter solutions (like performance drawbacks on actual length checks everywhere in the code base) and should at the same time already bring a lot of the benefits, mostly being a lot better documented function signatures and length hints. VSC mouseover of the one exemplaric type in Block e.g. now look like this:

For the PrefixedHex* thing I tested (and "stared" for quite some time 😂) at the different flavors of a possible naming scheme:

• PrefixedHexString20
• PrefixedHexStr20
• PrefixedHex20

I came to the conclusion that the last version should be sufficiently expressive and is at the same time the easiest to grasp, with the need to only digest 3 word parts instead of 4 (which is a lot), and after 3-4 rounds of usage latest it should be very intrinsic that this is about a string (if not clear from the beginning). So this version would be my personal favorite.

I would give this open for a first look and feedback and then continue a bit with it.

We do not need to get to completion here and merge this in a selectively applied state. Then we can just replace types over time, this is otherwise too tedious to accomplish in one go.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.87%. Comparing base (4be68d2) to head (3ddb5e0). Report is 2 commits behind head on master.

Additional details and impacted files

Flag	Coverage Δ
block	82.24% <ø> (ø)
blockchain	90.92% <ø> (ø)
client	?
common	94.02% <ø> (ø)
devp2p	?
evm	?
genesis	?
statemanager	?
trie	59.39% <ø> (-0.21%)	:arrow_down:
tx	?
util	81.32% <ø> (+<0.01%)	:arrow_up:
vm	?
wallet	?

Flags with carried forward coverage won't be shown. Click here to find out more.

[acolytec3]

This is lovely. We should definitely do this across the monorepo.

[gabrocheleau]

I like this!

One thing that came up while searching the monorepo, is that we have some exported Bytes32 and other similar types in the client, but in that case they served as aliases for string. It could be helpful to remove that ambiguity (in my view Bytes should be Uint8Array, while PrefixedHex should be strings). Link: https://github.com/ethereumjs/ethereumjs-monorepo/blob/new-byte-type-aliases/packages/client/src/rpc/modules/engine/types.ts#L40

[holgerd77]

One thing that came up while searching the monorepo, is that we have some exported Bytes32 and other similar types in the client, but in that case they served as aliases for string. It could be helpful to remove that ambiguity (in my view Bytes should be Uint8Array, while PrefixedHex should be strings). Link: https://github.com/ethereumjs/ethereumjs-monorepo/blob/new-byte-type-aliases/packages/client/src/rpc/modules/engine/types.ts#L40

Yes, that makes definitely sense to align in client, would also agree with the Bytes == Uint8Array stuff.

The whole PR is currently (and maybe permanently block). Context is that the type aliases do not propagate through in the editor (Visual Studio Code), so hovering over the code then shows Uint8Array again instead of e.g. Bytes32 (see chat exchange with Jochem somewhere). This makes the whole thing too brittle and half-useless and we should see if we find something both more stable and robust and still non-breaking (might not be a solution here).

Jochem actually tried the same thing as here already some time ago (didn't know that before) and stumbled upon the very same thing (and therefore drew the same conclusion here).

We'll see. Would be really really nice to have something like this.

Maybe also worth to have a look at other low-level libraries for some inspiration.