Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Use StorageNMap for Approvals in assets pallet (#8816)
Browse files Browse the repository at this point in the history 
* Use StorageNMap for Approvals in assets pallet

* Use EncodeLike on HashKeyPrefix trait bounds

* Add comments clarifying AccountId roles

* Properly document the keys in the Approvals storage

* Fix line width
  • Loading branch information
@KiChjang
KiChjang authored May 17, 2021
1 parent cf2d931 commit 66c85ae
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 57 deletions.
42 changes: 20 additions & 22 deletions frame/assets/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,14 @@ pub mod pallet {
#[pallet::storage]
/// Approved balance transfers. First balance is the amount approved for transfer. Second
/// is the amount of `T::Currency` reserved for storing this.
pub(super) type Approvals<T: Config<I>, I: 'static = ()> = StorageDoubleMap<
/// First key is the asset ID, second key is the owner and third key is the delegate.
pub(super) type Approvals<T: Config<I>, I: 'static = ()> = StorageNMap<
_,
Blake2_128Concat,
T::AssetId,
Blake2_128Concat,
ApprovalKey<T::AccountId>,
(
NMapKey<Blake2_128Concat, T::AssetId>,
NMapKey<Blake2_128Concat, T::AccountId>, // owner
NMapKey<Blake2_128Concat, T::AccountId>, // delegate
),
Approval<T::Balance, DepositBalanceOf<T, I>>,
OptionQuery,
>;
Expand Down Expand Up @@ -502,7 +504,7 @@ pub mod pallet {
details.deposit.saturating_add(metadata.deposit),
);

Approvals::<T, I>::remove_prefix(&id);
Approvals::<T, I>::remove_prefix((&id,));
Self::deposit_event(Event::Destroyed(id));

// NOTE: could use postinfo to reflect the actual number of accounts/sufficient/approvals
Expand Down Expand Up @@ -1118,19 +1120,18 @@ pub mod pallet {
let owner = ensure_signed(origin)?;
let delegate = T::Lookup::lookup(delegate)?;

let key = ApprovalKey { owner, delegate };
Approvals::<T, I>::try_mutate(id, &key, |maybe_approved| -> DispatchResult {
Approvals::<T, I>::try_mutate((id, &owner, &delegate), |maybe_approved| -> DispatchResult {
let mut approved = maybe_approved.take().unwrap_or_default();
let deposit_required = T::ApprovalDeposit::get();
if approved.deposit < deposit_required {
T::Currency::reserve(&key.owner, deposit_required - approved.deposit)?;
T::Currency::reserve(&owner, deposit_required - approved.deposit)?;
approved.deposit = deposit_required;
}
approved.amount = approved.amount.saturating_add(amount);
*maybe_approved = Some(approved);
Ok(())
})?;
Self::deposit_event(Event::ApprovedTransfer(id, key.owner, key.delegate, amount));
Self::deposit_event(Event::ApprovedTransfer(id, owner, delegate, amount));

Ok(())
}
Expand All @@ -1156,11 +1157,10 @@ pub mod pallet {
) -> DispatchResult {
let owner = ensure_signed(origin)?;
let delegate = T::Lookup::lookup(delegate)?;
let key = ApprovalKey { owner, delegate };
let approval = Approvals::<T, I>::take(id, &key).ok_or(Error::<T, I>::Unknown)?;
T::Currency::unreserve(&key.owner, approval.deposit);
let approval = Approvals::<T, I>::take((id, &owner, &delegate)).ok_or(Error::<T, I>::Unknown)?;
T::Currency::unreserve(&owner, approval.deposit);

Self::deposit_event(Event::ApprovalCancelled(id, key.owner, key.delegate));
Self::deposit_event(Event::ApprovalCancelled(id, owner, delegate));
Ok(())
}

Expand Down Expand Up @@ -1196,11 +1196,10 @@ pub mod pallet {
let owner = T::Lookup::lookup(owner)?;
let delegate = T::Lookup::lookup(delegate)?;

let key = ApprovalKey { owner, delegate };
let approval = Approvals::<T, I>::take(id, &key).ok_or(Error::<T, I>::Unknown)?;
T::Currency::unreserve(&key.owner, approval.deposit);
let approval = Approvals::<T, I>::take((id, &owner, &delegate)).ok_or(Error::<T, I>::Unknown)?;
T::Currency::unreserve(&owner, approval.deposit);

Self::deposit_event(Event::ApprovalCancelled(id, key.owner, key.delegate));
Self::deposit_event(Event::ApprovalCancelled(id, owner, delegate));
Ok(())
}

Expand Down Expand Up @@ -1234,8 +1233,7 @@ pub mod pallet {
let owner = T::Lookup::lookup(owner)?;
let destination = T::Lookup::lookup(destination)?;

let key = ApprovalKey { owner, delegate };
Approvals::<T, I>::try_mutate_exists(id, &key, |maybe_approved| -> DispatchResult {
Approvals::<T, I>::try_mutate_exists((id, &owner, delegate), |maybe_approved| -> DispatchResult {
let mut approved = maybe_approved.take().ok_or(Error::<T, I>::Unapproved)?;
let remaining = approved
.amount
Expand All @@ -1247,10 +1245,10 @@ pub mod pallet {
best_effort: false,
burn_dust: false
};
Self::do_transfer(id, &key.owner, &destination, amount, None, f)?;
Self::do_transfer(id, &owner, &destination, amount, None, f)?;

if remaining.is_zero() {
T::Currency::unreserve(&key.owner, approved.deposit);
T::Currency::unreserve(&owner, approved.deposit);
} else {
approved.amount = remaining;
*maybe_approved = Some(approved);
Expand Down
9 changes: 0 additions & 9 deletions frame/assets/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,6 @@ impl<Balance, AccountId, DepositBalance> AssetDetails<Balance, AccountId, Deposi
}
}

/// A pair to act as a key for the approval storage map.
#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug)]
pub struct ApprovalKey<AccountId> {
/// The owner of the funds that are being approved.
pub(super) owner: AccountId,
/// The party to whom transfer of the funds is being delegated.
pub(super) delegate: AccountId,
}

/// Data concerning an approval.
#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default)]
pub struct Approval<Balance, DepositBalance> {
Expand Down
74 changes: 48 additions & 26 deletions frame/support/src/storage/types/key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,19 +248,23 @@ pub trait HasReversibleKeyPrefix<P>: ReversibleKeyGenerator + HasKeyPrefix<P> {
macro_rules! impl_key_prefix_for {
(($($keygen:ident),+), ($($prefix:ident),+), ($($suffix:ident),+)) => {
paste! {
impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: StorageHasher),+>
HasKeyPrefix<($($prefix),+)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: StorageHasher,)+
$( [<KArg $prefix>]: EncodeLike<$prefix> ),+
> HasKeyPrefix<($( [<KArg $prefix>] ),+)> for ($(Key<[<$keygen $keygen>], $keygen>),+) {
type Suffix = ($($suffix),+);

fn partial_key(prefix: ($($prefix),+)) -> Vec<u8> {
fn partial_key(prefix: ($( [<KArg $prefix>] ),+)) -> Vec<u8> {
<($(Key<[<$prefix $prefix>], $prefix>),+)>::final_key(prefix)
}
}

impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: ReversibleStorageHasher),+>
HasReversibleKeyPrefix<($($prefix),+)> for
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: ReversibleStorageHasher,)+
$( [<KArg $prefix>]: EncodeLike<$prefix> ),+
> HasReversibleKeyPrefix<($( [<KArg $prefix>] ),+)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
fn decode_partial_key(key_material: &[u8]) -> Result<Self::Suffix, codec::Error> {
Expand All @@ -271,19 +275,23 @@ macro_rules! impl_key_prefix_for {
};
(($($keygen:ident),+), $prefix:ident, ($($suffix:ident),+)) => {
paste! {
impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: StorageHasher),+>
HasKeyPrefix<($prefix,)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: StorageHasher,)+
[<KArg $prefix>]: EncodeLike<$prefix>
> HasKeyPrefix<( [<KArg $prefix>] ,)> for ($(Key<[<$keygen $keygen>], $keygen>),+) {
type Suffix = ($($suffix),+);

fn partial_key(prefix: ($prefix,)) -> Vec<u8> {
fn partial_key(prefix: ( [<KArg $prefix>] ,)) -> Vec<u8> {
<Key<[<$prefix $prefix>], $prefix>>::final_key(prefix)
}
}

impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: ReversibleStorageHasher),+>
HasReversibleKeyPrefix<($prefix,)> for
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: ReversibleStorageHasher,)+
[<KArg $prefix>]: EncodeLike<$prefix>
> HasReversibleKeyPrefix<( [<KArg $prefix>] ,)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
fn decode_partial_key(key_material: &[u8]) -> Result<Self::Suffix, codec::Error> {
Expand All @@ -294,19 +302,23 @@ macro_rules! impl_key_prefix_for {
};
(($($keygen:ident),+), ($($prefix:ident),+), $suffix:ident) => {
paste! {
impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: StorageHasher),+>
HasKeyPrefix<($($prefix),+)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: StorageHasher,)+
$( [<KArg $prefix>]: EncodeLike<$prefix>),+
> HasKeyPrefix<($( [<KArg $prefix>] ),+)> for ($(Key<[<$keygen $keygen>], $keygen>),+) {
type Suffix = $suffix;

fn partial_key(prefix: ($($prefix),+)) -> Vec<u8> {
fn partial_key(prefix: ($( [<KArg $prefix>] ),+)) -> Vec<u8> {
<($(Key<[<$prefix $prefix>], $prefix>),+)>::final_key(prefix)
}
}

impl<$($keygen: FullCodec,)+ $( [<$keygen $keygen>]: ReversibleStorageHasher),+>
HasReversibleKeyPrefix<($($prefix),+)> for
impl<
$($keygen: FullCodec,)+
$( [<$keygen $keygen>]: ReversibleStorageHasher,)+
$( [<KArg $prefix>]: EncodeLike<$prefix> ),+
> HasReversibleKeyPrefix<($( [<KArg $prefix>] ),+)> for
($(Key<[<$keygen $keygen>], $keygen>),+)
{
fn decode_partial_key(key_material: &[u8]) -> Result<Self::Suffix, codec::Error> {
Expand All @@ -317,18 +329,28 @@ macro_rules! impl_key_prefix_for {
};
}

impl<A: FullCodec, B: FullCodec, X: StorageHasher, Y: StorageHasher> HasKeyPrefix<(A,)>
for (Key<X, A>, Key<Y, B>)
impl<A, B, X, Y, KArg> HasKeyPrefix<(KArg,)> for (Key<X, A>, Key<Y, B>)
where
A: FullCodec,
B: FullCodec,
X: StorageHasher,
Y: StorageHasher,
KArg: EncodeLike<A>,
{
type Suffix = B;

fn partial_key(prefix: (A,)) -> Vec<u8> {
fn partial_key(prefix: (KArg,)) -> Vec<u8> {
<Key<X, A>>::final_key(prefix)
}
}

impl<A: FullCodec, B: FullCodec, X: ReversibleStorageHasher, Y: ReversibleStorageHasher>
HasReversibleKeyPrefix<(A,)> for (Key<X, A>, Key<Y, B>)
impl<A, B, X, Y, KArg> HasReversibleKeyPrefix<(KArg,)> for (Key<X, A>, Key<Y, B>)
where
A: FullCodec,
B: FullCodec,
X: ReversibleStorageHasher,
Y: ReversibleStorageHasher,
KArg: EncodeLike<A>,
{
fn decode_partial_key(key_material: &[u8]) -> Result<B, codec::Error> {
<Key<Y, B>>::decode_final_key(key_material).map(|k| k.0)
Expand Down

0 comments on commit 66c85ae

Please sign in to comment.