-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Named reserve #7778
Named reserve #7778
Changes from 3 commits
d0b265b
d13c224
597b342
47643ad
91e0b59
22e9e69
281bd90
52cb8b1
59e9b01
747c318
cd63175
e504b60
b029920
3f9a057
734363b
995dac0
0a04dce
e1644d6
1ce677e
ddfc2b0
66a2517
0301c7f
dcaf864
772258c
b107903
29f821e
b6c55e1
77ccea7
5942996
4f9154a
baf4a0f
b19dc12
72fb670
d852a81
73a0d6f
2ee5092
7614ecc
e6a1d9d
643110f
49e18f1
44d9823
28ff17f
7ba4dc1
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 | ||||
---|---|---|---|---|---|---|
|
@@ -166,6 +166,7 @@ use frame_support::{ | |||||
WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, | ||||||
Imbalance, SignedImbalance, ReservableCurrency, Get, ExistenceRequirement::KeepAlive, | ||||||
ExistenceRequirement::AllowDeath, IsDeadAccount, BalanceStatus as Status, | ||||||
ReserveIdentifier, NamedReservableCurrency, | ||||||
} | ||||||
}; | ||||||
use sp_runtime::{ | ||||||
|
@@ -322,6 +323,12 @@ pub struct BalanceLock<Balance> { | |||||
pub reasons: Reasons, | ||||||
} | ||||||
|
||||||
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] | ||||||
pub struct ReserveData<Balance> { | ||||||
pub id: ReserveIdentifier, | ||||||
pub amount: Balance, | ||||||
} | ||||||
|
||||||
/// All balance information for an account. | ||||||
#[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug)] | ||||||
pub struct AccountData<Balance> { | ||||||
|
@@ -397,6 +404,8 @@ decl_storage! { | |||||
/// NOTE: Should only be accessed when setting, changing and freeing a lock. | ||||||
pub Locks get(fn locks): map hasher(blake2_128_concat) T::AccountId => Vec<BalanceLock<T::Balance>>; | ||||||
|
||||||
pub Reserves get(fn reserves): map hasher(blake2_128_concat) T::AccountId => Vec<ReserveData<T::Balance>>; | ||||||
|
||||||
/// Storage version of the pallet. | ||||||
/// | ||||||
/// This is set to v2.0.0 for new networks. | ||||||
|
@@ -1219,6 +1228,144 @@ impl<T: Config<I>, I: Instance> ReservableCurrency<T::AccountId> for Module<T, I | |||||
} | ||||||
} | ||||||
|
||||||
impl<T: Config<I>, I: Instance> NamedReservableCurrency<T::AccountId> for Module<T, I> where | ||||||
T::Balance: MaybeSerializeDeserialize + Debug | ||||||
{ | ||||||
fn reserved_balance_named(id: &ReserveIdentifier, who: &T::AccountId) -> Self::Balance { | ||||||
let reserves = Self::reserves(who); | ||||||
reserves | ||||||
.binary_search_by_key(id, |data| data.id) | ||||||
.map(|idx| reserves[idx].amount) | ||||||
.unwrap_or_default() | ||||||
} | ||||||
|
||||||
/// Move `value` from the free balance from `who` to their reserved balance. | ||||||
xlc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
/// | ||||||
/// Is a no-op if value to be reserved is zero. | ||||||
fn reserve_named(id: &ReserveIdentifier, who: &T::AccountId, value: Self::Balance) -> DispatchResult { | ||||||
xlc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
<Self as ReservableCurrency<_>>::reserve(who, value)?; | ||||||
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. hmm? you are doing a normal reserved and a named here? 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. named reserve is also a normal reserve so that the value from 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. ahh okay I thought they were entirely distinct, and you could only do either But I guess having them linked is beneficial as you can query the grand total reserved of an account which is useful. |
||||||
Reserves::<T, I>::mutate(who, |reserves| { | ||||||
match reserves.binary_search_by_key(id, |data| data.id) { | ||||||
Ok(idx) => { | ||||||
// this add can't overflow but just to be defensive. | ||||||
reserves[idx].amount = reserves[idx].amount.saturating_add(value) | ||||||
}, | ||||||
Err(idx) => { | ||||||
reserves.insert(idx, ReserveData { | ||||||
id: id.clone(), | ||||||
amount: value | ||||||
}) | ||||||
}, | ||||||
} | ||||||
}); | ||||||
Ok(()) | ||||||
} | ||||||
|
||||||
/// Unreserve some funds, returning any amount that was unable to be unreserved. | ||||||
/// | ||||||
/// Is a no-op if the value to be unreserved is zero. | ||||||
fn unreserve_named(id: &ReserveIdentifier, who: &T::AccountId, value: Self::Balance) -> Self::Balance { | ||||||
xlc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if value.is_zero() { return Zero::zero() } | ||||||
|
||||||
Reserves::<T, I>::mutate(who, |reserves| -> Self::Balance { | ||||||
match reserves.binary_search_by_key(id, |data| data.id) { | ||||||
Ok(idx) => { | ||||||
let to_change = cmp::min(reserves[idx].amount, value); | ||||||
|
||||||
let remain = <Self as ReservableCurrency<_>>::unreserve(who, to_change); | ||||||
|
||||||
// remain should always be zero but just to be defensive here | ||||||
let actual = to_change.saturating_sub(remain); | ||||||
|
||||||
// `actual <= to_change` and `to_change <= amount`; qed; | ||||||
reserves[idx].amount -= actual; | ||||||
|
||||||
value - actual | ||||||
}, | ||||||
Err(_) => { | ||||||
value | ||||||
}, | ||||||
} | ||||||
}) | ||||||
} | ||||||
|
||||||
/// Slash from reserved balance, returning the negative imbalance created, | ||||||
/// and any amount that was unable to be slashed. | ||||||
/// | ||||||
/// Is a no-op if the value to be slashed is zero. | ||||||
fn slash_reserved_named( | ||||||
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. Could foresee having a But correct me if I am wrong, I think rustc is luckily not smart about this and even if only one of the traits is in scope, it would demand the universal function invocation syntax? sth to consider overall. 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. Good idea. I think it is possible to create a 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. Something similar to what we have here https://github.com/open-web3-stack/open-runtime-module-library/blob/2de1e024781a7744a3aeb82229ebcb555163452a/currencies/src/lib.rs#L448
xlc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
id: &ReserveIdentifier, | ||||||
who: &T::AccountId, | ||||||
value: Self::Balance | ||||||
) -> (Self::NegativeImbalance, Self::Balance) { | ||||||
if value.is_zero() { return (NegativeImbalance::zero(), Zero::zero()) } | ||||||
|
||||||
Reserves::<T, I>::mutate(who, |reserves| -> (Self::NegativeImbalance, Self::Balance) { | ||||||
match reserves.binary_search_by_key(id, |data| data.id) { | ||||||
Ok(idx) => { | ||||||
let to_change = cmp::min(reserves[idx].amount, value); | ||||||
|
||||||
let (imb, remain) = <Self as ReservableCurrency<_>>::slash_reserved(who, to_change); | ||||||
|
||||||
// remain should always be zero but just to be defensive here | ||||||
let actual = to_change.saturating_sub(remain); | ||||||
|
||||||
// `actual <= to_change` and `to_change <= amount`; qed; | ||||||
reserves[idx].amount -= actual; | ||||||
|
||||||
(imb, value - actual) | ||||||
}, | ||||||
Err(_) => { | ||||||
(NegativeImbalance::zero(), value) | ||||||
}, | ||||||
} | ||||||
}) | ||||||
} | ||||||
|
||||||
/// Move the reserved balance of one account into the balance of another, according to `status`. | ||||||
/// | ||||||
/// Is a no-op if: | ||||||
/// - the value to be moved is zero; or | ||||||
/// - the `slashed` id equal to `beneficiary` and the `status` is `Reserved`. | ||||||
fn repatriate_reserved_named( | ||||||
xlc marked this conversation as resolved.
Show resolved
Hide resolved
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.
Suggested change
probably doesnt matter, but name here is strange 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 simply add |
||||||
id: &ReserveIdentifier, | ||||||
slashed: &T::AccountId, | ||||||
beneficiary: &T::AccountId, | ||||||
value: Self::Balance, | ||||||
status: Status, | ||||||
) -> Result<Self::Balance, DispatchError> { | ||||||
if value.is_zero() { return Ok(Zero::zero()) } | ||||||
|
||||||
if slashed == beneficiary { | ||||||
return match status { | ||||||
Status::Free => Ok(Self::unreserve_named(id, slashed, value)), | ||||||
Status::Reserved => Ok(value.saturating_sub(Self::reserved_balance_named(id, slashed))), | ||||||
}; | ||||||
} | ||||||
|
||||||
Reserves::<T, I>::try_mutate(slashed, |reserves| -> Result<Self::Balance, DispatchError> { | ||||||
match reserves.binary_search_by_key(id, |data| data.id) { | ||||||
Ok(idx) => { | ||||||
let to_change = cmp::min(reserves[idx].amount, value); | ||||||
|
||||||
let remain = <Self as ReservableCurrency<_>>::repatriate_reserved(slashed, beneficiary, to_change, status)?; | ||||||
|
||||||
// remain should always be zero but just to be defensive here | ||||||
let actual = to_change.saturating_sub(remain); | ||||||
|
||||||
// `actual <= to_change` and `to_change <= amount`; qed; | ||||||
reserves[idx].amount -= actual; | ||||||
|
||||||
Ok(value - actual) | ||||||
}, | ||||||
Err(_) => { | ||||||
Ok(value) | ||||||
}, | ||||||
} | ||||||
}) | ||||||
} | ||||||
} | ||||||
|
||||||
/// Implement `OnKilledAccount` to remove the local account, if using local account storage. | ||||||
/// | ||||||
/// NOTE: You probably won't need to use this! This only needs to be "wired in" to System module | ||||||
|
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.
Note that despite the binary search, this is still
O(named_reservations)
(due to needing the full deserialisation ofSelf::reserves
).