near-daos / sputnik-dao-contract

Smart contracts for https://app.astrodao.com
https://astrodao.com/
MIT License
107 stars 79 forks source link

Try to remove any dependency on `std` library from sputnik #90

Open ctindogaru opened 2 years ago

ctindogaru commented 2 years ago

If possible, remove any use of std inside sputnik.

Alternatives: NEAR's own libraries or implementing a different logic in all the places that use std.

nninkovicSQA commented 2 years ago

Do you plan to replace HashMap with LookupMap/UnorderedMap? If so, do you have any recommendations on how? Same question about std::cmp::min

ctindogaru commented 2 years ago

Yes, this is the plan, depending on the situations. If you we are very intensive with the reads and no need to iterate through the HashMap, then we definitely go with LookupMap. If you need any kind of iterations/getting all the elements -> UnorderedMap.

std::cmp::min not sure about this one, but near must have something similar. If not, we can implement our own method, sounds pretty trivial.

nninkovicSQA commented 2 years ago

So that means we will have to implement custom Serialization in order to replace all the HashMaps,

ctindogaru commented 2 years ago

I hear you. I assume you're talking about serde serialization, since borsh is already supported.

I would avoid to implement our own serialization since that's pretty overkill and it's something that should be available by default in the near sdk.

I would be curious to hear from near why they don't support serde on UnorderedMap. On LookupMap it's pretty obvious since there is no way for getting all the key/value pairs, but on UnorderedMap should be supported.

A simple

[derive(Serialize, Deserialize)] on their UnorderedMap definition should solve this and then we can replace HashMap with UnorderedMap

herolind-sqa commented 2 years ago

Hi @ctindogaru,

Sometimes you may see compile-time errors that tell you:

the trait `serde::ser::Serialize` is not implemented for `...`
even though the struct or enum clearly has #[derive(Serialize)] on it.

This almost always means that you are using libraries that depend on incompatible versions of Serde. You may be depending on serde 1.0 in your Cargo.toml but using some other library that depends on serde 0.9. So the Serialize trait from serde 1.0 may be implemented, but the library expects an implementation of the Serialize trait from serde 0.9. From the Rust compiler's perspective these are totally different traits.

The fix is to upgrade or downgrade libraries as appropriate until the Serde versions match. The cargo tree -d command is helpful for finding all the places that duplicate dependencies are being pulled in.
ctindogaru commented 2 years ago

We get the serde version indirectly from the near-sdk, so we shouldn't run into the issue where the versions don't match.