feat (pop api): add native token to local fungibles use case

r0gue-io / pop-node

Pop Network makes it easy for smart contract developers to use the Power of Polkadot.

The Unlicense

18 stars 3 forks source link

feat (pop api): add native token to local fungibles use case #97

Open Daanvdplas opened 2 months ago

Daanvdplas commented 2 months ago

The fungibles use case allows a contract to interact with tokens on Pop Network. As of now this is only possible with tokens in pallet assets. The native token (pallet balances) should be included. First check whether this has any blockers with the current functions, also look at #104, #98 & #92. This should be implemented with #113 in mind as final implementation of the pop api.

Open Questions:

Having insufficient balance for transferring tokens results into InsufficientBalance. When there is no balance to pay for fees or create an asset, which requires a deposit, the balances pallet also return InsufficientBalance. Right now this is changed to NoBalance because there needs to be a separation for good developer experience. However, NoBalance will also be returned when there are not enough tokens for a requested transfer with the native token. Do we need to add an error NoFees and NoBalance?

chungquantin commented 1 month ago

About the open questions:

The discussion related to the design of the returned error can be referenced from this: https://github.com/paritytech/polkadot-sdk/issues/2240

From my perspective, adding NoFees would require extra implementation on the pop-api side to check the total_weight the method takes and the total_balance of the account (requires additional runtime dispatchs for on-chain data retrieval). Even though this does create a better user experience, I don't think trying to optimize it at this stage is recommended.

If there can be better implementation to cover the case without adding extra dispatches, then adding NoFees would bring the user experience to the next level.

Daanvdplas commented 1 month ago

Thanks for looking into this. Lets keep it as simple as possible for now.

peterwht commented 1 month ago

Maybe a little late for this comment. But why do we need the API to support native balance transfers from pallet-balances?

ink! supports this by default: https://github.com/use-ink/ink-examples/blob/main/contract-transfer/lib.rs#L35

chungquantin commented 1 month ago

@peterwht We want the users only need to care about using pop-api for interacting with the runtime and local fungible is more than just transfer(). But for now, seems like implementing the native balance in the same fungible pallet with asset is not very clean as there are too many mixed states. Would love to hear some thoughts from @Daanvdplas