-
Notifications
You must be signed in to change notification settings - Fork 738
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
Implement transparent new-types in ssz_derive
#3644
Conversation
FYI @realbigsean here's that SSZ impl we discussed last week IIRC, you need something like this (which is supported): /// The encoded/decoded bytes will be identical to a `Vec<u8>`
#[derive(PartialEq, Debug, Encode, Decode)]
#[ssz(struct_behaviour = "transparent")]
struct TransparentStruct {
serialized_thing:: Vec<u8>,
#[ssz(skip_serializing, skip_deserializing)]
local_thing: u8,
} |
ssz(transparent)
for new-type structsssz_derive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! The new docs are great. I took a stab at also supporting unnamed fields here:
realbigsean@697e190
I included the suggested changes below in that commit ^
consensus/ssz_derive/src/lib.rs
Outdated
); | ||
} | ||
|
||
let mut field_names = vec![]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these field_names
are used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Co-authored-by: realbigsean <seananderson33@GMAIL.com>
That was great, I included it in the PR! I made a change in 27a5a36 to support the scenario where there are skipped tuple fields before the non-skipped field. This is ready for review again |
Closed in favor of #3861 |
Issue Addressed
Related to #3625
Proposed Changes
Allows
transparent
encoding for new-type structs, just likeserde
does.Additional Info
I added some top-level docs to
ssz_derive
to make sure we're keeping track of all the attributes somewhere.I've also moved the tests for the derive macro from
eth2_ssz
toeth2_ssz_derive
. I did this primarily becausecargo-udeps
can't detect when a dev-dependency is only used in doc-tests and raises a false positive. Moving the tests causedeth2_ssz
to be used in tests and therefore udeps was happy again (there's an ignore method for udeps but it was too blunt IMO). It also kinda makes sense to have those tests in the derive crate rather than where they were.