Implement transparent new-types in ssz_derive#3644
Closed
paulhauner wants to merge 15 commits intosigp:unstablefrom
Closed
Implement transparent new-types in ssz_derive#3644paulhauner wants to merge 15 commits intosigp:unstablefrom
ssz_derive#3644paulhauner wants to merge 15 commits intosigp:unstablefrom
Conversation
paulhauner
added
ready-for-review
Member
Author
|
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,
} |
paulhauner
changed the title
ssz(transparent) for new-type structsssz_derive
realbigsean
reviewed
Oct 19, 2022
Member
There was a problem hiding this comment.
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![]; |
Member
There was a problem hiding this comment.
I don't think these field_names are used
realbigsean
added
waiting-on-author
Merged
Co-authored-by: realbigsean <[email protected]>
paulhauner
added
ready-for-review
Member
Author
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 |
paulhauner
added
ready-for-review
Member
Author
|
Closed in favor of #3861 |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
Related to #3625
Proposed Changes
Allows
transparentencoding for new-type structs, just likeserdedoes.Additional Info
I added some top-level docs to
ssz_deriveto make sure we're keeping track of all the attributes somewhere.I've also moved the tests for the derive macro from
eth2_ssztoeth2_ssz_derive. I did this primarily becausecargo-udepscan't detect when a dev-dependency is only used in doc-tests and raises a false positive. Moving the tests causedeth2_sszto 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.