Borsh 09 feature #61
Borsh 09 feature #61
Conversation
We need to change sealable-trie to support multiple borsh crate versions. Since it’s borsh-serialising CryptoHash, this by extension means that lib needs to support all the same versions of borsh. To avoid the latter, rather than serialising and deserialising CryptoHash, deal with the underlying arrays of bytes. This makes sealable-trie code slightly more verbose but removes the need for lib to support multiple borsh versions. (We’ll see whether the support will need to be eventually added. So far this simplifies stuff a bit).
| @@ -42,7 +43,7 @@ strum = { version = "0.25.0", default-features = false, features = ["derive"] } | |||
|
|
|||
| lib = { path = "common/lib" } | |||
| memory = { path = "common/memory" } | |||
| sealable-trie = { path = "common/sealable-trie" } | |||
| sealable-trie = { path = "common/sealable-trie", features = ["borsh09"] } | |||
There was a problem hiding this comment.
| sealable-trie = { path = "common/sealable-trie", features = ["borsh09"] } | |
| sealable-trie = { path = "common/sealable-trie" } |
Enable this where you need it rather than globally.
There was a problem hiding this comment.
I actually enabled it for testing so that the rust-analyzer catches it. But will remove it
There was a problem hiding this comment.
We have a --all-features test in CI so that should cover it.
There was a problem hiding this comment.
in that case i need to remove the features on imports like u mentioned
|
|
||
| [features] | ||
| borsh = ["dep:borsh", "lib/borsh"] | ||
| borsh09 = ["dep:borsh09", "lib/borsh"] |
There was a problem hiding this comment.
| [features] | |
| borsh = ["dep:borsh", "lib/borsh"] | |
| borsh09 = ["dep:borsh09", "lib/borsh"] |
This isn’t needed. lib/borsh after all is not used any longer. You might need to merge upstream I guess.
| #[cfg(feature = "borsh")] | ||
| use borsh::maybestd::io; | ||
| #[cfg(feature = "borsh")] | ||
| use borsh::{BorshDeserialize, BorshSerialize}; | ||
| #[cfg(feature = "borsh09")] | ||
| use borsh09::maybestd::io; | ||
| #[cfg(feature = "borsh09")] | ||
| use borsh09::{BorshDeserialize, BorshSerialize}; |
There was a problem hiding this comment.
You probably need to remove all those uses and instead use full paths in places where the given types are used. Otherwise we get conflicts.
|
I thinnk we can close this for now. Lets open it when we face issue with borsh. Right now its fine imo. |
|
Sounds good. We can keep it as draft in the meanwhile. |
Adding the
borsh09as a feature insealable_triecrate so that projects using an older version of borsh can use the compatible version and not face version conflicts.