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

Revert "Auto-incremental CollectionId" #12059

Merged
merged 2 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 9 additions & 16 deletions frame/uniques/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,13 @@ fn create_collection<T: Config<I>, I: 'static>(
let caller_lookup = T::Lookup::unlookup(caller.clone());
let collection = T::Helper::collection(0);
T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
assert!(
Uniques::<T, I>::force_create(SystemOrigin::Root.into(), caller_lookup.clone(), false,)
.is_ok()
);
assert!(Uniques::<T, I>::force_create(
SystemOrigin::Root.into(),
collection,
caller_lookup.clone(),
false,
)
.is_ok());
(collection, caller, caller_lookup)
}

Expand Down Expand Up @@ -140,7 +143,7 @@ benchmarks_instance_pallet! {
whitelist_account!(caller);
let admin = T::Lookup::unlookup(caller.clone());
T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
let call = Call::<T, I>::create { admin };
let call = Call::<T, I>::create { collection, admin };
}: { call.dispatch_bypass_filter(origin)? }
verify {
assert_last_event::<T, I>(Event::Created { collection: T::Helper::collection(0), creator: caller.clone(), owner: caller }.into());
Expand All @@ -149,7 +152,7 @@ benchmarks_instance_pallet! {
force_create {
let caller: T::AccountId = whitelisted_caller();
let caller_lookup = T::Lookup::unlookup(caller.clone());
}: _(SystemOrigin::Root, caller_lookup, true)
}: _(SystemOrigin::Root, T::Helper::collection(0), caller_lookup, true)
verify {
assert_last_event::<T, I>(Event::ForceCreated { collection: T::Helper::collection(0), owner: caller }.into());
}
Expand Down Expand Up @@ -405,16 +408,6 @@ benchmarks_instance_pallet! {
}.into());
}

try_increment_id {
let (_, caller, _) = create_collection::<T, I>();
Uniques::<T, I>::set_next_id(0);
}: _(SystemOrigin::Signed(caller.clone()))
verify {
assert_last_event::<T, I>(Event::NextCollectionIdIncremented {
next_id: 1u32.into()
}.into());
}

set_price {
let (collection, caller, _) = create_collection::<T, I>();
let (item, ..) = mint_item::<T, I>(0);
Expand Down
15 changes: 0 additions & 15 deletions frame/uniques/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
},
);

let next_id = collection.saturating_add(1u32.into());

CollectionAccount::<T, I>::insert(&owner, &collection, ());
NextCollectionId::<T, I>::set(next_id);

Self::deposit_event(Event::NextCollectionIdIncremented { next_id });
Self::deposit_event(event);
Ok(())
}
Expand Down Expand Up @@ -213,16 +208,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(())
}

#[cfg(any(test, feature = "runtime-benchmarks"))]
pub fn set_next_id(count: u32) {
NextCollectionId::<T, I>::set(count.into());
}

#[cfg(test)]
pub fn get_next_id() -> T::CollectionId {
NextCollectionId::<T, I>::get()
}

