-
Notifications
You must be signed in to change notification settings - Fork 70
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
icolomina
wants to merge
2
commits into
stellar:main
Choose a base branch
from
icolomina:token_supply
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+137
−10
Open
Token supply #302
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,4 +28,5 @@ pub enum DataKey { | |
Nonce(Address), | ||
State(Address), | ||
Admin, | ||
Supply | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
use soroban_sdk::{Env}; | ||
use crate::storage_types::DataKey; | ||
|
||
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); | ||
} | ||
|
||
pub fn decrement_supply(e: &Env, amount: &i128) { | ||
let key = DataKey::Supply; | ||
let current_supply: i128 = e.storage().instance().get(&key).unwrap(); | ||
let new_supply = current_supply - amount; | ||
|
||
e.storage().instance().set(&key, &new_supply); | ||
} | ||
|
||
pub fn increment_supply(e: &Env, amount: &i128) { | ||
let key = DataKey::Supply; | ||
let current_supply: i128 = e.storage().instance().get(&key).unwrap(); | ||
let new_supply = current_supply + amount; | ||
|
||
e.storage().instance().set(&key, &new_supply); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.