Use custom types in data structures and constants

[Nashatyrev]

Original issue: #667

The goal is to improve Spec readability by using alias custom types in data structures and as constant units instead of generic types.

What was done

• Add Hash and Bitfield custom types. The Hash type is treated as a result of any 256-bit cryptographic hash function or its derivative preserving hash properties (like xoring two hashes)
• Replace generic SSZ types in data structures with custom types where appropriate
• Add/update constant units according to custom types

What else can be done

• Add more custom types (like Time, ForkVersion, Bitfield64) which are now referred at 1-2 places in the Spec
• As per @hwwhww suggestion in Use custom types in data structures #667 cover all elements with custom types
• In some way describe array index types (like 'validator_balances': [ValidatorIndex => Gwei])

[hwwhww]

@JustinDrake
Should it be Bytes32?

[Nashatyrev]

Already returned it back to Bytes32 in this commit a693737

[hwwhww]

But there are two withdrawal_credentialss: one is in DepositInput and another one is in Validator.

[Nashatyrev]

Oh, my bad.
Sure this one should also be Bytes32
Thanks for spotting this!

[JustinDrake]

I'm tempted to nuke Bytes32 (which is semantically redundant with bytes32) and use Hash everywhere. After all, withdrawal_crendentials fits with the definition: "result of hashing and its derivatives"

[Nashatyrev]

I'm fine with it. Committed

[vbuterin]

After all, withdrawal_crendentials fits with the definition: "result of hashing and its derivatives"

Technically the first byte is a version number, but hashes have had built-in version numbers before (see ipfs multi hash) so that's not too terrible...

[djrtwo]

I think this is a net positive. Is there any hangup on merge at this point?

[hwwhww]

I’m inclined to have custom types cover all elements. Compromise: define a general Uint64?

[Nashatyrev]

IMHO, defining aliases for types which are used just in a couple of places in the spec wouldn't make the spec clearer.
As well I see no big reason to make UInt64 => uint64 alias.
We probably need someone else opinion here. @djrtwo ?

[hwwhww]

Hmm, I think this PR would break CI tests...
Basically, it requires (i) defining a custom type in Python and also (ii) converting Python types to SSZ types when defining SSZ classes.

@Nashatyrev could you rebase it and then let's see how to fix it in function_puller.py?

[Nashatyrev]

@hwwhww rebased. Let me know if I can help you with CI tests

[protolambda]

1. Is this still up to date? (I don't think so)
2. Do we still plan to merge this?
3. FYI: this breaks the typing syntax in many of the SSZ utilities in the pyspec. I.e. the default zero values, random instantiation of ssz, and minimal_ssz.py itself. These basic types are declared with strings, not types: they instruct what the typing looks like, they are not the type itself. I recommend to add an alias functionality, where we just lookup the "raw" type for any given type used in the SSZ container type declarations, and only then serialize/hash it etc.

[JustinDrake]

Do we still plan to merge this?

I'm very pro adding aliases for readability. Do you want to go ahead @protolambda and fix whatever needs fixing? We can then merge.

[protolambda]

I think we addressed most of the typing ideas with #1077. We can go further with it and use NewType here and there to describe some things in addition to typing them (e.g. Slot), but I think it's clean enough right now with the current data-types. So I'm closing this PR. Anyone who is interested is welcome to open a new PR for typing improvements, if anything can be improved on the current dev branch (this PR is quite out of date now).

👏 for bringing this up so early, and apologies from us all that typing didn't make it into the spec so quick, I hope the new more complete typing support makes up for it.