-
Notifications
You must be signed in to change notification settings - Fork 973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Network message SSZ schema #871
Comments
Thanks @jannikluhn! These are points well taken. During the networking workshop tomorrow I intend on working through these with the implementation teams; it's becoming clear that SSZ isn't a good choice for network data. |
Personally I totally agree with this, but many people seem to like using the same format for everything. If we go with something else, I'd suggest RLP: It works quite well for Eth1, there are tons of implementations, it doesn't require a schema (at least not for the first decoding step), and messages can be extended. Sadly, I'm not going to be able to participate in the workshop, but I'm looking forward to hear about the results. |
So I just overheard the discussion from the workshop yesterday, and if the most important thing that is missing from SSZ right now is an Option type and a Null object, I think it may be possible to add this with 10ish lines of code. I think having the same serialization format in the core and networking would be nice. However I am wondering what the future implications with regards to upgrade (#861) are? |
We have an option type (if you view option as a special-cased list of 0 or 1 elements) - the above messages could be defined as:
|
this is a bit of a stretch - to do something meaningful with the data you need to interpret the values even in RLP - that's effectively what a schema does. it's just a matter of how many steps there are between raw bytes and useful data. |
@
I see, this seems like a very hacky way of achieving this. Have a look at #893 for my suggestion? |
my initial reaction would be that it doesn't carry its weight, considering the list trick. you can always decorate your code to treat the list as an option, but adding a special byte encoding and hashing instruction for it seems unnecessary. |
Opened PR #974 with my suggested solution. |
closing in favor of #1328 |
(Continuing discussion from the PR)
Two nitpicks regarding the network message serialization format:
Response
andErrorResponse
are identical, butRequest
differs a little. It would also be nice to give them the same name and field names, because that's what implementations will use when they first decode a message (e.g.Message
withmessage_id
,request_id
, andbody
).body
would be abytes
that carries the SSZ serialized request/response/error. This adds an additional 4-byte prefix, but I think that's unavoidable.There aren't optional fields in SSZ right now, so it should just be a blank string if there's no information.
@mslipper
The text was updated successfully, but these errors were encountered: