Feat/resource removal

pallets/rmrk-core/src/functions.rs

@@ -92,6 +92,7 @@ where
 			thumb,
 			parts,
 			pending: root_owner != sender,
+			pending_removal: false,
 		};
 		Resources::<T>::insert((collection_id, nft_id, resource_id), res);
 
@@ -121,6 +122,58 @@ where
 		Self::deposit_event(Event::ResourceAccepted { nft_id, resource_id });
 		Ok(())
 	}
+
+	fn resource_remove(
+		sender: T::AccountId,
+		collection_id: CollectionId,
+		nft_id: NftId,
+		resource_id: BoundedResource<T::ResourceSymbolLimit>,
+	) -> DispatchResult {
+		let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
+		let collection = Self::collections(collection_id).ok_or(Error::<T>::CollectionUnknown)?;
+		ensure!(collection.issuer == sender, Error::<T>::NoPermission);
+		ensure!(Resources::<T>::contains_key((collection_id, nft_id, &resource_id)), Error::<T>::ResourceDoesntExist);
+
+		if root_owner == sender {
+			Resources::<T>::remove((collection_id, nft_id, resource_id));
+		} else {
+			Resources::<T>::try_mutate_exists(
+				(collection_id, nft_id, resource_id),
+				|resource| -> DispatchResult {
+					if let Some(res) = resource {
+						res.pending_removal = true;
+					}
+					Ok(())
+				},
+			)?;
+		}
+
+		Ok(())
+	}
+
+	fn accept_removal(
+		sender: T::AccountId,
+		collection_id: CollectionId,
+		nft_id: NftId,
+		resource_id: BoundedResource<T::ResourceSymbolLimit>,
+	) -> DispatchResult {
+		let (root_owner, _) = Pallet::<T>::lookup_root_owner(collection_id, nft_id)?;
+		ensure!(root_owner == sender, Error::<T>::NoPermission);
+		ensure!(Resources::<T>::contains_key((collection_id, nft_id, &resource_id)), Error::<T>::ResourceDoesntExist);
+
+		Resources::<T>::try_mutate_exists(
+			(collection_id, nft_id, resource_id),
+			|resource| -> DispatchResult {
+				if let Some(res) = resource {
+					ensure!(res.pending_removal, Error::<T>::ResourceNotPending);
+					*resource = None;
+				}
+				Ok(())
+			},
+		)?;
+
+		Ok(())
+	}
 }
 
 impl<T: Config> Collection<StringLimitOf<T>, BoundedCollectionSymbolOf<T>, T::AccountId> for Pallet<T>

pallets/rmrk-core/src/lib.rs

@@ -13,7 +13,7 @@ use sp_std::{convert::TryInto, vec::Vec};
 use rmrk_traits::{
 	primitives::*, AccountIdOrCollectionNftTuple, Collection, CollectionInfo, Nft, NftInfo,
 	Priority, Property, 
-	ResourceInfo, 
+	ResourceInfo,
 	Resource
 };
 use sp_std::result::Result;
@@ -221,6 +221,14 @@ pub mod pallet {
 			nft_id: NftId,
 			resource_id: BoundedResource<T::ResourceSymbolLimit>,
 		},
+		ResourceRemoval {
+			nft_id: NftId,
+			resource_id: BoundedResource<T::ResourceSymbolLimit>,
+		},
+		ResourceRemovalAccepted {
+			nft_id: NftId,
+			resource_id: BoundedResource<T::ResourceSymbolLimit>,
+		},
 		PrioritySet {
 			collection_id: CollectionId,
 			nft_id: NftId,
@@ -605,6 +613,50 @@ pub mod pallet {
 			Ok(())
 		}
 
+		/// remove resource
+		#[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))]
+		#[transactional]
+		pub fn remove_resource(
+			origin: OriginFor<T>,
+			collection_id: CollectionId,
+			nft_id: NftId,
+			resource_id: BoundedResource<T::ResourceSymbolLimit>,
+		) -> DispatchResult {
+			let sender = ensure_signed(origin.clone())?;
+
+			Self::resource_remove(
+				sender,
+				collection_id,
+				nft_id,
+				resource_id.clone()
+			)?;
+
+			Self::deposit_event(Event::ResourceRemoval { nft_id, resource_id });
+			Ok(())
+		}
+
+		/// accept the removal of a resource of an existing NFT
+		#[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))]
+		#[transactional]
+		pub fn accept_resource_removal(
+			origin: OriginFor<T>,
+			collection_id: CollectionId,
+			nft_id: NftId,
+			resource_id: BoundedResource<T::ResourceSymbolLimit>,
+		) -> DispatchResult {
+			let sender = ensure_signed(origin.clone())?;
+
+			Self::accept_removal(
+				sender,
+				collection_id,
+				nft_id,
+				resource_id.clone()
+			)?;
+
+			Self::deposit_event(Event::ResourceRemovalAccepted { nft_id, resource_id });
+			Ok(())
+		}
+
 		/// set a different order of resource priority
 		#[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))]
 		#[transactional]

pallets/rmrk-core/src/tests.rs

@@ -810,6 +810,135 @@ fn add_resource_pending_works() {
 	});
 }
 
