[Uniques V2] Final improvements (#12736)

* Use KeyPrefixIterator instead of Box

* Change create_collection()

* Restrict from claiming NFTs twice

* Update Readme

* Remove dead code

* Refactoring

* Update readme

* Fix clippy

bin/node/runtime/src/lib.rs

@@ -1519,6 +1519,7 @@ impl pallet_uniques::Config for Runtime {
 
 parameter_types! {
 	pub Features: PalletFeatures = PalletFeatures::all_enabled();
+	pub const NftsPalletId: PalletId = PalletId(*b"py/nfts_");
 }
 
 impl pallet_nfts::Config for Runtime {
@@ -1541,6 +1542,7 @@ impl pallet_nfts::Config for Runtime {
 	type MaxDeadlineDuration = MaxDeadlineDuration;
 	type Features = Features;
 	type WeightInfo = pallet_nfts::weights::SubstrateWeight<Runtime>;
+	type PalletId = NftsPalletId;
 	#[cfg(feature = "runtime-benchmarks")]
 	type Helper = ();
 	type CreateOrigin = AsEnsureOriginWithArg<EnsureSigned<AccountId>>;

frame/nfts/README.md

@@ -1,72 +1,100 @@
-# Uniques Module
+# NFTs pallet
 
-A simple, secure module for dealing with non-fungible assets.
+A pallet for dealing with non-fungible assets.
 
 ## Overview
 
-The Uniques module provides functionality for asset management of non-fungible asset classes, including:
+The NFTs pallet provides functionality for non-fungible tokens' management, including:
 
-* Asset Issuance
-* Asset Transfer
-* Asset Destruction
+* Collection Creation
+* NFT Minting
+* NFT Transfers and Atomic Swaps
+* NFT Trading methods
+* Attributes Management
+* NFT Burning
 
-To use it in your runtime, you need to implement the assets [`uniques::Config`](https://paritytech.github.io/substrate/master/pallet_uniques/pallet/trait.Config.html).
+To use it in your runtime, you need to implement [`nfts::Config`](https://paritytech.github.io/substrate/master/pallet_nfts/pallet/trait.Config.html).
 
-The supported dispatchable functions are documented in the [`uniques::Call`](https://paritytech.github.io/substrate/master/pallet_uniques/pallet/enum.Call.html) enum.
+The supported dispatchable functions are documented in the [`nfts::Call`](https://paritytech.github.io/substrate/master/pallet_nfts/pallet/enum.Call.html) enum.
 
 ### Terminology
 
-* **Asset issuance:** The creation of a new asset instance.
-* **Asset transfer:** The action of transferring an asset instance from one account to another.
-* **Asset burning:** The destruction of an asset instance.
-* **Non-fungible asset:** An asset for which each unit has unique characteristics. There is exactly
-  one instance of such an asset in existence and there is exactly one owning account.
+* **Collection creation:** The creation of a new collection.
+* **NFT minting:** The action of creating a new item within a collection.
+* **NFT transfer:** The action of sending an item from one account to another.
+* **Atomic swap:** The action of exchanging items between accounts without needing a 3rd party service.
+* **NFT burning:** The destruction of an item.
+* **Non-fungible token (NFT):** An item for which each unit has unique characteristics. There is exactly
+  one instance of such an item in existence and there is exactly one owning account (though that owning account could be a proxy account or multi-sig account).
+* **Soul Bound NFT:** An item that is non-transferable from the account which it is minted into.
 
 ### Goals
 
-The Uniques pallet in Substrate is designed to make the following possible:
+The NFTs pallet in Substrate is designed to make the following possible:
 
-* Allow accounts to permissionlessly create asset classes (collections of asset instances).
-* Allow a named (permissioned) account to mint and burn unique assets within a class.
-* Move asset instances between accounts permissionlessly.
-* Allow a named (permissioned) account to freeze and unfreeze unique assets within a
-  class or the entire class.
-* Allow the owner of an asset instance to delegate the ability to transfer the asset to some
+* Allow accounts to permissionlessly create nft collections.
+* Allow a named (permissioned) account to mint and burn unique items within a collection.
+* Move items between accounts permissionlessly.
+* Allow a named (permissioned) account to freeze and unfreeze items within a
+  collection or the entire collection.
+* Allow the owner of an item to delegate the ability to transfer the item to some
   named third-party.
+* Allow third-parties to store information in an NFT _without_ owning it (Eg. save game state).
 
 ## Interface
 
 ### Permissionless dispatchables
-* `create`: Create a new asset class by placing a deposit.
-* `transfer`: Transfer an asset instance to a new owner.
-* `redeposit`: Update the deposit amount of an asset instance, potentially freeing funds.
-* `approve_transfer`: Name a delegate who may authorise a transfer.
+
+* `create`: Create a new collection by placing a deposit.
+* `mint`: Mint a new item within a collection (when the minting is public).
+* `transfer`: Send an item to a new owner.
+* `redeposit`: Update the deposit amount of an item, potentially freeing funds.
+* `approve_transfer`: Name a delegate who may authorize a transfer.
 * `cancel_approval`: Revert the effects of a previous `approve_transfer`.
+* `approve_item_attributes`: Name a delegate who may change item's attributes within a namespace.
+* `cancel_item_attributes_approval`: Revert the effects of a previous `approve_item_attributes`.
+* `set_price`: Set the price for an item.
+* `buy_item`: Buy an item.
+* `pay_tips`: Pay tips, could be used for paying the creator royalties.
+* `create_swap`: Create an offer to swap an NFT for another NFT and optionally some fungibles.
+* `cancel_swap`: Cancel previously created swap offer.
+* `claim_swap`: Swap items in an atomic way.
+
 
 ### Permissioned dispatchables
-* `destroy`: Destroy an asset class.
-* `mint`: Mint a new asset instance within an asset class.
-* `burn`: Burn an asset instance within an asset class.
-* `freeze`: Prevent an individual asset from being transferred.
-* `thaw`: Revert the effects of a previous `freeze`.
-* `freeze_class`: Prevent all asset within a class from being transferred.
-* `thaw_class`: Revert the effects of a previous `freeze_class`.
-* `transfer_ownership`: Alter the owner of an asset class, moving all associated deposits.
-* `set_team`: Alter the permissioned accounts of an asset class.
+
+* `destroy`: Destroy a collection. This destroys all the items inside the collection and refunds the deposit.
+* `force_mint`: Mint a new item within a collection.
+* `burn`: Destroy an item within a collection.
+* `lock_item_transfer`: Prevent an individual item from being transferred.
+* `unlock_item_transfer`: Revert the effects of a previous `lock_item_transfer`.
+* `clear_all_transfer_approvals`: Clears all transfer approvals set by calling the `approve_transfer`.
+* `lock_collection`: Prevent all items within a collection from being transferred (making them all `soul bound`).
+* `lock_item_properties`: Lock item's metadata or attributes.
+* `transfer_ownership`: Alter the owner of a collection, moving all associated deposits. (Ownership of individual items will not be affected.)
+* `set_team`: Alter the permissioned accounts of a collection.
+* `set_collection_max_supply`: Change the max supply of a collection.
+* `update_mint_settings`: Update the minting settings for collection.
+
 
 ### Metadata (permissioned) dispatchables
-* `set_attribute`: Set a metadata attribute of an asset instance or class.
-* `clear_attribute`: Remove a metadata attribute of an asset instance or class.
-* `set_metadata`: Set general metadata of an asset instance.
-* `clear_metadata`: Remove general metadata of an asset instance.
-* `set_class_metadata`: Set general metadata of an asset class.
-* `clear_class_metadata`: Remove general metadata of an asset class.
+
+* `set_attribute`: Set a metadata attribute of an item or collection.
+* `clear_attribute`: Remove a metadata attribute of an item or collection.
+* `set_metadata`: Set general metadata of an item (E.g. an IPFS address of an image url).
+* `clear_metadata`: Remove general metadata of an item.
+* `set_collection_metadata`: Set general metadata of a collection.
+* `clear_collection_metadata`: Remove general metadata of a collection.
+
 
 ### Force (i.e. governance) dispatchables
-* `force_create`: Create a new asset class.
-* `force_asset_status`: Alter the underlying characteristics of an asset class.
 
-Please refer to the [`Call`](https://paritytech.github.io/substrate/master/pallet_uniques/pallet/enum.Call.html) enum
+* `force_create`: Create a new collection (the collection id can not be chosen).
+* `force_collection_owner`: Change collection's owner.
+* `force_collection_config`: Change collection's config.
+* `force_set_attribute`: Set an attribute.
+
+Please refer to the [`Call`](https://paritytech.github.io/substrate/master/pallet_nfts/pallet/enum.Call.html) enum
 and its associated variants for documentation on each function.
 
 ## Related Modules

frame/nfts/src/features/attributes.rs

@@ -304,4 +304,18 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		};
 		Ok(result)
 	}
+
+	/// A helper method to construct attribute's key.
+	pub fn construct_attribute_key(
+		key: Vec<u8>,
+	) -> Result<BoundedVec<u8, T::KeyLimit>, DispatchError> {
+		Ok(BoundedVec::try_from(key).map_err(|_| Error::<T, I>::IncorrectData)?)
+	}
+
+	/// A helper method to construct attribute's value.
+	pub fn construct_attribute_value(
+		value: Vec<u8>,
+	) -> Result<BoundedVec<u8, T::ValueLimit>, DispatchError> {
+		Ok(BoundedVec::try_from(value).map_err(|_| Error::<T, I>::IncorrectData)?)
+	}
 }

frame/nfts/src/impl_nonfungibles.rs

@@ -20,6 +20,7 @@
 use super::*;
 use frame_support::{
 	ensure,
+	storage::KeyPrefixIterator,
 	traits::{tokens::nonfungibles_v2::*, Get},
 	BoundedSlice,
 };
@@ -104,24 +105,28 @@ impl<T: Config<I>, I: 'static> Create<<T as SystemConfig>::AccountId, Collection
 {
 	/// Create a `collection` of nonfungible items to be owned by `who` and managed by `admin`.
 	fn create_collection(
-		collection: &Self::CollectionId,
 		who: &T::AccountId,
 		admin: &T::AccountId,
 		config: &CollectionConfigFor<T, I>,
-	) -> DispatchResult {
+	) -> Result<T::CollectionId, DispatchError> {
 		// DepositRequired can be disabled by calling the force_create() only
 		ensure!(
 			!config.has_disabled_setting(CollectionSetting::DepositRequired),
 			Error::<T, I>::WrongSetting
 		);
+
+		let collection =
+			NextCollectionId::<T, I>::get().unwrap_or(T::CollectionId::initial_value());
+
 		Self::do_create_collection(
-			*collection,
+			collection,
 			who.clone(),
 			admin.clone(),
 			*config,
 			T::CollectionDeposit::get(),
-			Event::Created { collection: *collection, creator: who.clone(), owner: admin.clone() },
-		)
+			Event::Created { collection, creator: who.clone(), owner: admin.clone() },
+		)?;
+		Ok(collection)
 	}
 }
 
@@ -186,25 +191,31 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
 }
 
 impl<T: Config<I>, I: 'static> InspectEnumerable<T::AccountId> for Pallet<T, I> {
+	type CollectionsIterator = KeyPrefixIterator<<T as Config<I>>::CollectionId>;
+	type ItemsIterator = KeyPrefixIterator<<T as Config<I>>::ItemId>;
+	type OwnedIterator =
+		KeyPrefixIterator<(<T as Config<I>>::CollectionId, <T as Config<I>>::ItemId)>;
+	type OwnedInCollectionIterator = KeyPrefixIterator<<T as Config<I>>::ItemId>;
+
 	/// Returns an iterator of the collections in existence.
 	///
 	/// NOTE: iterating this list invokes a storage read per item.
-	fn collections() -> Box<dyn Iterator<Item = Self::CollectionId>> {
-		Box::new(CollectionMetadataOf::<T, I>::iter_keys())
+	fn collections() -> Self::CollectionsIterator {
+		Collection::<T, I>::iter_keys()
 	}
 
 	/// Returns an iterator of the items of a `collection` in existence.
 	///
 	/// NOTE: iterating this list invokes a storage read per item.
-	fn items(collection: &Self::CollectionId) -> Box<dyn Iterator<Item = Self::ItemId>> {
-		Box::new(ItemMetadataOf::<T, I>::iter_key_prefix(collection))
+	fn items(collection: &Self::CollectionId) -> Self::ItemsIterator {
+		Item::<T, I>::iter_key_prefix(collection)
 	}
 
 	/// Returns an iterator of the items of all collections owned by `who`.
 	///
 	/// NOTE: iterating this list invokes a storage read per item.
-	fn owned(who: &T::AccountId) -> Box<dyn Iterator<Item = (Self::CollectionId, Self::ItemId)>> {
-		Box::new(Account::<T, I>::iter_key_prefix((who,)))
+	fn owned(who: &T::AccountId) -> Self::OwnedIterator {
+		Account::<T, I>::iter_key_prefix((who,))
 	}
 
 	/// Returns an iterator of the items of `collection` owned by `who`.
@@ -213,7 +224,7 @@ impl<T: Config<I>, I: 'static> InspectEnumerable<T::AccountId> for Pallet<T, I>
 	fn owned_in_collection(
 		collection: &Self::CollectionId,
 		who: &T::AccountId,
-	) -> Box<dyn Iterator<Item = Self::ItemId>> {
-		Box::new(Account::<T, I>::iter_key_prefix((who, collection)))
+	) -> Self::OwnedInCollectionIterator {
+		Account::<T, I>::iter_key_prefix((who, collection))
 	}
 }

frame/nfts/src/lib.rs

@@ -65,7 +65,7 @@ type AccountIdLookupOf<T> = <<T as SystemConfig>::Lookup as StaticLookup>::Sourc
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
-	use frame_support::{pallet_prelude::*, traits::ExistenceRequirement};
+	use frame_support::{pallet_prelude::*, traits::ExistenceRequirement, PalletId};
 	use frame_system::pallet_prelude::*;
 
 	#[pallet::pallet]
@@ -171,6 +171,10 @@ pub mod pallet {
 		#[pallet::constant]
 		type Features: Get<PalletFeatures>;
 
+		/// The pallet's id.
+		#[pallet::constant]
+		type PalletId: Get<PalletId>;
+
 		#[cfg(feature = "runtime-benchmarks")]
 		/// A set of helper functions for benchmarking.
 		type Helper: BenchmarkHelper<Self::CollectionId, Self::ItemId>;
@@ -583,6 +587,10 @@ pub mod pallet {
 		MintNotStated,
 		/// Mint has already ended.
 		MintEnded,
+		/// The provided Item was already used for claiming.
+		AlreadyClaimed,
+		/// The provided data is incorrect.
+		IncorrectData,
 	}
 
 	#[pallet::call]
@@ -756,16 +764,35 @@ pub mod pallet {
 							)
 						},
 						MintType::HolderOf(collection_id) => {
-							let correct_witness = match witness_data {
-								Some(MintWitness { owner_of_item }) =>
-									Account::<T, I>::contains_key((
-										&caller,
-										&collection_id,
-										&owner_of_item,
-									)),
-								None => false,
-							};
-							ensure!(correct_witness, Error::<T, I>::BadWitness)
+							let MintWitness { owner_of_item } =
+								witness_data.ok_or(Error::<T, I>::BadWitness)?;
+
+							let has_item = Account::<T, I>::contains_key((
+								&caller,
+								&collection_id,
+								&owner_of_item,
+							));
+							ensure!(has_item, Error::<T, I>::BadWitness);
+
+							let attribute_key = Self::construct_attribute_key(
+								PalletAttributes::<T::CollectionId>::UsedToClaim(collection)
+									.encode(),
+							)?;
+
+							let key = (
+								&collection_id,
+								Some(owner_of_item),
+								AttributeNamespace::Pallet(T::PalletId::get()),
+								&attribute_key,
+							);
+							let already_claimed = Attribute::<T, I>::contains_key(key.clone());
+							ensure!(!already_claimed, Error::<T, I>::AlreadyClaimed);
+
+							let value = Self::construct_attribute_value(vec![0])?;
+							Attribute::<T, I>::insert(
+								key,
+								(value, AttributeDeposit { account: None, amount: Zero::zero() }),
+							);
 						},
 						_ => {},
 					}

frame/nfts/src/mock.rs

@@ -23,6 +23,7 @@ use crate as pallet_nfts;
 use frame_support::{
 	construct_runtime, parameter_types,
 	traits::{AsEnsureOriginWithArg, ConstU32, ConstU64},
+	PalletId,
 };
 use sp_core::H256;
 use sp_runtime::{
@@ -86,6 +87,7 @@ impl pallet_balances::Config for Test {
 
 parameter_types! {
 	pub storage Features: PalletFeatures = PalletFeatures::all_enabled();
+	pub const NftsPalletId: PalletId = PalletId(*b"py/nfts_");
 }
 
 impl Config for Test {
@@ -110,6 +112,7 @@ impl Config for Test {
 	type MaxDeadlineDuration = ConstU64<10000>;
 	type Features = Features;
 	type WeightInfo = ();
+	type PalletId = NftsPalletId;
 	#[cfg(feature = "runtime-benchmarks")]
 	type Helper = ();
 }

frame/nfts/src/tests.rs

@@ -276,6 +276,12 @@ fn mint_should_work() {
 			42,
 			Some(MintWitness { owner_of_item: 43 })
 		));
+
+		// can't mint twice
+		assert_noop!(
+			Nfts::mint(RuntimeOrigin::signed(2), 1, 46, Some(MintWitness { owner_of_item: 43 })),
+			Error::<Test>::AlreadyClaimed
+		);
 	});
 }
 

frame/nfts/src/types.rs

@@ -291,6 +291,12 @@ pub struct CancelAttributesApprovalWitness {
 	pub account_attributes: u32,
 }
 
+#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
+pub enum PalletAttributes<CollectionId> {
+	/// Marks an item as being used in order to claim another item.
+	UsedToClaim(CollectionId),
+}
+
 #[derive(
 	Clone, Copy, Decode, Default, Encode, MaxEncodedLen, PartialEq, RuntimeDebug, TypeInfo,
 )]