Uniques: Allowing multiple approvals

bin/node/runtime/src/lib.rs

@@ -1453,6 +1453,7 @@ parameter_types! {
 	pub const ItemDeposit: Balance = 1 * DOLLARS;
 	pub const KeyLimit: u32 = 32;
 	pub const ValueLimit: u32 = 256;
+	pub const ApprovalsLimit: u32 = 20;
 }
 
 impl pallet_uniques::Config for Runtime {
@@ -1469,6 +1470,7 @@ impl pallet_uniques::Config for Runtime {
 	type StringLimit = StringLimit;
 	type KeyLimit = KeyLimit;
 	type ValueLimit = ValueLimit;
+	type ApprovalsLimit = ApprovalsLimit;
 	type WeightInfo = pallet_uniques::weights::SubstrateWeight<Runtime>;
 	#[cfg(feature = "runtime-benchmarks")]
 	type Helper = ();

frame/uniques/src/benchmarking.rs

@@ -380,7 +380,7 @@ benchmarks_instance_pallet! {
 		let delegate_lookup = T::Lookup::unlookup(delegate.clone());
 		let origin = SystemOrigin::Signed(caller.clone()).into();
 		Uniques::<T, I>::approve_transfer(origin, collection, item, delegate_lookup.clone())?;
-	}: _(SystemOrigin::Signed(caller.clone()), collection, item, Some(delegate_lookup))
+	}: _(SystemOrigin::Signed(caller.clone()), collection, item, delegate_lookup)
 	verify {
 		assert_last_event::<T, I>(Event::ApprovalCancelled { collection, item, owner: caller, delegate }.into());
 	}

frame/uniques/src/functions.rs

@@ -21,6 +21,7 @@ use super::*;
 use frame_support::{
 	ensure,
 	traits::{ExistenceRequirement, Get},
+	BoundedVec,
 };
 use sp_runtime::{DispatchError, DispatchResult};
 
@@ -47,12 +48,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Account::<T, I>::remove((&details.owner, &collection, &item));
 		Account::<T, I>::insert((&dest, &collection, &item), ());
 		let origin = details.owner;
+
 		details.owner = dest;
 
-		// The approved account has to be reset to None, because otherwise pre-approve attack would
-		// be possible, where the owner can approve his second account before making the transaction
-		// and then claiming the item back.
-		details.approved = None;
+		// The approved accounts have to be reset to None, because otherwise pre-approve attack
+		// would be possible, where the owner can approve his second account before making the
+		// transaction and then claiming the item back.
+		details.approved = BoundedVec::default();
 
 		Item::<T, I>::insert(&collection, &item, &details);
 		ItemPriceOf::<T, I>::remove(&collection, &item);
@@ -174,7 +176,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 
 				let owner = owner.clone();
 				Account::<T, I>::insert((&owner, &collection, &item), ());
-				let details = ItemDetails { owner, approved: None, is_frozen: false, deposit };
+				let details = ItemDetails {
+					owner,
+					approved: BoundedVec::default(),
+					is_frozen: false,
+					deposit,
+				};
 				Item::<T, I>::insert(&collection, &item, details);
 				Ok(())
 			},

frame/uniques/src/lib.rs

