feat: allow admin to force cancel approvals

pallets/nfts/src/features/approvals.rs

@@ -188,7 +188,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Ok(())
 	}
 
-	/// Approves the transfer of all collection items of `owner` to a delegate.
+	/// Approves the transfer of all collection items of `owner` to a `delegate`.
 	///
 	/// This function is used to approve the transfer of all (current and future) collection items
 	/// of `owner` to a `delegate`. The `delegate` account will be allowed to take control of the
@@ -203,7 +203,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// - `owner`: The owner of the collection items.
 	/// - `collection`: The identifier of the collection.
-	/// - `delegate`: The account that will be allowed to take control of the collection items.
+	/// - `delegate`: The account that will be approved to take control of the collection items.
 	/// - `maybe_deadline`: The optional deadline (in block numbers) specifying the time limit for
 	///   the approval.
 	pub(crate) fn do_approve_collection_transfer(
@@ -232,9 +232,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				let deposit_required = T::CollectionApprovalDeposit::get();
 				let current_deposit =
 					maybe_approval.take().map(|(_, deposit)| deposit).unwrap_or_default();
-				if current_deposit < deposit_required {
-					T::Currency::reserve(&owner, deposit_required - current_deposit)?;
-				}
+				T::Currency::reserve(&owner, deposit_required - current_deposit)?;
 				*maybe_approval = Some((deadline, deposit_required));
 				Ok(())
 			},
@@ -266,7 +264,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		delegate: T::AccountId,
 	) -> DispatchResult {
 		let (_, deposit) = CollectionApprovals::<T, I>::take((&collection, &owner, &delegate))
-			.ok_or(Error::<T, I>::Unapproved)?;
+			.ok_or(Error::<T, I>::NotDelegate)?;
 
 		T::Currency::unreserve(&owner, deposit);
 
@@ -312,12 +310,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Ok(removed_approvals)
 	}
 
-	/// Checks whether the `delegate` is approved to transfer collection items of `owner`.
+	/// Checks whether the `delegate` has permission to transfer collection items of `owner`.
 	///
 	/// - `collection`: The identifier of the collection.
 	/// - `owner`: The owner of the collection items.
