chore: revert admin permission on collection approvals

r0gue-io · Jan 10, 2025 · 3e2620a · 3e2620a
1 parent 5d350b1
commit 3e2620a
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 129 deletions.
diff --git a/pallets/nfts/src/lib.rs b/pallets/nfts/src/lib.rs
@@ -59,8 +59,8 @@ use codec::{Decode, Encode};
 use frame_support::{
 	dispatch::WithPostDispatchInfo,
 	traits::{
-		nonfungibles_v2::InspectRole, tokens::Locker, BalanceStatus::Reserved, Currency,
-		EnsureOriginWithArg, Incrementable, ReservableCurrency,
+		tokens::Locker, BalanceStatus::Reserved, Currency, EnsureOriginWithArg, Incrementable,
+		ReservableCurrency,
 	},
 };
 use frame_system::Config as SystemConfig;
@@ -2110,19 +2110,9 @@ pub mod pallet {
 			collection: T::CollectionId,
 			delegate: AccountIdLookupOf<T>,
 		) -> DispatchResult {
-			let maybe_check_origin = T::ForceOrigin::try_origin(origin)
-				.map(|_| None)
-				.or_else(|origin| ensure_signed(origin).map(Some).map_err(DispatchError::from))?;
+			T::ForceOrigin::ensure_origin(origin)?;
 			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)
 		}
 
@@ -2173,23 +2163,9 @@ pub mod pallet {
 			collection: T::CollectionId,
 			limit: u32,
 		) -> DispatchResultWithPostInfo {
-			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))
-					})
-				})?;
+			T::ForceOrigin::ensure_origin(origin)
+				.map_err(|e| 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())
 		}

diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs
@@ -4559,19 +4559,17 @@ fn force_clear_collection_approvals_works() {
 		let item_owner = account(1);
 		let delegates = 10..20;
 
-		assert_noop!(
-			Nfts::force_clear_collection_approvals(
-				RuntimeOrigin::signed(item_owner.clone()),
-				item_owner.clone(),
-				collection_id,
-				0
-			),
-			Error::<Test>::NoPermission.with_weight(WeightOf::clear_collection_approvals(0))
-		);
-		assert_noop!(
-			Nfts::force_clear_collection_approvals(none(), item_owner.clone(), collection_id, 0),
-			BadOrigin.with_weight(WeightOf::clear_collection_approvals(0))
-		);
+		for origin in [RuntimeOrigin::signed(item_owner.clone()), none()] {
+			assert_noop!(
+				Nfts::force_clear_collection_approvals(
+					origin,
+					item_owner.clone(),
+					collection_id,
+					0
+				),
+				BadOrigin.with_weight(WeightOf::clear_collection_approvals(0))
+			);
+		}
 		assert_ok!(Nfts::force_create(
 			RuntimeOrigin::root(),
 			item_owner.clone(),
@@ -5009,75 +5007,6 @@ fn cancel_collection_approval_works() {
 	});
 }
 
-#[test]
-fn force_cancel_collection_approval_works_with_admin() {
-	new_test_ext().execute_with(|| {
-		let collection_id = 0;
-		let item_id = 42;
-		let collection_owner = account(1);
-		let item_owner = account(2);
-		let delegate = account(3);
-		let admin = account(4);
-		let issuer = account(5);
-		let freezer = account(6);
-
-		Balances::make_free_balance_be(&item_owner, 100);
-		assert_ok!(Nfts::force_create(
-			RuntimeOrigin::root(),
-			collection_owner.clone(),
-			default_collection_config()
-		));
-		assert_ok!(Nfts::set_team(
-			RuntimeOrigin::signed(collection_owner.clone()),
-			collection_id,
-			Some(issuer.clone()),
-			Some(admin.clone()),
-			Some(freezer.clone())
-		));
-		assert_ok!(Nfts::force_mint(
-			RuntimeOrigin::signed(issuer.clone()),
-			collection_id,
-			item_id,
-			item_owner.clone(),
-			default_item_config()
-		));
-
-		// Successfully cancel a collection approval.
-		for origin in [issuer, admin] {
-			assert_ok!(Nfts::approve_collection_transfer(
-				RuntimeOrigin::signed(item_owner.clone()),
-				collection_id,
-				delegate.clone(),
-				None
-			));
-			assert_ok!(Nfts::force_cancel_collection_approval(
-				RuntimeOrigin::signed(origin.clone()),
-				item_owner.clone(),
-				collection_id,
-				delegate.clone()
-			));
-			assert_eq!(Balances::reserved_balance(&item_owner), 0);
-			assert!(!CollectionApprovals::contains_key((collection_id, &item_owner, &delegate)));
-		}
-
-		assert_ok!(Nfts::approve_collection_transfer(
-			RuntimeOrigin::signed(item_owner.clone()),
-			collection_id,
-			delegate.clone(),
-			None
-		));
-		assert_noop!(
-			Nfts::force_cancel_collection_approval(
-				RuntimeOrigin::signed(freezer),
-				item_owner,
-				collection_id,
-				delegate
-			),
-			Error::<Test>::NoPermission
-		);
-	});
-}
-
 #[test]
 fn force_cancel_collection_approval_works() {
 	new_test_ext().execute_with(|| {
@@ -5087,24 +5016,17 @@ fn force_cancel_collection_approval_works() {
 		let item_id = 42;
 		let item_owner = account(2);
 
-		assert_noop!(
-			Nfts::force_cancel_collection_approval(
-				RuntimeOrigin::signed(item_owner.clone()),
-				item_owner.clone(),
-				collection_id,
-				delegate.clone()
-			),
-			Error::<Test>::NoPermission
-		);
-		assert_noop!(
-			Nfts::force_cancel_collection_approval(
-				none(),
-				item_owner.clone(),
-				collection_id,
-				delegate.clone()
-			),
-			BadOrigin
-		);
+		for origin in [RuntimeOrigin::signed(item_owner.clone()), none()] {
+			assert_noop!(
+				Nfts::force_cancel_collection_approval(
+					origin,
+					item_owner.clone(),
+					collection_id,
+					delegate.clone()
+				),
+				BadOrigin
+			);
+		}
 		Balances::make_free_balance_be(&item_owner, 100);
 		assert_ok!(Nfts::force_create(
 			RuntimeOrigin::root(),