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.

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

Token supply #302

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Token supply #302

wants to merge 2 commits into from

Conversation

icolomina
Copy link

This adds supply control to the soroban token example:

  • Can initialize the token with a supply amount (not required).
  • If supply provided, cannot mint if amount to mint is greater than supply. If minted, supply is decremented with minted amount.
  • burn and burn_from increments supply with the amount burned.

Comment on lines +4 to +17
pub fn has_supply(e: &Env) -> bool {
let key = DataKey::Supply;
e.storage().instance().has(&key)
}

pub fn read_supply(e: &Env) -> i128 {
let key = DataKey::Supply;
e.storage().instance().get(&key).unwrap()
}

pub fn write_supply(e: &Env, amount: &i128) {
let key = DataKey::Supply;
e.storage().instance().set(&key, amount);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supply might be necessary for some tokens, but is probably not ideal to include on most tokens on Soroban.

Soroban is designed to execute invocations concurrently in the future. This is why transactions have a footprint as the environment uses the footprint to figure out which contracts needed to execute serially and which can execute concurrently. Invocations need to occur serially if they write to the same data. An example of this is when account A makes a payment to account B, and account C also makes a payment to account B in the same ledger. Both payments will be processed serially because they both write to account B's balance data entry. If an operation requires writing to a single/global/instance data entry, every invocation using that operation will be serialised and won't be able to executed concurrently. The concurrent part of Soroban isn't implemented yet, but we can imagine that when it is transactions that must be executed serially may be more expensive as there will be less capacity to execute a lot of serial transactions as there are to execute parallel.

For this reason I'm not sure we should add the supply to the token example. We've seen plenty of token implementations start by forking the example, and if they forked the example with supply implemented then every token would carry this serialisation limitation without necessarily needing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add another example that is a token example with supply? But with disclaimers as to the cost / impact.

Copy link
Author

@icolomina icolomina Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. I was thinking in those situations where supply is required but i did not think about that cost / impact you've commented. I can send another pull request to the soroban examples with a new folder holding the token with supply example. This way it wouldn't affect to the existing one.
What do you think ?
Thanks in advance

Copy link
Member

@leighmcculloch leighmcculloch Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomerweller @kalepail Is supply a common feature that you see folks using in other ecosystems, or needing that we should have an example for? We're definitely avoiding it for tokens that need to scale because a global value like supply will prevent ops that write to the supply value to be parallelised, but I recognise that shouldn't constrain everything.

@icolomina icolomina mentioned this pull request Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants