-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add ExistenceRequirement to Currency trait
#4000
Changes from 6 commits
eebdb06
412ff66
e659ee6
caa9476
91e73bd
e95212d
9b49f3f
e14169d
ac0baa2
8b1559f
1b62003
596c8b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -464,6 +464,23 @@ decl_module! { | |||
| let dest = T::Lookup::lookup(dest)?; | ||||
| <Self as Currency<_>>::transfer(&source, &dest, value)?; | ||||
| } | ||||
|
|
||||
| /// Transfer some liquid free balance to another account, while checking that the transfer | ||||
| /// will not kill the account. | ||||
| /// | ||||
| /// 99% of the time you want `transfer` instead. | ||||
| #[weight = SimpleDispatchInfo::FixedNormal(1_000_000)] | ||||
| pub fn transfer_some( | ||||
|
gui1117 marked this conversation as resolved.
Outdated
|
||||
| origin, | ||||
| dest: <T::Lookup as StaticLookup>::Source, | ||||
| #[compact] value: T::Balance | ||||
| ) { | ||||
| let transactor = ensure_signed(origin)?; | ||||
| let dest = T::Lookup::lookup(dest)?; | ||||
|
|
||||
| Self::transfer_inner(&transactor, &dest, value, ExistenceRequirement::KeepAlive)?; | ||||
| } | ||||
|
|
||||
| } | ||||
| } | ||||
|
|
||||
|
|
@@ -852,42 +869,7 @@ where | |||
| } | ||||
|
|
||||
| fn transfer(transactor: &T::AccountId, dest: &T::AccountId, value: Self::Balance) -> Result { | ||||
| let from_balance = Self::free_balance(transactor); | ||||
| let to_balance = Self::free_balance(dest); | ||||
| let would_create = to_balance.is_zero(); | ||||
| let fee = if would_create { T::CreationFee::get() } else { T::TransferFee::get() }; | ||||
| let liability = match value.checked_add(&fee) { | ||||
| Some(l) => l, | ||||
| None => return Err("got overflow after adding a fee to value"), | ||||
| }; | ||||
|
|
||||
| let new_from_balance = match from_balance.checked_sub(&liability) { | ||||
| None => return Err("balance too low to send value"), | ||||
| Some(b) => b, | ||||
| }; | ||||
| if would_create && value < T::ExistentialDeposit::get() { | ||||
| return Err("value too low to create account"); | ||||
| } | ||||
| Self::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; | ||||
|
|
||||
| // NOTE: total stake being stored in the same type means that this could never overflow | ||||
| // but better to be safe than sorry. | ||||
| let new_to_balance = match to_balance.checked_add(&value) { | ||||
| Some(b) => b, | ||||
| None => return Err("destination balance too high to receive value"), | ||||
| }; | ||||
|
|
||||
| if transactor != dest { | ||||
| Self::set_free_balance(transactor, new_from_balance); | ||||
| if !<FreeBalance<T, I>>::exists(dest) { | ||||
| Self::new_account(dest, new_to_balance); | ||||
| } | ||||
| Self::set_free_balance(dest, new_to_balance); | ||||
| T::TransferPayment::on_unbalanced(NegativeImbalance::new(fee)); | ||||
| Self::deposit_event(RawEvent::Transfer(transactor.clone(), dest.clone(), value, fee)); | ||||
| } | ||||
|
|
||||
| Ok(()) | ||||
| Self::transfer_inner(transactor, dest, value, ExistenceRequirement::AllowDeath) | ||||
| } | ||||
|
|
||||
| fn withdraw( | ||||
|
|
@@ -1151,3 +1133,55 @@ where | |||
| Self::total_balance(who).is_zero() | ||||
| } | ||||
| } | ||||
|
|
||||
| impl<T: Trait<I>, I: Instance> Module<T, I> { | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not in substrate/srml/balances/src/lib.rs Line 789 in 91e73bd
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to hassle
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could change transfer signature of Currency trait to: fn transfer(transactor: &T::AccountId, dest: &T::AccountId, value: Self::Balance, liveness: ExistenceRequirement) -> Result {
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we should try to avoid breaking anything if we can. See @xlc's first comment in this PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Xlc comment is about not changing the Call. We can still have transfer and transfer_keep_alive in Call and have I feel generalizing the trait here doesn't harm
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Depends if anyone else has implemented anything on it - third parties might have done. That said, if it is to be changed, now is the time to do it. |
||||
| fn transfer_inner( | ||||
| transactor: &T::AccountId, | ||||
| dest: &T::AccountId, | ||||
| value: T::Balance, | ||||
| existential_requirement: ExistenceRequirement, | ||||
|
expenses marked this conversation as resolved.
Outdated
|
||||
| ) -> Result { | ||||
| let from_balance = Self::free_balance(transactor); | ||||
| let to_balance = Self::free_balance(dest); | ||||
| let would_create = to_balance.is_zero(); | ||||
| let fee = if would_create { T::CreationFee::get() } else { T::TransferFee::get() }; | ||||
| let liability = match value.checked_add(&fee) { | ||||
| Some(l) => l, | ||||
| None => return Err("got overflow after adding a fee to value"), | ||||
| }; | ||||
|
|
||||
| let new_from_balance = match from_balance.checked_sub(&liability) { | ||||
| None => return Err("balance too low to send value"), | ||||
| Some(b) => b, | ||||
| }; | ||||
| if would_create && value < T::ExistentialDeposit::get() { | ||||
| return Err("value too low to create account"); | ||||
| } | ||||
| Self::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; | ||||
|
|
||||
| // NOTE: total stake being stored in the same type means that this could never overflow | ||||
| // but better to be safe than sorry. | ||||
| let new_to_balance = match to_balance.checked_add(&value) { | ||||
| Some(b) => b, | ||||
| None => return Err("destination balance too high to receive value"), | ||||
| }; | ||||
|
|
||||
| if transactor != dest { | ||||
| if existential_requirement == ExistenceRequirement::KeepAlive { | ||||
| if new_from_balance < Self::minimum_balance() { | ||||
| return Err("payment would kill account"); | ||||
| } | ||||
| } | ||||
|
|
||||
| Self::set_free_balance(transactor, new_from_balance); | ||||
| if !<FreeBalance<T, I>>::exists(dest) { | ||||
| Self::new_account(dest, new_to_balance); | ||||
| } | ||||
| Self::set_free_balance(dest, new_to_balance); | ||||
| T::TransferPayment::on_unbalanced(NegativeImbalance::new(fee)); | ||||
| Self::deposit_event(RawEvent::Transfer(transactor.clone(), dest.clone(), value, fee)); | ||||
| } | ||||
|
|
||||
| Ok(()) | ||||
| } | ||||
| } | ||||
Uh oh!
There was an error while loading. Please reload this page.