Revert "Auto-incremental CollectionId (paritytech#11796)" (paritytech…

…#12059)

This reverts commit 7d8e5a1.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

frame/uniques/src/benchmarking.rs

@@ -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)
 }
 
@@ -139,7 +142,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());
@@ -148,7 +151,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());
 	}
@@ -404,16 +407,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);

frame/uniques/src/functions.rs

@@ -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(())
 	}
@@ -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,

frame/uniques/src/lib.rs

@@ -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::*;
@@ -94,12 +94,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;
@@ -273,12 +268,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 = ()> {
@@ -370,8 +359,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,
@@ -423,10 +410,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.
@@ -458,16 +441,19 @@ 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.
 		///
 		/// Emits `Created` event when successful.
 		///
 		/// Weight: `O(1)`
 		#[pallet::weight(T::WeightInfo::create())]
-		pub fn create(origin: OriginFor<T>, admin: AccountIdLookupOf<T>) -> DispatchResult {
-			let collection = NextCollectionId::<T, I>::get();
-
+		pub fn create(
+			origin: OriginFor<T>,
+			collection: T::CollectionId,
+			admin: AccountIdLookupOf<T>,
+		) -> DispatchResult {
 			let owner = T::CreateOrigin::ensure_origin(origin, &collection)?;
 			let admin = T::Lookup::lookup(admin)?;
 
@@ -489,6 +475,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
@@ -500,14 +487,13 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::force_create())]
 		pub fn force_create(
 			origin: OriginFor<T>,
+			collection: T::CollectionId,
 			owner: AccountIdLookupOf<T>,
 			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(),
@@ -518,31 +504,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