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

Implement fungible and fungibles for tokens #517

Merged
merged 9 commits into from
Jun 14, 2021
Merged

Conversation

zjb0807
Copy link
Contributor

@zjb0807 zjb0807 commented Jun 10, 2021

#417

currencies module is updating.

@zjb0807 zjb0807 requested a review from xlc June 10, 2021 03:04
@xlc xlc requested a review from shaunxw June 10, 2021 03:07
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
source: &T::AccountId,
dest: &T::AccountId,
amount: T::Balance,
_keep_alive: bool,
Copy link
Member

Choose a reason for hiding this comment

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

we shouldn't ignore keep_alive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fn transfer(
currency_id: Self::CurrencyId,
from: &T::AccountId,
to: &T::AccountId,
amount: Self::Balance,
) -> DispatchResult {
if amount.is_zero() || from == to {
return Ok(());
}
Self::ensure_can_withdraw(currency_id, from, amount)?;
let from_balance = Self::free_balance(currency_id, from);
let to_balance = Self::free_balance(currency_id, to)
.checked_add(&amount)
.ok_or(ArithmeticError::Overflow)?;
// Cannot underflow because ensure_can_withdraw check
Self::set_free_balance(currency_id, from, from_balance - amount);
Self::set_free_balance(currency_id, to, to_balance);
Ok(())

Tokens transfer not handle the ExistenceRequirement

Copy link
Member

Choose a reason for hiding this comment

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

we need a version that handles it, and transfer can call it with keep_alive: false

tokens/src/lib.rs Outdated Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
tokens/src/lib.rs Show resolved Hide resolved
@zjb0807 zjb0807 requested review from xlc and shaunxw June 11, 2021 06:46
from: &AccountId,
to: &AccountId,
amount: Self::Balance,
existence_requirement: ExistenceRequirement,
Copy link
Member

Choose a reason for hiding this comment

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

this will break too many existing code.
I was thinking only update the transfer implementation of orml-tokens. Add a new helper method to take existence_requirement.

@@ -49,6 +49,7 @@ pub trait MultiCurrency<AccountId> {
from: &AccountId,
to: &AccountId,
amount: Self::Balance,
existence_requirement: ExistenceRequirement,
Copy link
Member

Choose a reason for hiding this comment

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

remove this as well

@zjb0807 zjb0807 requested a review from xlc June 14, 2021 03:00
@zjb0807 zjb0807 merged commit ffa1a26 into master Jun 14, 2021
@zjb0807 zjb0807 deleted the implement-fungible branch June 14, 2021 03:28
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.

3 participants