diff --git a/pallets/rmrk-core/src/functions.rs b/pallets/rmrk-core/src/functions.rs index 687740df..fe7bc769 100644 --- a/pallets/rmrk-core/src/functions.rs +++ b/pallets/rmrk-core/src/functions.rs @@ -92,6 +92,7 @@ where thumb, parts, pending: root_owner != sender, + pending_removal: false, }; Resources::::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, + ) -> DispatchResult { + let (root_owner, _) = Pallet::::lookup_root_owner(collection_id, nft_id)?; + let collection = Self::collections(collection_id).ok_or(Error::::CollectionUnknown)?; + ensure!(collection.issuer == sender, Error::::NoPermission); + ensure!(Resources::::contains_key((collection_id, nft_id, &resource_id)), Error::::ResourceDoesntExist); + + if root_owner == sender { + Resources::::remove((collection_id, nft_id, resource_id)); + } else { + Resources::::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, + ) -> DispatchResult { + let (root_owner, _) = Pallet::::lookup_root_owner(collection_id, nft_id)?; + ensure!(root_owner == sender, Error::::NoPermission); + ensure!(Resources::::contains_key((collection_id, nft_id, &resource_id)), Error::::ResourceDoesntExist); + + Resources::::try_mutate_exists( + (collection_id, nft_id, resource_id), + |resource| -> DispatchResult { + if let Some(res) = resource { + ensure!(res.pending_removal, Error::::ResourceNotPending); + *resource = None; + } + Ok(()) + }, + )?; + + Ok(()) + } } impl Collection, BoundedCollectionSymbolOf, T::AccountId> for Pallet diff --git a/pallets/rmrk-core/src/lib.rs b/pallets/rmrk-core/src/lib.rs index 306010b9..b54e0f99 100644 --- a/pallets/rmrk-core/src/lib.rs +++ b/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, }, + ResourceRemoval { + nft_id: NftId, + resource_id: BoundedResource, + }, + ResourceRemovalAccepted { + nft_id: NftId, + resource_id: BoundedResource, + }, 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, + collection_id: CollectionId, + nft_id: NftId, + resource_id: BoundedResource, + ) -> 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, + collection_id: CollectionId, + nft_id: NftId, + resource_id: BoundedResource, + ) -> 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] diff --git a/pallets/rmrk-core/src/tests.rs b/pallets/rmrk-core/src/tests.rs index 2a2ea961..4607fbd7 100644 --- a/pallets/rmrk-core/src/tests.rs +++ b/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::::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::::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::::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::::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::::NoPermission); + // Resource res-1 doesn't exist + assert_noop!( + RMRKCore::accept_resource_removal(Origin::signed(BOB), 0, 0, stbr("res-1")), + Error::::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() { diff --git a/traits/src/resource.rs b/traits/src/resource.rs index ca638c0d..272c6247 100644 --- a/traits/src/resource.rs +++ b/traits/src/resource.rs @@ -20,6 +20,9 @@ pub struct ResourceInfo { /// 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>, @@ -69,4 +72,16 @@ pub trait Resource { 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; }