pub fn do_set_price(
collection: T::CollectionId,
item: T::ItemId,
Expand Down
54 changes: 6 additions & 48 deletions frame/uniques/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use frame_support::{
};
use frame_system::Config as SystemConfig;
use sp_runtime::{
traits::{AtLeast32BitUnsigned, Saturating, StaticLookup, Zero},
traits::{Saturating, StaticLookup, Zero},
ArithmeticError, RuntimeDebug,
};
use sp_std::prelude::*;
Expand Down Expand Up @@ -92,12 +92,7 @@ pub mod pallet {
type Event: From<Event<Self, I>> + IsType<<Self as frame_system::Config>::Event>;

/// Identifier for the collection of item.
type CollectionId: Member
+ Parameter
+ MaxEncodedLen
+ Copy
+ Default
+ AtLeast32BitUnsigned;
type CollectionId: Member + Parameter + MaxEncodedLen + Copy;

/// The type used to identify a unique item within a collection.
type ItemId: Member + Parameter + MaxEncodedLen + Copy;
Expand Down Expand Up @@ -271,12 +266,6 @@ pub mod pallet {
pub(super) type CollectionMaxSupply<T: Config<I>, I: 'static = ()> =
StorageMap<_, Blake2_128Concat, T::CollectionId, u32, OptionQuery>;

#[pallet::storage]
/// Stores the `CollectionId` that is going to be used for the next collection.
/// This gets incremented by 1 whenever a new collection is created.
pub(super) type NextCollectionId<T: Config<I>, I: 'static = ()> =
StorageValue<_, T::CollectionId, ValueQuery>;

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config<I>, I: 'static = ()> {
Expand Down Expand Up @@ -368,8 +357,6 @@ pub mod pallet {
OwnershipAcceptanceChanged { who: T::AccountId, maybe_collection: Option<T::CollectionId> },
/// Max supply has been set for a collection.
CollectionMaxSupplySet { collection: T::CollectionId, max_supply: u32 },
/// Event gets emmited when the `NextCollectionId` gets incremented.
NextCollectionIdIncremented { next_id: T::CollectionId },
/// The price was set for the instance.
ItemPriceSet {
collection: T::CollectionId,
Expand Down Expand Up @@ -421,10 +408,6 @@ pub mod pallet {
MaxSupplyAlreadySet,
/// The provided max supply is less to the amount of items a collection already has.
MaxSupplyTooSmall,
/// The `CollectionId` in `NextCollectionId` is not being used.
///
/// This means that you can directly proceed to call `create`.
NextIdNotUsed,
/// The given item ID is unknown.
UnknownItem,
/// Item is not for sale.
Expand Down Expand Up @@ -456,6 +439,7 @@ pub mod pallet {
/// `ItemDeposit` funds of sender are reserved.
///
/// Parameters:
/// - `collection`: The identifier of the new collection. This must not be currently in use.
/// - `admin`: The admin of this collection. The admin is the initial address of each
/// member of the collection's admin team.
///
Expand All @@ -465,10 +449,9 @@ pub mod pallet {
#[pallet::weight(T::WeightInfo::create())]
pub fn create(
origin: OriginFor<T>,
collection: T::CollectionId,
admin: <T::Lookup as StaticLookup>::Source,
) -> DispatchResult {
let collection = NextCollectionId::<T, I>::get();

let owner = T::CreateOrigin::ensure_origin(origin, &collection)?;
let admin = T::Lookup::lookup(admin)?;

Expand All @@ -490,6 +473,7 @@ pub mod pallet {
///
/// Unlike `create`, no funds are reserved.
///
/// - `collection`: The identifier of the new item. This must not be currently in use.
/// - `owner`: The owner of this collection of items. The owner has full superuser
/// permissions
/// over this item, but may later change and configure the permissions using
Expand All @@ -501,14 +485,13 @@ pub mod pallet {
#[pallet::weight(T::WeightInfo::force_create())]
pub fn force_create(
origin: OriginFor<T>,
collection: T::CollectionId,
owner: <T::Lookup as StaticLookup>::Source,
free_holding: bool,
) -> DispatchResult {
T::ForceOrigin::ensure_origin(origin)?;
let owner = T::Lookup::lookup(owner)?;

let collection = NextCollectionId::<T, I>::get();

Self::do_create_collection(
collection,
owner.clone(),
Expand All @@ -519,31 +502,6 @@ pub mod pallet {
)
}

/// Increments the `CollectionId` stored in `NextCollectionId`.
///
/// This is only callable when the next `CollectionId` is already being
/// used for some other collection.
///
/// The origin must be Signed and the sender must have sufficient funds
/// free.
///
/// Emits `NextCollectionIdIncremented` event when successful.
///
/// Weight: `O(1)`
#[pallet::weight(T::WeightInfo::try_increment_id())]
pub fn try_increment_id(origin: OriginFor<T>) -> DispatchResult {
ensure_signed(origin)?;
ensure!(
Collection::<T, I>::contains_key(NextCollectionId::<T, I>::get()),
Error::<T, I>::NextIdNotUsed
);

let next_id = NextCollectionId::<T, I>::get().saturating_add(1u32.into());
NextCollectionId::<T, I>::set(next_id);
Self::deposit_event(Event::NextCollectionIdIncremented { next_id });
Ok(())
}

/// Destroy a collection of fungible items.
///
/// The origin must conform to `ForceOrigin` or must be `Signed` and the sender must be the
Expand Down