Storage trait is missing from icrc3 value, which is necessary to store data to stable memory.
Can we add it ?
It would be ugly to wrap a type to add this trait
I imagine adding :
use candid::{ Decode, Encode };
use ic_stable_structures::storable::Bound;
use std::borrow::Cow;
impl Storable for ICRC3Value {
fn to_bytes(&self) -> Cow<[u8]> {
Cow::Owned(Encode!(self).unwrap())
}
fn from_bytes(bytes: Cow<[u8]>) -> Self {
Decode!(&bytes, Self).unwrap()
}
const BOUND: Bound = Bound::Unbounded;
}
Thanks for bringing this up! What you propose is certainly one way to implement it. However, candid is not very efficient for serialization, neither in terms of instructions used for serialization/deserialization, nor in terms of storage space for the serialized data. E.g., for the icrc_ledger_types::icrc1::account::Accountstruct, the impl Storable uses minicbor.
On the one hand, having this implementation detail in the type crate doesn’t seem like such a great fit, but on the other hand, there’s no other place for it. As you pointed out, the alternative would be to have a wrapper type. Maybe different implementation would benefit from different serialization approaches, but again, you could do that with a wrapper type even if some implementation exists for ICRC3Value, since the two aren’t mutually exclusive.
Let me bring this up with the rest of the team to see if they have other suggestions!
Thanks for your message Mathias.
Let’s keep in touch here.
i’m still thinking - like for Account type you shared - icrc3Value should have a default impl for storage, i think what you described with minicbor should fit for almost every usecases.
Thanks for your patience, Gautier! We discussed this within the team and had a few concerns:
We could implement Storable using minicbor, but then we’re tied to a particular format (and to a lesser degree, a particular implementation).
If we implement Storable using one approach, changing it later (if e.g., some more efficient way comes up) would be challenging. We could e.g., bump the major version and make it a breaking change, but user would potentially have to migrate all their data.
If we later switch to another serialization method with a breaking change, and more features were later added (e.g., types for another ICRC standard), then user would either have to live without those features, or migrate their data to make use of the new features.
What we settled on as somewhat of a compromise, is to propose implementing Storable using minicbor, but put it behind a feature flag. That way, if someone wanted to use it, they would have to consciously opt in. We could also later add other Storable implementations using e.g., candid, and user could pick and choose for clean-slate use cases. Migration from one to the other would be a different story, but we’d kick the can down the road regarding that at this point. Maybe we’re being overly cautions - I’ll still try to get a better understanding and see if there are other/better options, and I’d appreciate your feedback on this also!