Add union type and null to SSZ #893

dankrad · 2019-04-10T14:13:44Z

This is a suggestion to add an option type and a null type to the SSZ spec. While we will not use it in the spec at the moment, this could be a very simple addition that would make SSZ much more powerful.

In a discussion with @mslipper it transpired that this was one of the main stumbling blocks for making more extensive use of SSZ in the networking. I wonder if this would solve the main problems?

specs/simple-serialize.md

Co-Authored-By: dankrad <dankrad@ethereum.org>

snario · 2019-04-12T06:27:24Z

Hi there! I’m just commenting to make a point that in state channels we need to have a single data type to represent the state in the channel and we need it to be of dynamic length. Also we need this to be in solidity early as possible

mslipper · 2019-04-15T06:22:49Z

This would solve some of the issues in the networking spec, but not all of them. While we can now represent nullable values (which would clean up some of the data structures in the networking spec), it doesn't solve the problem of needing to know an upfront schema to serialize/deserialize SSZ-encoded data. For things like the RPC wrapper, we need a serialization scheme that can model something similar to generics in object-oriented languages. Right now, the easiest way to do this is to not use any serialization scheme for the RPC wrapper and instead just prepend bytes to whatever SSZ-encoded object is being sent.

Since the introduction of a null type helps us clean up data structures elsewhere, I'd still like to merge this.

dankrad · 2019-04-16T00:19:15Z

For things like the RPC wrapper, we need a serialization scheme that can model something similar to generics in object-oriented languages.

Would it not be possible to do this using the Union type? If the call can take typs A, B, C or D, you can do this bye defining a union type of Union(A, B, C, D) and defining your RPC call on that. The type index also makes it easy to add more types to the union later without breaking compatibility with the old type.

mslipper · 2019-04-16T08:31:07Z

Would it not be possible to do this using the Union type? If the call can take typs A, B, C or D, you can do this bye defining a union type of Union(A, B, C, D) and defining your RPC call on that. The type index also makes it easy to add more types to the union later without breaking compatibility with the old type.

This is possible, but unwieldy - there are something like 7 different types that the RPC wrapper can include in the request/response body field. Defining a union type on all of them seems error-prone and difficult to maintain.

dankrad · 2019-04-17T05:20:50Z

@JustinDrake @vbuterin I am not sure if you think this is a strong enough endorsement to merge this change. I would be in favour of it as it is fairly simple and will not change anything in the consensus layer, but I don't feel very strongly either way.

djrtwo · 2019-04-17T19:36:52Z

Should we discuss this on tomorrow's call?

dankrad · 2019-04-17T20:29:45Z

Should we discuss this on tomorrow's call?

Sounds like a good idea

Nashatyrev · 2019-04-18T20:07:27Z

Don't we need to adjust 'fixed/variable-size' definition?
I believe we should treat the union as a variable-size type
Should we treat the union of fixed-size types of the same size as fixed-type?

dankrad · 2019-04-19T09:24:37Z

Don't we need to adjust 'fixed/variable-size' definition?

I think it is already correct?

I believe we should treat the union as a variable-size type

Yes.

Should we treat the union of fixed-size types of the same size as fixed-type?

Hmm, I can see that, but not sure if it would be hugely beneficial to add this special case. I can see it would simplify things if we say that a union of several fixed-size object has the (fixed) size of the max size of those objects.

Nashatyrev · 2019-04-19T11:58:05Z

I believe we should treat the union as a variable-size type

Yes.

We recursively define "variable-size" types to be lists and all types that contains a variable-size type. All other types are said to be "fixed-size"
According to this statement a union of fixed-size types is itself fixed-size. Am I missing something?

dankrad · 2019-04-20T10:47:10Z

According to this statement a union of fixed-size types is itself fixed-size. Am I missing something?

Ah, yes, I overlooked that sentence. Fixed now.

JustinDrake · 2019-04-20T10:58:29Z

Can we alias null to {} to avoid creating a new type?

dankrad · 2019-04-20T11:48:10Z

Can we alias null to {} to avoid creating a new type?

You mean the empty container? That sounds like an interesting idea.

JustinDrake · 2019-04-20T12:02:33Z

You mean the empty container?

Yes

# Conflicts: # specs/simple-serialize.md

dankrad · 2019-04-27T20:01:53Z

Can we alias null to {} to avoid creating a new type?

Done.

specs/simple-serialize.md

hwwhww · 2019-05-01T07:41:50Z

specs/simple-serialize.md