+/// Resource: Basic resource removal
+#[test]
+fn resource_removal_works() {
+	ExtBuilder::default().build().execute_with(|| {
+		// Create a basic collection
+		assert_ok!(basic_collection());
+		// Mint NFT
+		assert_ok!(basic_mint());
+		// Add resource to NFT
+		assert_ok!(RMRKCore::add_resource(
+			Origin::signed(ALICE),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+			Some(0), // base_id
+			None, // src
+			None, // metadata
+			None, // slot
+			None, // license
+			None, // thumb
+			None, // parts
+		));
+		// Resource res-1 doesn't exist
+		assert_noop!(RMRKCore::remove_resource(
+			Origin::signed(ALICE),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-1"), // resource_id
+		), Error::<Test>::ResourceDoesntExist);
+		// Only collection issuer can request resource removal
+		assert_noop!(RMRKCore::remove_resource(
+			Origin::signed(BOB),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+		), Error::<Test>::NoPermission);
+		// Remove resource
+		assert_ok!(RMRKCore::remove_resource(
+			Origin::signed(ALICE),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+		));
+		// Successful resource removal should trigger ResourceRemoval event
+		System::assert_last_event(MockEvent::RmrkCore(crate::Event::ResourceRemoval {
+			nft_id: 0,
+			resource_id: stbr("res-0"), // resource_id
+		}));
+		// Since ALICE rootowns NFT, resource should be removed
+		assert_eq!(RMRKCore::resources((0, 0, stbr("res-0"))), None);
+	});
+}
+
+/// Resource: Resource removal with pending and accept
+#[test]
+fn resource_removal_pending_works() {
+	ExtBuilder::default().build().execute_with(|| {
+		// Create a basic collection
+		assert_ok!(basic_collection());
+		// Mint NFT
+		assert_ok!(RMRKCore::mint_nft(
+			Origin::signed(ALICE),
+			BOB,
+			COLLECTION_ID_0,
+			Some(BOB),
+			Some(Permill::from_float(1.525)),
+			bvec![0u8; 20],
+		));
+		// Add resource to NFT
+		assert_ok!(RMRKCore::add_resource(
+			Origin::signed(ALICE),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+			Some(0), // base_id
+			None, // src
+			None, // metadata
+			None, // slot
+			None, // license
+			None, // thumb
+			None, // parts
+		));
+		assert_ok!(RMRKCore::accept_resource(
+			Origin::signed(BOB),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"),
+		));
+		// Accepting a resource removal that is not pending should fail
+		assert_noop!(
+			RMRKCore::accept_resource_removal(Origin::signed(BOB), 0, 0, stbr("res-0")),
+			Error::<Test>::ResourceNotPending);
+		// Only collection's issuer can request resource removal
+		assert_noop!(RMRKCore::remove_resource(
+			Origin::signed(BOB),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+		), Error::<Test>::NoPermission);
+		// Resource removal requested by the collection issuer
+		assert_ok!(RMRKCore::remove_resource(
+			Origin::signed(ALICE),
+			COLLECTION_ID_0,
+			NFT_ID_0,
+			stbr("res-0"), // resource_id
+		));
+		// Since ALICE doesn't root-own NFT, resource's removal is waiting for acceptance
+		assert_eq!(RMRKCore::resources((0, 0, stbr("res-0"))).unwrap().pending_removal, true);
+		// ALICE doesn't own BOB's NFT, so accept should fail
+		assert_noop!(
+			RMRKCore::accept_resource_removal(Origin::signed(ALICE), 0, 0, stbr("res-0")),
+			Error::<Test>::NoPermission);
+		// Resource res-1 doesn't exist
+		assert_noop!(
+			RMRKCore::accept_resource_removal(Origin::signed(BOB), 0, 0, stbr("res-1")),
+			Error::<Test>::ResourceDoesntExist);
+		// BOB can accept his own NFT's pending resource removal
+		assert_ok!(RMRKCore::accept_resource_removal(Origin::signed(BOB), 0, 0, stbr("res-0")));
+		// Successful resource removal acceptance should trigger ResourceRemovalAccepted event
+		System::assert_last_event(MockEvent::RmrkCore(crate::Event::ResourceRemovalAccepted {
+			nft_id: 0,
+			resource_id: stbr("res-0"), // resource_id
+		}));
+		// Resource removed
+		assert_eq!(RMRKCore::resources((0, 0, stbr("res-0"))), None);
+	});
+}
+
+
 /// Property: Setting property tests (RMRK2.0 spec: SETPROPERTY)
 #[test]
 fn set_property_works() {

traits/src/resource.rs

@@ -20,6 +20,9 @@ pub struct ResourceInfo<BoundedResource, BoundedString> {
 	/// If resource is sent to non-rootowned NFT, pending will be false and need to be accepted
 	pub pending: bool,
 
+	/// If resource removal request is sent by non-rootowned NFT, pending will be true and need to be accepted
+	pub pending_removal: bool,
+
 	/// If a resource is composed, it will have an array of parts that compose it
 	pub parts: Option<Vec<PartId>>,
 
@@ -69,4 +72,16 @@ pub trait Resource<BoundedString, AccountId, BoundedResource> {
 		nft_id: NftId,
 		resource_id: BoundedResource,
 	) -> DispatchResult;
+	fn resource_remove(
+		sender: AccountId,
+		collection_id: CollectionId,
+		nft_id: NftId,
+		resource_id: BoundedResource,
+	) -> DispatchResult;
+	fn accept_removal(
+		sender: AccountId,
+		collection_id: CollectionId,
+		nft_id: NftId,
+		resource_id: BoundedResource,
+	) -> DispatchResult;
 }