[Uniques V2] Refactor roles (#12437)

* Basics

* WIP: change the data format

* Refactor

* Remove redundant new() method

* Rename settings

* Enable tests

* Chore

* Change params order

* Delete the config on collection removal

* Chore

* Remove redundant system features

* Rename force_item_status to force_collection_status

* Update node runtime

* Chore

* Remove thaw_collection

* Chore

* Connect collection.is_frozen to config

* Allow to lock the collection in a new way

* Move free_holding into settings

* Connect collection's metadata locker to feature flags

* DRY

* Chore

* Connect pallet level feature flags

* Prepare tests for the new changes

* Implement Item settings

* Allow to lock the metadata or attributes of an item

* Common -> Settings

* Extract settings related code to a separate file

* Move feature flag checks inside the do_* methods

* Split settings.rs into parts

* Extract repeated code into macro

* Extract macros into their own file

* Chore

* Fix traits

* Fix traits

* Test SystemFeatures

* Fix benchmarks

* Add missing benchmark

* Fix node/runtime/lib.rs

* ".git/.scripts/bench-bot.sh" pallet dev pallet_nfts

* Keep item's config on burn if it's not empty

* Fix the merge artifacts

* Fmt

* Add SystemFeature::NoSwaps check

* Refactor roles structure

* Rename SystemFeatures to PalletFeatures

* Rename errors

* Add docs

* Change error message

* Rework pallet features

* Move macros

* Change comments

* Fmt

* Refactor Incrementable

* Use pub(crate) for do_* functions

* Update comments

* Refactor freeze and lock functions

* Rework Collection config and Item confg api

* Chore

* Make clippy happy

* Chore

* Fix artifacts

* Address comments

* Further refactoring

* Add comments

* Add tests for group_roles_by_account()

* Update frame/nfts/src/impl_nonfungibles.rs

* Add test

* Replace Itertools group_by with a custom implementation

* ItemsNotTransferable => ItemsNonTransferable

* Update frame/nfts/src/features/roles.rs

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* Address PR comments

* Add missed comment

Co-authored-by: command-bot <>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

frame/nfts/src/benchmarking.rs

@@ -68,7 +68,7 @@ fn add_collection_metadata<T: Config<I>, I: 'static>() -> (T::AccountId, Account
 fn mint_item<T: Config<I>, I: 'static>(
 	index: u16,
 ) -> (T::ItemId, T::AccountId, AccountIdLookupOf<T>) {
-	let caller = Collection::<T, I>::get(T::Helper::collection(0)).unwrap().admin;
+	let caller = Collection::<T, I>::get(T::Helper::collection(0)).unwrap().owner;
 	if caller != whitelisted_caller() {
 		whitelist_account!(caller);
 	}

frame/nfts/src/features/lock.rs

@@ -24,10 +24,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		collection: T::CollectionId,
 		lock_config: CollectionConfig,
 	) -> DispatchResult {
-		let details =
-			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
-		ensure!(origin == details.freezer, Error::<T, I>::NoPermission);
-
+		ensure!(
+			Self::has_role(&collection, &origin, CollectionRole::Freezer),
+			Error::<T, I>::NoPermission
+		);
 		CollectionConfigOf::<T, I>::try_mutate(collection, |maybe_config| {
 			let config = maybe_config.as_mut().ok_or(Error::<T, I>::NoConfig)?;
 
@@ -51,9 +51,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		collection: T::CollectionId,
 		item: T::ItemId,
 	) -> DispatchResult {
-		let collection_details =
-			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
-		ensure!(collection_details.freezer == origin, Error::<T, I>::NoPermission);
+		ensure!(
+			Self::has_role(&collection, &origin, CollectionRole::Freezer),
+			Error::<T, I>::NoPermission
+		);
 
 		let mut config = Self::get_item_config(&collection, &item)?;
 		if !config.has_disabled_setting(ItemSetting::Transferable) {
@@ -70,9 +71,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		collection: T::CollectionId,
 		item: T::ItemId,
 	) -> DispatchResult {
-		let collection_details =
-			Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
-		ensure!(collection_details.freezer == origin, Error::<T, I>::NoPermission);
+		ensure!(
+			Self::has_role(&collection, &origin, CollectionRole::Freezer),
+			Error::<T, I>::NoPermission
+		);
 
 		let mut config = Self::get_item_config(&collection, &item)?;
 		if config.has_disabled_setting(ItemSetting::Transferable) {

frame/nfts/src/features/mod.rs

@@ -18,4 +18,5 @@
 pub mod atomic_swap;
 pub mod buy_sell;
 pub mod lock;
+pub mod roles;
 pub mod settings;

frame/nfts/src/features/roles.rs

@@ -0,0 +1,69 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::*;
+use frame_support::pallet_prelude::*;
+use sp_std::collections::btree_map::BTreeMap;
+
+impl<T: Config<I>, I: 'static> Pallet<T, I> {
+	/// Clears all the roles in a specified collection.
+	///
+	/// - `collection_id`: A collection to clear the roles in.
+	///
+	/// Throws an error if some of the roles were left in storage.
+	/// This means the `CollectionRoles::max_roles()` needs to be adjusted.
+	pub(crate) fn clear_roles(collection_id: &T::CollectionId) -> Result<(), DispatchError> {
+		let res = CollectionRoleOf::<T, I>::clear_prefix(
+			&collection_id,
+			CollectionRoles::max_roles() as u32,
+			None,
+		);
+		ensure!(res.maybe_cursor.is_none(), Error::<T, I>::RolesNotCleared);
+		Ok(())
+	}
+
+	/// Returns true if a specified account has a provided role within that collection.
+	///
+	/// - `collection_id`: A collection to check the role in.
+	/// - `account_id`: An account to check the role for.
+	/// - `role`: A role to validate.
+	///
+	/// Returns boolean.
+	pub(crate) fn has_role(
+		collection_id: &T::CollectionId,
+		account_id: &T::AccountId,
+		role: CollectionRole,
+	) -> bool {
+		CollectionRoleOf::<T, I>::get(&collection_id, &account_id)
+			.map_or(false, |roles| roles.has_role(role))
+	}
+
+	/// Groups provided roles by account, given one account could have multiple roles.
+	///
+	/// - `input`: A vector of (Account, Role) tuples.
+	///
+	/// Returns a grouped vector.
+	pub fn group_roles_by_account(
+		input: Vec<(T::AccountId, CollectionRole)>,
+	) -> Vec<(T::AccountId, CollectionRoles)> {
+		let mut result = BTreeMap::new();
+		for (account, role) in input.into_iter() {
+			result.entry(account).or_insert(CollectionRoles::none()).add_role(role);
+		}
+		result.into_iter().collect()
+	}
+}

frame/nfts/src/functions.rs

@@ -41,7 +41,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let collection_config = Self::get_collection_config(&collection)?;
 		ensure!(
 			collection_config.is_setting_enabled(CollectionSetting::TransferableItems),
-			Error::<T, I>::ItemsNotTransferable
+			Error::<T, I>::ItemsNonTransferable
 		);
 
 		let item_config = Self::get_item_config(&collection, &item)?;
@@ -93,15 +93,19 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			collection,
 			CollectionDetails {
 				owner: owner.clone(),
-				issuer: admin.clone(),
-				admin: admin.clone(),
-				freezer: admin,
 				total_deposit: deposit,
 				items: 0,
 				item_metadatas: 0,
 				attributes: 0,
 			},
 		);
+		CollectionRoleOf::<T, I>::insert(
+			collection,
+			admin,
+			CollectionRoles(
+				CollectionRole::Admin | CollectionRole::Freezer | CollectionRole::Issuer,
+			),
+		);
 
 		let next_id = collection.increment();
 
@@ -142,6 +146,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			#[allow(deprecated)]
 			PendingSwapOf::<T, I>::remove_prefix(&collection, None);
 			CollectionMetadataOf::<T, I>::remove(&collection);
+			Self::clear_roles(&collection)?;
 			#[allow(deprecated)]
 			Attribute::<T, I>::remove_prefix((&collection,), None);
 			CollectionAccount::<T, I>::remove(&collection_details.owner, &collection);
@@ -165,7 +170,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		item: T::ItemId,
 		owner: T::AccountId,
 		config: ItemConfig,
-		with_details: impl FnOnce(&CollectionDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
 		ensure!(!Item::<T, I>::contains_key(collection, item), Error::<T, I>::AlreadyExists);
 
@@ -175,8 +179,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				let collection_details =
 					maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 
-				with_details(collection_details)?;
-
 				if let Ok(max_supply) = CollectionMaxSupply::<T, I>::try_get(&collection) {
 					ensure!(collection_details.items < max_supply, Error::<T, I>::MaxSupplyReached);
 				}
@@ -218,7 +220,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	pub fn do_burn(
 		collection: T::CollectionId,
 		item: T::ItemId,
-		with_details: impl FnOnce(&CollectionDetailsFor<T, I>, &ItemDetailsFor<T, I>) -> DispatchResult,
+		with_details: impl FnOnce(&ItemDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
 		let owner = Collection::<T, I>::try_mutate(
 			&collection,
@@ -227,7 +229,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 					maybe_collection_details.as_mut().ok_or(Error::<T, I>::UnknownCollection)?;
 				let details = Item::<T, I>::get(&collection, &item)
 					.ok_or(Error::<T, I>::UnknownCollection)?;
-				with_details(collection_details, &details)?;
+				with_details(&details)?;
 
 				// Return the deposit.
 				T::Currency::unreserve(&collection_details.owner, details.deposit);
@@ -271,7 +273,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let collection_config = Self::get_collection_config(&collection)?;
 		ensure!(
 			collection_config.is_setting_enabled(CollectionSetting::TransferableItems),
-			Error::<T, I>::ItemsNotTransferable
+			Error::<T, I>::ItemsNonTransferable
 		);
 
 		let item_config = Self::get_item_config(&collection, &item)?;

frame/nfts/src/impl_nonfungibles.rs

@@ -143,15 +143,15 @@ impl<T: Config<I>, I: 'static> Mutate<<T as SystemConfig>::AccountId, ItemSettin
 		who: &T::AccountId,
 		settings: &ItemSettings,
 	) -> DispatchResult {
-		Self::do_mint(*collection, *item, who.clone(), ItemConfig(*settings), |_| Ok(()))
+		Self::do_mint(*collection, *item, who.clone(), ItemConfig(*settings))
 	}
 
 	fn burn(
 		collection: &Self::CollectionId,
 		item: &Self::ItemId,
 		maybe_check_owner: Option<&T::AccountId>,
 	) -> DispatchResult {
-		Self::do_burn(*collection, *item, |_, d| {
+		Self::do_burn(*collection, *item, |d| {
 			if let Some(check_owner) = maybe_check_owner {
 				if &d.owner != check_owner {
 					return Err(Error::<T, I>::NoPermission.into())