@@ -97,6 +113,16 @@ fixed_parts = [part if part != None else variable_offsets[i] for i, part in enum
 return b"".join(fixed_parts + variable_parts)
 ```

+If `value` is an union type:
+
+Define value as an object that has properties `value.value` with the contained value, and `value.type_index` which indexes the type.


IMO maybe too many "value"s in these sentences and code. Perhaps using value.contained_value would be more readable?

Shall we use object instead? (That's what these things are after all, objects.)

You mean value.object or object.value @JustinDrake ?

I was thinking object.value

I'm happy to go with that

Oh but 'object' is a type in python :(

Co-Authored-By: dankrad <dankrad@ethereum.org>

specs/simple-serialize.md

# Conflicts: # specs/simple-serialize.md

Add option type and null

cd85546

dankrad mentioned this pull request Apr 10, 2019

Network message SSZ schema #871

Closed

dankrad changed the title ~~Add option type and null~~ Add option type and null to SSZ Apr 10, 2019

jannikluhn reviewed Apr 11, 2019

View reviewed changes

specs/simple-serialize.md Show resolved Hide resolved

specs/simple-serialize.md Outdated Show resolved Hide resolved

jannikluhn and others added 2 commits April 11, 2019 16:00

Update specs/simple-serialize.md

283ba8f

Co-Authored-By: dankrad <dankrad@ethereum.org>

Rename "option" -> "union"; "null" only in unions

2017ce9

dankrad changed the title ~~Add option type and null to SSZ~~ Add union type and null to SSZ Apr 11, 2019

JustinDrake added the scope:SSZ Simple Serialize label Apr 14, 2019

dankrad requested a review from JustinDrake April 17, 2019 04:58

dankrad mentioned this pull request Apr 18, 2019

Eth2.0 Implementers Call 16 Agenda ethereum/eth2.0-pm#37

Closed

Update simple-serialize.md

63bdf95

Update simple-serialize.md

a481a4e

dankrad added 2 commits April 27, 2019 20:53

Merge remote-tracking branch 'origin/dev' into dankrad-patch-3

f9130a8

# Conflicts: # specs/simple-serialize.md

Define null as alias of {}

101449e

djrtwo mentioned this pull request May 1, 2019

Use SSZ union-types in RPC request/response #974

Merged

hwwhww reviewed May 1, 2019

View reviewed changes

hwwhww and others added 4 commits May 1, 2019 12:39

Update specs/simple-serialize.md

4a48330

Co-Authored-By: dankrad <dankrad@ethereum.org>

Update specs/simple-serialize.md

d044702

Co-Authored-By: dankrad <dankrad@ethereum.org>

Update specs/simple-serialize.md

cc22432

Co-Authored-By: dankrad <dankrad@ethereum.org>

Update simple-serialize.md

a33ee00

JustinDrake reviewed May 1, 2019

View reviewed changes

specs/simple-serialize.md Show resolved Hide resolved

JustinDrake and others added 3 commits May 1, 2019 13:55

Update simple-serialize.md

563df14

Union default values

b1930d2

Merge branch 'dev' into dankrad-patch-3

b3501fb

# Conflicts: # specs/simple-serialize.md

dankrad requested a review from JustinDrake May 7, 2019 17:33

JustinDrake approved these changes May 7, 2019

View reviewed changes

dankrad merged commit b970962 into dev May 7, 2019

hwwhww deleted the dankrad-patch-3 branch May 9, 2019 07:20

dankrad mentioned this pull request May 21, 2019

Disallow empty containers or vectors #854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add union type and null to SSZ #893

Add union type and null to SSZ #893

dankrad commented Apr 10, 2019

snario commented Apr 12, 2019

mslipper commented Apr 15, 2019

dankrad commented Apr 16, 2019

mslipper commented Apr 16, 2019

dankrad commented Apr 17, 2019

djrtwo commented Apr 17, 2019

dankrad commented Apr 17, 2019

Nashatyrev commented Apr 18, 2019

dankrad commented Apr 19, 2019

Nashatyrev commented Apr 19, 2019

dankrad commented Apr 20, 2019

JustinDrake commented Apr 20, 2019

dankrad commented Apr 20, 2019

JustinDrake commented Apr 20, 2019

dankrad commented Apr 27, 2019

hwwhww May 1, 2019

JustinDrake May 1, 2019

dankrad May 1, 2019

JustinDrake May 1, 2019

dankrad May 1, 2019

dankrad May 1, 2019

Add union type and null to SSZ #893

Add union type and null to SSZ #893

Conversation

dankrad commented Apr 10, 2019

snario commented Apr 12, 2019

mslipper commented Apr 15, 2019

dankrad commented Apr 16, 2019

mslipper commented Apr 16, 2019

dankrad commented Apr 17, 2019

djrtwo commented Apr 17, 2019

dankrad commented Apr 17, 2019

Nashatyrev commented Apr 18, 2019

dankrad commented Apr 19, 2019

Nashatyrev commented Apr 19, 2019

dankrad commented Apr 20, 2019

JustinDrake commented Apr 20, 2019

dankrad commented Apr 20, 2019

JustinDrake commented Apr 20, 2019

dankrad commented Apr 27, 2019

hwwhww May 1, 2019

Choose a reason for hiding this comment

JustinDrake May 1, 2019

Choose a reason for hiding this comment

dankrad May 1, 2019

Choose a reason for hiding this comment

JustinDrake May 1, 2019

Choose a reason for hiding this comment

dankrad May 1, 2019

Choose a reason for hiding this comment

dankrad May 1, 2019

Choose a reason for hiding this comment