search

r0gue-io / pop-node

Pop Network makes it easy for smart contract developers to use the Power of Polkadot.
The Unlicense
21 stars 5 forks source link

chore (api): define the `FungiblesError` #99

Open Daanvdplas opened 2 months ago

Daanvdplas commented 2 months ago

Found cases were pallet assets doesn't return BalanceLow but TokenError::FundsUnavailable

chungquantin commented 1 month ago

@Daanvdplas Which cases? Can you expand it a bit so I can reproduce?

chungquantin commented 1 month ago

Asks for feedback from one developer in the ecosystem and here is his view:

"If we leave the raw status code like this, POP will need to have specific instructions on how to interpret the status code. Like with dispatchError when calling a transaction, it also returns raw bytes, and to decode the error, we need to look up the metadata.".

And his suggestion is instead of returning the raw byte PopApiStatusCode, we should just return the error directly. Or, we will need a thoroughly written documentation to guide the developers how to use theses Error.

One thing I realized that if we support NFT, we will have NonFungiblesError. Hence, if the contract of Dapps use both Fungible and NonFungible feature, there will be a mix of FungiblesError and NonFungiblesError in their code, which to me, is not very clean.

Daanvdplas commented 1 month ago

And his suggestion is instead of returning the raw byte PopApiStatusCode, we should just return the error directly. Or, we will need a thoroughly written documentation to guide the developers how to use theses Error.

So while the first is a good devex it increases the contract size (pop_api/integration-tests/contracts/fungibles 10.3K -> 11.4K). However, the developer can use it if it wants.

As for the latter, it would be encoding the u32 and decoding it in the pop_api::Error.

In general, our docs should explain thoroughly how developers can debug Error.

Hence, if the contract of Dapps use both Fungible and NonFungible feature, there will be a mix of FungiblesError and NonFungiblesError in their code, which to me, is not very clean.

Yes I'm also not so confident in the specific use case errors.