-	/// - `delegate`: The account to check for approval to transfer collection items of `owner`.
-	fn check_collection_approval(
+	/// - `delegate`: The account to check for permission to transfer collection items of `owner`.
+	fn check_collection_approval_permission(
 		collection: &T::CollectionId,
 		owner: &T::AccountId,
 		delegate: &T::AccountId,
@@ -332,24 +330,26 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Ok(())
 	}
 
-	/// Checks whether the `delegate` is approved by `owner` to transfer its collection item(s). If
-	/// the `delegate` is approved for all `owner`'s collection items it can transfer every item
-	/// without requiring explicit approval for an item.
+	/// Checks whether the `delegate` has permission to transfer `owner`'s collection item(s).
+	/// If the `delegate` has permission to transfer all `owner`'s collection items, they can
+	/// transfer any item without needing explicit approval for each individual item.
 	///
 	/// - `collection`: The identifier of the collection.
-	/// - `maybe_item`: The optional item of the collection that the delegated account has an
-	///   approval to transfer. If not provided, an approval to transfer all `owner`'s collection
+	/// - `maybe_item`: The optional item of the collection that the delegated account has
+	///   permission to transfer. If not provided, permission to transfer all `owner`'s collection
 	///   items will be checked.
 	/// - `owner`: The owner of the specified collection item.
-	/// - `delegate`: The account to check for approval to transfer collection item(s) of owner.
-	pub fn check_approval(
+	/// - `delegate`: The account to check for permission to transfer collection item(s) from the
+	///   owner.
+	pub fn check_approval_permission(
 		collection: &T::CollectionId,
 		maybe_item: &Option<T::ItemId>,
 		owner: &T::AccountId,
 		delegate: &T::AccountId,
 	) -> DispatchResult {
 		// Check if `delegate` has permission to transfer `owner`'s collection items.
-		let Err(error) = Self::check_collection_approval(collection, owner, delegate) else {
+		let Err(error) = Self::check_collection_approval_permission(collection, owner, delegate)
+		else {
 			return Ok(());
 		};
 

pallets/nfts/src/lib.rs

@@ -59,8 +59,8 @@ use codec::{Decode, Encode};
 use frame_support::{
 	dispatch::WithPostDispatchInfo,
 	traits::{
-		tokens::Locker, BalanceStatus::Reserved, Currency, EnsureOriginWithArg, Incrementable,
-		ReservableCurrency,
+		nonfungibles_v2::InspectRole, tokens::Locker, BalanceStatus::Reserved, Currency,
+		EnsureOriginWithArg, Incrementable, ReservableCurrency,
 	},
 };
 use frame_system::Config as SystemConfig;
@@ -448,7 +448,13 @@ pub mod pallet {
 			NMapKey<Blake2_128Concat, T::AccountId>, // owner
 			NMapKey<Blake2_128Concat, T::AccountId>, // delegate
 		),
-		(Option<BlockNumberFor<T>>, DepositBalanceOf<T, I>),
+		(
+			// The optional deadline for the approval. If specified, the approval is valid on or
+			// before the given block number.
+			Option<BlockNumberFor<T>>,
+			// The balance to be deposited.
+			DepositBalanceOf<T, I>,
+		),
 	>;
 
 	#[pallet::event]
@@ -494,17 +500,19 @@ pub mod pallet {
 			admin: Option<T::AccountId>,
 			freezer: Option<T::AccountId>,
 		},
-		/// An `item` of a `collection` has been approved by the `owner` for transfer by
-		/// a `delegate`.
+		/// A provided `item` of a `collection`, or if no `item` is provided, all items in the
+		/// `collection` owned by the `owner` have been approved by the `owner` for transfer by a
+		/// `delegate`.
 		TransferApproved {
 			collection: T::CollectionId,
 			item: Option<T::ItemId>,
 			owner: T::AccountId,
 			delegate: T::AccountId,
 			deadline: Option<BlockNumberFor<T>>,
 		},
-		/// An approval for a `delegate` account to transfer a specific `item` in a `collection` or
-		/// all collection items owned by the `owner` has been cancelled by the owner.
+		/// An approval for a `delegate` account to transfer a specific `item` in a `collection`,
+		/// or if no `item` is provided, all collection items owned by the `owner` have been
+		/// cancelled by the `owner`.
 		ApprovalCancelled {
 			collection: T::CollectionId,
 			item: Option<T::ItemId>,
@@ -658,8 +666,6 @@ pub mod pallet {
 		NotDelegate,
 		/// The delegate turned out to be different to what was expected.
 		WrongDelegate,
-		/// No approval exists that would allow the transfer.
-		Unapproved,
 		/// The named owner has not signed ownership acceptance of the collection.
 		Unaccepted,
 		/// The item is locked (non-transferable).
@@ -1088,7 +1094,12 @@ pub mod pallet {
 
 			Self::do_transfer(collection, item, dest, |_, details| {
 				if details.owner != origin {
-					Self::check_approval(&collection, &Some(item), &details.owner, &origin)?;
+					Self::check_approval_permission(
+						&collection,
+						&Some(item),
+						&details.owner,
+						&origin,
+					)?;
 				}
 				Ok(())
 			})
@@ -2089,9 +2100,19 @@ pub mod pallet {
 			collection: T::CollectionId,
 			delegate: AccountIdLookupOf<T>,
 		) -> DispatchResult {
-			T::ForceOrigin::ensure_origin(origin)?;
+			let maybe_check_origin = 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 owner = T::Lookup::lookup(owner)?;
+
+			if let Some(check_origin) = maybe_check_origin {
+				ensure!(
+					Self::is_admin(&collection, &check_origin) ||
+						Self::is_issuer(&collection, &check_origin),
+					Error::<T, I>::NoPermission
+				);
+			}
 			Self::do_cancel_collection_approval(owner, collection, delegate)
 		}
 
@@ -2142,9 +2163,23 @@ pub mod pallet {
 			collection: T::CollectionId,
 			limit: u32,
 		) -> DispatchResultWithPostInfo {
-			T::ForceOrigin::ensure_origin(origin)
-				.map_err(|e| e.with_weight(T::WeightInfo::clear_collection_approvals(0)))?;
+			let maybe_check_origin =
+				T::ForceOrigin::try_origin(origin).map(|_| None).or_else(|origin| {
+					ensure_signed(origin).map(Some).map_err(|e| {
+						DispatchError::from(e)
+							.with_weight(T::WeightInfo::clear_collection_approvals(0))
+					})
+				})?;
 			let owner = T::Lookup::lookup(owner)?;
+
+			if let Some(check_origin) = maybe_check_origin {
+				ensure!(
+					Self::is_admin(&collection, &check_origin) ||
+						Self::is_issuer(&collection, &check_origin),
+					Error::<T, I>::NoPermission
+						.with_weight(T::WeightInfo::clear_collection_approvals(0))
+				);
+			}
 			let removed_approvals = Self::do_clear_collection_approvals(owner, collection, limit)?;
 			Ok(Some(T::WeightInfo::clear_collection_approvals(removed_approvals)).into())
 		}