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

Consider implment borshSerialize/de for (serde::)Serialize/de #8

Closed
ailisp opened this issue Jan 22, 2021 · 4 comments
Closed

Consider implment borshSerialize/de for (serde::)Serialize/de #8

ailisp opened this issue Jan 22, 2021 · 4 comments

Comments

@ailisp
Copy link
Member

ailisp commented Jan 22, 2021

This gives a few benefits:

  • issue like Failed to derive borsh ser/de in a recursive data structure #7 will work because serde gives handle recursive types support for free
  • serde as defacto standard, those 3rd party container libraries, like IndexMap, usually have serde::deserialize implemented for it. And we cannot impl borshSerialize for all 3rd party containers. If user would like to impl borshSerialize for IndexMap, they cannot do because both the trait and the type is not in their crate. They have to wrap it to struct MyIndexMap(IndexMap) and impl borshSerialize for MyIndexMap
  • issue like borsh#112 won't exist

Cons is with extra layer of Serde::Serialize trait, this may slowdown borsh, to be research

@frol
Copy link
Collaborator

frol commented Jan 22, 2021

Great proposal! I think it should be possible to implement it under a feature-flag, and also prefer native Borsh implementations before falling back to serde serializations.

@praveenperera
Copy link

This would be a great feature.

I was looking to replace Bincode with Borsh, but without being able to use serde_rename and having derives for UUID and other third party structs the friction is too high.

@matklad
Copy link
Contributor

matklad commented Nov 10, 2021

Hm, I think this is not the right thing to do. My understanding is that one (primary) design goal of borsh is canonicity/consistency:

Consistency means there is a bijective mapping between objects and their binary representations. There is no two binary representations that deserialize into the same object. This is extremely useful for applications that use binary representation to compute hash;

https://borsh.io

Serde is not canonical -- it can deserialize the same object from different representations (eg, order of key/values in a hashmap). We can't layer a canonical repr on top of a non-canonical one.

If the particular use-case does not require canonicity, then bincode should be used instead of borsh.

@frol
Copy link
Collaborator

frol commented Nov 23, 2021

I did not know that feature of serde. I am closing this issue since there are not action items

@frol frol closed this as completed Nov 23, 2021
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

4 participants