@@ -150,6 +150,10 @@ pub mod pallet {
 		#[pallet::constant]
 		type ValueLimit: Get<u32>;
 
+		/// The maximum approvals an item could have.
+		#[pallet::constant]
+		type ApprovalsLimit: Get<u32>;
+
 		#[cfg(feature = "runtime-benchmarks")]
 		/// A set of helper functions for benchmarking.
 		type Helper: BenchmarkHelper<Self::CollectionId, Self::ItemId>;
@@ -210,7 +214,7 @@ pub mod pallet {
 		T::CollectionId,
 		Blake2_128Concat,
 		T::ItemId,
-		ItemDetails<T::AccountId, DepositBalanceOf<T, I>>,
+		ItemDetails<T::AccountId, DepositBalanceOf<T, I>, T::ApprovalsLimit>,
 		OptionQuery,
 	>;
 
@@ -272,13 +276,26 @@ pub mod pallet {
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config<I>, I: 'static = ()> {
 		/// A `collection` was created.
-		Created { collection: T::CollectionId, creator: T::AccountId, owner: T::AccountId },
+		Created {
+			collection: T::CollectionId,
+			creator: T::AccountId,
+			owner: T::AccountId,
+		},
 		/// A `collection` was force-created.
-		ForceCreated { collection: T::CollectionId, owner: T::AccountId },
+		ForceCreated {
+			collection: T::CollectionId,
+			owner: T::AccountId,
+		},
 		/// A `collection` was destroyed.
-		Destroyed { collection: T::CollectionId },
+		Destroyed {
+			collection: T::CollectionId,
+		},
 		/// An `item` was issued.
-		Issued { collection: T::CollectionId, item: T::ItemId, owner: T::AccountId },
+		Issued {
+			collection: T::CollectionId,
+			item: T::ItemId,
+			owner: T::AccountId,
+		},
 		/// An `item` was transferred.
 		Transferred {
 			collection: T::CollectionId,
@@ -287,17 +304,34 @@ pub mod pallet {
 			to: T::AccountId,
 		},
 		/// An `item` was destroyed.
-		Burned { collection: T::CollectionId, item: T::ItemId, owner: T::AccountId },
+		Burned {
+			collection: T::CollectionId,
+			item: T::ItemId,
+			owner: T::AccountId,
+		},
 		/// Some `item` was frozen.
-		Frozen { collection: T::CollectionId, item: T::ItemId },
+		Frozen {
+			collection: T::CollectionId,
+			item: T::ItemId,
+		},
 		/// Some `item` was thawed.
-		Thawed { collection: T::CollectionId, item: T::ItemId },
+		Thawed {
+			collection: T::CollectionId,
+			item: T::ItemId,
+		},
 		/// Some `collection` was frozen.
-		CollectionFrozen { collection: T::CollectionId },
+		CollectionFrozen {
+			collection: T::CollectionId,
+		},
 		/// Some `collection` was thawed.
-		CollectionThawed { collection: T::CollectionId },
+		CollectionThawed {
+			collection: T::CollectionId,
+		},
 		/// The owner changed.
-		OwnerChanged { collection: T::CollectionId, new_owner: T::AccountId },
+		OwnerChanged {
+			collection: T::CollectionId,
+			new_owner: T::AccountId,
+		},
 		/// The management team changed.
 		TeamChanged {
 			collection: T::CollectionId,
@@ -321,16 +355,25 @@ pub mod pallet {
 			owner: T::AccountId,
 			delegate: T::AccountId,
 		},
+		AllApprovalsCancelled {
+			collection: T::CollectionId,
+			item: T::ItemId,
+			owner: T::AccountId,
+		},
 		/// A `collection` has had its attributes changed by the `Force` origin.
-		ItemStatusChanged { collection: T::CollectionId },
+		ItemStatusChanged {
+			collection: T::CollectionId,
+		},
 		/// New metadata has been set for a `collection`.
 		CollectionMetadataSet {
 			collection: T::CollectionId,
 			data: BoundedVec<u8, T::StringLimit>,
 			is_frozen: bool,
 		},
 		/// Metadata has been cleared for a `collection`.
-		CollectionMetadataCleared { collection: T::CollectionId },
+		CollectionMetadataCleared {
+			collection: T::CollectionId,
+		},
 		/// New metadata has been set for an item.
 		MetadataSet {
 			collection: T::CollectionId,
@@ -339,9 +382,15 @@ pub mod pallet {
 			is_frozen: bool,
 		},
 		/// Metadata has been cleared for an item.
-		MetadataCleared { collection: T::CollectionId, item: T::ItemId },
+		MetadataCleared {
+			collection: T::CollectionId,
+			item: T::ItemId,
+		},
 		/// Metadata has been cleared for an item.
-		Redeposited { collection: T::CollectionId, successful_items: Vec<T::ItemId> },
+		Redeposited {
+			collection: T::CollectionId,
+			successful_items: Vec<T::ItemId>,
+		},
 		/// New attribute metadata has been set for a `collection` or `item`.
 		AttributeSet {
 			collection: T::CollectionId,
@@ -356,9 +405,15 @@ pub mod pallet {
 			key: BoundedVec<u8, T::KeyLimit>,
 		},
 		/// Ownership acceptance has changed for an account.
-		OwnershipAcceptanceChanged { who: T::AccountId, maybe_collection: Option<T::CollectionId> },
+		OwnershipAcceptanceChanged {
+			who: T::AccountId,
+			maybe_collection: Option<T::CollectionId>,
+		},
 		/// Max supply has been set for a collection.
-		CollectionMaxSupplySet { collection: T::CollectionId, max_supply: u32 },
+		CollectionMaxSupplySet {
+			collection: T::CollectionId,
+			max_supply: u32,
+		},
 		/// The price was set for the instance.
 		ItemPriceSet {
 			collection: T::CollectionId,
@@ -367,7 +422,10 @@ pub mod pallet {
 			whitelisted_buyer: Option<T::AccountId>,
 		},
 		/// The price for the instance was removed.
-		ItemPriceRemoved { collection: T::CollectionId, item: T::ItemId },
+		ItemPriceRemoved {
+			collection: T::CollectionId,
+			item: T::ItemId,
+		},
 		/// An item was bought.
 		ItemBought {
 			collection: T::CollectionId,
@@ -394,8 +452,8 @@ pub mod pallet {
 		InUse,
 		/// The item or collection is frozen.
 		Frozen,
-		/// The delegate turned out to be different to what was expected.
-		WrongDelegate,
+		/// The provided account is not a delegate.
+		NotDelegate,
 		/// There is no delegate approved.
 		NoDelegate,
 		/// No approval exists that would allow the transfer.
@@ -416,6 +474,8 @@ pub mod pallet {
 		NotForSale,
 		/// The provided bid is too low.
 		BidTooLow,
+		/// The item has reached its approval limit.
+		ReachedApprovalLimit,
 	}
 
 	impl<T: Config<I>, I: 'static> Pallet<T, I> {
@@ -633,8 +693,7 @@ pub mod pallet {
 
 			Self::do_transfer(collection, item, dest, |collection_details, details| {
 				if details.owner != origin && collection_details.admin != origin {
-					let approved = details.approved.take().map_or(false, |i| i == origin);
-					ensure!(approved, Error::<T, I>::NoPermission);
+					ensure!(details.approved.contains(&origin), Error::<T, I>::NoPermission);
 				}
 				Ok(())
 			})
@@ -944,10 +1003,12 @@ pub mod pallet {
 				ensure!(permitted, Error::<T, I>::NoPermission);
 			}
 
-			details.approved = Some(delegate);
+			let _ = details
+				.approved
+				.try_push(delegate.clone())
+				.map_err(|_| Error::<T, I>::ReachedApprovalLimit)?;
 			Item::<T, I>::insert(&collection, &item, &details);
 
-			let delegate = details.approved.expect("set as Some above; qed");
 			Self::deposit_event(Event::ApprovedTransfer {
 				collection,
 				item,
@@ -979,12 +1040,14 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			collection: T::CollectionId,
 			item: T::ItemId,
-			maybe_check_delegate: Option<AccountIdLookupOf<T>>,
+			delegate: AccountIdLookupOf<T>,
 		) -> DispatchResult {
 			let maybe_check: Option<T::AccountId> = T::ForceOrigin::try_origin(origin)
 				.map(|_| None)
 				.or_else(|origin| ensure_signed(origin).map(Some).map_err(DispatchError::from))?;
 
+			let delegate = T::Lookup::lookup(delegate)?;
+
 			let collection_details =
 				Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
 			let mut details =
@@ -993,18 +1056,48 @@ pub mod pallet {
 				let permitted = check == collection_details.admin || check == details.owner;
 				ensure!(permitted, Error::<T, I>::NoPermission);
 			}
-			let maybe_check_delegate = maybe_check_delegate.map(T::Lookup::lookup).transpose()?;
-			let old = details.approved.take().ok_or(Error::<T, I>::NoDelegate)?;
-			if let Some(check_delegate) = maybe_check_delegate {
-				ensure!(check_delegate == old, Error::<T, I>::WrongDelegate);
-			}
 
+			ensure!(!details.approved.is_empty(), Error::<T, I>::NoDelegate);
+			ensure!(details.approved.contains(&delegate), Error::<T, I>::NotDelegate);
+
+			details.approved.retain(|d| *d != delegate);
 			Item::<T, I>::insert(&collection, &item, &details);
 			Self::deposit_event(Event::ApprovalCancelled {
 				collection,
 				item,
 				owner: details.owner,
-				delegate: old,
+				delegate,
+			});
+
+			Ok(())
+		}
+
+		#[pallet::weight(10_000)]
+		pub fn clear_all_transfer_approvals(
+			origin: OriginFor<T>,
+			collection: T::CollectionId,
+			item: T::ItemId,
+		) -> DispatchResult {
+			let maybe_check: Option<T::AccountId> = T::ForceOrigin::try_origin(origin)
+				.map(|_| None)
+				.or_else(|origin| ensure_signed(origin).map(Some).map_err(DispatchError::from))?;
+
+			let collection_details =
+				Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
+			let mut details =
+				Item::<T, I>::get(&collection, &item).ok_or(Error::<T, I>::UnknownCollection)?;
+			if let Some(check) = maybe_check {
+				let permitted = check == collection_details.admin || check == details.owner;
+				ensure!(permitted, Error::<T, I>::NoPermission);
+			}
+
+			// clear all approvals.
+			details.approved = BoundedVec::default();
+			Item::<T, I>::insert(&collection, &item, &details);
+			Self::deposit_event(Event::AllApprovalsCancelled {
+				collection,
+				item,
+				owner: details.owner,
 			});
 
 			Ok(())

frame/uniques/src/mock.rs

@@ -100,6 +100,7 @@ impl Config for Test {
 	type StringLimit = ConstU32<50>;
 	type KeyLimit = ConstU32<50>;
 	type ValueLimit = ConstU32<50>;
+	type ApprovalsLimit = ConstU32<10>;
 	type WeightInfo = ();
 	#[cfg(feature = "runtime-benchmarks")]
 	type Helper = ();