FT donate review

[Tarnadas]

Hey,

plug has been asking for reviews for the donate-ft branch, so I looked into it. Here are my comments:

---

https://github.com/PotLock/core/blob/23b414af64945ce2c4c0d7d3bd12a4615c418742/contracts/donation/src/donations.rs#L45

The near_contract_standards crate has a trait FungibleTokenReceiver, that should be implemented here. The signature of the function is slightly wrong, because it should return PromiseOrValue<U128>

---

https://github.com/PotLock/core/compare/main...feat/donation-ft#diff-e8b8efb3ad30c822332ee8178ce282beed45dfb2789658fe1a34c39c0b613800R51

You can certainly do the manual deserialization, but why not use serde with Deserialize? It's way more convenient imho.

---

https://github.com/PotLock/core/compare/main...feat/donation-ft#diff-e8b8efb3ad30c822332ee8178ce282beed45dfb2789658fe1a34c39c0b613800R12

For the Donation which is stored in the smart contract, you could use u128 instead of U128 to save on storage deposit, but I certainly understand, that this would involve a migration. For anything that is sent as cross contract data or as result of view function you would still have to use U128, so you have two Donation types. It is common to just convert them via From trait.

---

https://github.com/PotLock/core/compare/main...feat/donation-ft#diff-e8b8efb3ad30c822332ee8178ce282beed45dfb2789658fe1a34c39c0b613800R258-R260

You could use an enum instead of three distinct booleans.

---

https://github.com/PotLock/core/compare/main...feat/donation-ft#diff-e8b8efb3ad30c822332ee8178ce282beed45dfb2789658fe1a34c39c0b613800R279

You can use ext_ft_core from near-contract-standards.

---

https://github.com/PotLock/core/compare/main...feat/donation-ft#diff-6ba773d8f53c9e505b27bac12660cfad87257703fa80c72218643e47bcf75f51R4

Please implement StorageManagement from near-contract-standards to be spec compliant with storage staking. https://nomicon.io/Standards/StorageManagement

[lachlanglen]

@Tarnadas thanks so much for taking the time to review! This is all really useful feedback. It's my first time integrating with FT so I appreciate it very much.

Re. your point You can certainly do the manual deserialization, but why not use serde with Deserialize? It's way more convenient imho. - I'm not sure where specifically you are referring to?

[Tarnadas]

I'm referring to the msg field. It can be deseralized via serde in a single line, if you define the struct. Example: https://github.com/Protocol-Pawns/chess-on-chain/blob/master/crates/chess-lib/src/ft_receiver.rs#L61

[lachlanglen]

Ah got it, thanks :)