Skip to content
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

Move SerializeForVersion into the super module so it can be implemented on common types #2181

Open
Tracked by #2220
sistemd opened this issue Aug 22, 2024 · 1 comment
Assignees

Comments

@sistemd
Copy link
Contributor

sistemd commented Aug 22, 2024

Currently, SerializeForVersion is defined in the dto::serialization module. For this reason, we need various odd wrappers to serialize common types, such as this TxnHash type, as well as many others added in #2178.

Instead, we can move SerializeForVersion into the dto module, allowing us to implement it directly on the common types and avoid the wrappers.

@sistemd sistemd self-assigned this Aug 22, 2024
@sistemd
Copy link
Contributor Author

sistemd commented Aug 22, 2024

Admittedly, another reason for these wrappers is to enable this blanket impl, but it's probably more elegant to make a Serde wrapper that delegates to serde::Deserialize and get rid of the blanket impl.

@sistemd sistemd mentioned this issue Sep 5, 2024
5 tasks
@t00ts t00ts self-assigned this Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants