ethereum / beacon_chain

MIT License
209 stars 65 forks source link

The necessity to infer types in ssz #113

Closed Mikerah closed 5 years ago

Mikerah commented 5 years ago

Hi, While implementing ssz in Js, this part of the serialize function stumped me:

elif isinstance(typ, type):
        sub = b''.join(
            [serialize(getattr(val, k), typ.fields[k]) for k in sorted(typ.fields.keys())]
        )?
return len(sub).to_bytes(4, 'big') + sub

I know it's for inferring types but is it a necessity for the ssz specification? Otherwise, I am thinking of leaving it out of the Js implementation of ssz completely.

Any thoughts?

djrtwo commented 5 years ago

I was just looking at this today. What is important here is that it is not one of the base supported types (int, byte32, address, etc) but is instead an object with a "fields" attribute. This code below is saying "okay, no type passed in, but does it have 'fields' defined? if so, we can handle that. https://github.com/ethereum/beacon_chain/blob/19fa101ee0d8e33bc060621fe2b81f5ff3a48ace/ssz/ssz.py#L5-L6

Then the code you linked to is where it handles that. It iterates through the fields and concatenates the serialization of each.

In JS, you would need to have an object that implements the standard ssz interface. This serialize method would need to make note of that and do the recursive serialization on each field.

Mikerah commented 5 years ago

So, why is the code that I pointed to in a different if statement? Wouldn't it make sense to put this portion in the part that you linked?

djrtwo commented 5 years ago

The first if is to handle the optional param.

Later in the code (the last elif), we handle if the val is a serializable object.

Your code could function without the first conditional if you removed the optionality from the second param.

The last elif in JS would be for the serialization of an ssz formatted object.

Mikerah commented 5 years ago

That clarifies things a lot. Thank you