Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
Daanvdplas opened this issue Jul 5, 2024 · 4 comments · May be fixed by #147
Open

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

Daanvdplas opened this issue Jul 5, 2024 · 4 comments · May be fixed by #147
Assignees
@chungquantin
Labels
blocked question Further information is requested

Comments

@Daanvdplas
Copy link
Collaborator

Daanvdplas commented Jul 5, 2024 •
edited
Loading

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?
@Daanvdplas Daanvdplas mentioned this issue Jul 5, 2024
Closed
@Daanvdplas Daanvdplas changed the title feat (pop api): add native token to local fungibles use case feat: add native token to local fungibles use case Pop api Jul 17, 2024
@Daanvdplas Daanvdplas changed the title feat: add native token to local fungibles use case Pop api feat (pop api): add native token to local fungibles use case Jul 17, 2024
@Daanvdplas Daanvdplas mentioned this issue Jul 17, 2024
Closed
16 tasks
@chungquantin
Copy link
Collaborator

About the open questions:

The discussion related to the design of the returned error can be referenced from this: paritytech/polkadot-sdk#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 Daanvdplas mentioned this issue Jul 26, 2024
Merged
19 tasks
@Daanvdplas
Copy link
Collaborator Author

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

@chungquantin chungquantin self-assigned this Jul 30, 2024
@chungquantin chungquantin linked a pull request Jul 30, 2024 that will close this issue
Open
16 tasks
@peterwht
Copy link
Collaborator

peterwht commented Aug 2, 2024

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
Copy link
Collaborator

@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

@chungquantin chungquantin linked a pull request Aug 4, 2024 that will close this issue
feat: add native token support for fungibles api #147
Open
16 tasks
@Daanvdplas Daanvdplas added question Further information is requested high priority blocked labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chungquantin chungquantin

Labels
blocked question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add native token support for fungibles api r0gue-io/pop-node
3 participants
@peterwht @chungquantin @Daanvdplas