-
Notifications
You must be signed in to change notification settings - Fork 58
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
dusk-core: add serde support #3046
base: master
Are you sure you want to change the base?
Conversation
8b9d6ae
to
458f951
Compare
458f951
to
9114601
Compare
e329820
to
645e9bc
Compare
645e9bc
to
fc895f6
Compare
0b0c5c4
to
ca73df8
Compare
ca73df8
to
4951ee5
Compare
4951ee5
to
21e5150
Compare
be97d5f
to
dec5bcd
Compare
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.
Is there a reason why the traits are re-implemented for each type?
You might also be able to simply derive types that are composed of other types that implement Serialize
and Deserialize
already:
For example for StakeFundOwner
adding this on top of its declaration will be enough:
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub enum StakeFundOwner {
...
}
This is probably true for many other types as well.
The reason is to keep all implementations in one place, for ease of auditing. I think @Neotamandua mentioned something about this at some point before? Edit: the re-implementations aren't needed. |
ff81362
to
21089d6
Compare
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 good from my side.
But please wait for a go from @ZER0 and @Neotamandua as well as they are the ones that can judge the serialization approach.
21089d6
to
2589132
Compare
This is to add the feature described in #2773.
I've only added serde support for a single event struct -MoonlightTransactionEvent
.I need some thoughts on whether what I have now is okay.I've added serde support for the event structs.
I also added
PartialEq
derive impls forSlashEvent
andStakeEvent
so that they could be compared in tests.Changed
dusk-jubjub
andbls12_381-bls
versions to the ones that now have serde support.