diff --git a/pallets/rmrk-core/src/functions.rs b/pallets/rmrk-core/src/functions.rs index 0e6c1942..d3f541f1 100644 --- a/pallets/rmrk-core/src/functions.rs +++ b/pallets/rmrk-core/src/functions.rs @@ -835,6 +835,12 @@ where Ok(()) } + /// Helper function for checking if an NFT exists + pub fn nft_exists(item: (CollectionId, NftId)) -> bool { + let (item_collection_id, item_nft_id) = item; + Nfts::::get(item_collection_id, item_nft_id).is_some() + } + // Check NFT is not equipped pub fn check_is_not_equipped(nft: &InstanceInfoOf) -> DispatchResult { ensure!(!nft.equipped, Error::::CannotSendEquippedItem); diff --git a/pallets/rmrk-equip/src/functions.rs b/pallets/rmrk-equip/src/functions.rs index b1693c53..78d88f4c 100644 --- a/pallets/rmrk-equip/src/functions.rs +++ b/pallets/rmrk-equip/src/functions.rs @@ -32,52 +32,53 @@ impl Pallet { }) } - pub fn iterate_part_types(base_id: BaseId) -> impl Iterator> { + /// Helper function for checking if an item is equipped + /// If the Equippings storage contains the Base/Slot for the Collection+NFT ID, the item is + /// already equipped + pub fn slot_is_equipped(item: (CollectionId, NftId), base_id: BaseId, slot_id: SlotId) -> bool { + let (equipper_collection_id, equipper_nft_id) = item; + Equippings::::get(((equipper_collection_id, equipper_nft_id), base_id, slot_id)) + .is_some() + } + + pub fn iterate_part_types(base_id: BaseId) -> impl Iterator> { Parts::::iter_prefix_values(base_id) } - pub fn iterate_theme_names(base_id: BaseId) -> impl Iterator> { - Themes::::iter_key_prefix((base_id,)) - .map(|(theme_name, ..)| theme_name) + pub fn iterate_theme_names(base_id: BaseId) -> impl Iterator> { + Themes::::iter_key_prefix((base_id,)).map(|(theme_name, ..)| theme_name) } pub fn get_theme( base_id: BaseId, theme_name: StringLimitOf, - filter_keys: Option>> + filter_keys: Option>>, ) -> Result>, Error> { - let properties: BoundedThemePropertiesOf = Self::query_theme_kv(base_id, &theme_name, filter_keys)?; + let properties: BoundedThemePropertiesOf = + Self::query_theme_kv(base_id, &theme_name, filter_keys)?; if properties.is_empty() { Ok(None) } else { - Ok(Some(BoundedThemeOf:: { - name: theme_name, - properties, - inherit: false, - })) + Ok(Some(BoundedThemeOf:: { name: theme_name, properties, inherit: false })) } } fn query_theme_kv( base_id: BaseId, theme_name: &StringLimitOf, - filter_keys: Option>> + filter_keys: Option>>, ) -> Result, Error> { BoundedVec::try_from( Themes::::iter_prefix((base_id, theme_name.clone())) .filter(|(key, _)| match &filter_keys { - Some (filter_keys) => filter_keys.contains(key), - None => true + Some(filter_keys) => filter_keys.contains(key), + None => true, }) - .map(|(key, value)| { - ThemeProperty { - key, - value, - } - }) - .collect::>() - ).or(Err(Error::::TooManyProperties)) + .map(|(key, value)| ThemeProperty { key, value }) + .collect::>(), + ) + .or(Err(Error::::TooManyProperties)) } } @@ -95,7 +96,7 @@ impl T::PartsLimit, >, BoundedVec, - BoundedVec>, T::MaxPropertiesPerTheme> + BoundedVec>, T::MaxPropertiesPerTheme>, > for Pallet where T: pallet_uniques::Config, @@ -122,7 +123,7 @@ where >, ) -> Result { let base_id = Self::get_next_base_id()?; - for part in parts.clone() { + for part in parts { match part.clone() { PartType::SlotPart(p) => { Parts::::insert(base_id, p.id, part); @@ -161,17 +162,14 @@ where /// Implementation of the do_equip function for the Base trait /// Called by the equip extrinsic to equip a child NFT's resource to a parent's slot, if all are - /// available. Also can be called to unequip, which can be successful if - /// - Item has beeen burned - /// - Item is equipped and extrinsic called by equipping item owner - /// - Item is equipped and extrinsic called by equipper NFT owner + /// available. /// Equipping operations are maintained inside the Equippings storage. /// Modeled after [equip interaction](https://github.com/rmrk-team/rmrk-spec/blob/master/standards/rmrk2.0.0/interactions/equip.md) /// /// Parameters: /// - issuer: The caller of the function, not necessarily anything else - /// - item: Child NFT being equipped (or unequipped) - /// - equipper: Parent NFT which will equip (or unequip) the item + /// - item: Child NFT being equipped + /// - equipper: Parent NFT which will equip the item /// - base_id: ID of the base which the item and equipper must each have a resource referencing /// - slot_id: ID of the slot which the item and equipper must each have a resource referencing fn do_equip( @@ -181,7 +179,7 @@ where resource_id: ResourceId, base_id: BaseId, slot_id: SlotId, - ) -> Result<(CollectionId, NftId, BaseId, SlotId, bool), DispatchError> { + ) -> Result<(CollectionId, NftId, BaseId, SlotId), DispatchError> { let item_collection_id = item.0; let item_nft_id = item.1; let equipper_collection_id = equipper.0; @@ -197,85 +195,44 @@ where pallet_uniques::Error::::Locked ); - let item_is_equipped = - Equippings::::get(((equipper_collection_id, equipper_nft_id), base_id, slot_id)) - .is_some(); - let item_exists = - pallet_rmrk_core::Pallet::::nfts(item_collection_id, item_nft_id).is_some(); - - // If item doesn't exist, anyone can unequip it. - if !item_exists && item_is_equipped { - // Remove from Equippings nft/base/slot storage - Equippings::::remove(((equipper_collection_id, equipper_nft_id), base_id, slot_id)); - - // Update item's equipped property - pallet_rmrk_core::Nfts::::try_mutate_exists( - item_collection_id, - item_nft_id, - |nft| -> DispatchResult { - if let Some(nft) = nft { - nft.equipped = false; - } - Ok(()) - }, - )?; - - // Return unequip event details - return Ok((item_collection_id, item_nft_id, base_id, slot_id, false)) - } - - let item_owner = - pallet_rmrk_core::Pallet::::lookup_root_owner(item_collection_id, item_nft_id)?; - let equipper_owner = pallet_rmrk_core::Pallet::::lookup_root_owner( - equipper_collection_id, - equipper_nft_id, - )?; - - // If the item is equipped in this slot, and either the equipper or the item owner is the - // caller, it will be unequipped - if item_is_equipped && (item_owner.0 == issuer || equipper_owner.0 == issuer) { - // Remove from Equippings nft/base/slot storage - Equippings::::remove(((equipper_collection_id, equipper_nft_id), base_id, slot_id)); - - // Update item's equipped property - pallet_rmrk_core::Nfts::::try_mutate_exists( - item_collection_id, - item_nft_id, - |nft| -> DispatchResult { - if let Some(nft) = nft { - nft.equipped = false; - } - Ok(()) - }, - )?; - - // Return unequip event details - return Ok((item_collection_id, item_nft_id, base_id, slot_id, false)) - } + // If the item's equipped property is true, it is already equipped + ensure!( + !pallet_rmrk_core::Pallet::::nfts(item_collection_id, item_nft_id) + .unwrap() + .equipped, + Error::::ItemAlreadyEquipped + ); - // Equipper NFT must exist + // If the Equippings storage contains the Base/Slot for the Collection+NFT ID, the item is + // already equipped ensure!( - pallet_rmrk_core::Pallet::::nfts(equipper_collection_id, equipper_nft_id).is_some(), - Error::::EquipperDoesntExist + !Self::slot_is_equipped((equipper_collection_id, equipper_nft_id), base_id, slot_id), + Error::::SlotAlreadyEquipped ); // Item must exist + let item_exists = + pallet_rmrk_core::Pallet::::nft_exists((item_collection_id, item_nft_id)); ensure!(item_exists, Error::::ItemDoesntExist); - // Is the item equipped anywhere? + // Equipper must exist ensure!( - !pallet_rmrk_core::Pallet::::nfts(item_collection_id, item_nft_id) - .unwrap() - .equipped, - Error::::AlreadyEquipped + pallet_rmrk_core::Pallet::::nfts(equipper_collection_id, equipper_nft_id).is_some(), + Error::::EquipperDoesntExist ); - // Caller must root-own equipper? - ensure!(equipper_owner.0 == issuer, Error::::PermissionError); - // Caller must root-own item + let item_owner = + pallet_rmrk_core::Pallet::::lookup_root_owner(item_collection_id, item_nft_id)?; ensure!(item_owner.0 == issuer, Error::::PermissionError); + // Caller must root-own equipper + let equipper_owner = pallet_rmrk_core::Pallet::::lookup_root_owner( + equipper_collection_id, + equipper_nft_id, + )?; + ensure!(equipper_owner.0 == issuer, Error::::PermissionError); + // Equipper must be direct parent of item let equipper_owner = pallet_rmrk_core::Pallet::::nfts(item_collection_id, item_nft_id) .unwrap() @@ -318,7 +275,7 @@ where // Part must exist ensure!(Self::parts(base_id, slot_id).is_some(), Error::::PartDoesntExist); - // Returns Result + // Check the PartType stored for this Base + Slot match Self::parts(base_id, slot_id).unwrap() { PartType::FixedPart(_) => { // Part must be SlotPart type @@ -352,11 +309,110 @@ where Ok(()) }, )?; - Ok((item_collection_id, item_nft_id, base_id, slot_id, true)) + Ok((item_collection_id, item_nft_id, base_id, slot_id)) }, } } + /// Implementation of the do_unequip function for the Base trait + /// Called by the equip extrinsic to unequip a child NFT's resource from a parent's slot, if it + /// is equipped. Unequip can be successful if + /// - Item has been burned + /// - Item is equipped and extrinsic called by equipping item owner + /// - Item is equipped and extrinsic called by equipper NFT owner + /// Equipping operations are maintained inside the Equippings storage. + /// Modeled after [equip interaction](https://github.com/rmrk-team/rmrk-spec/blob/master/standards/rmrk2.0.0/interactions/equip.md) + /// + /// Parameters: + /// - issuer: The caller of the function, not necessarily anything else + /// - item: Child NFT being equipped (or unequipped) + /// - equipper: Parent NFT which will equip (or unequip) the item + /// - base_id: ID of the equipped item's base + /// - slot_id: ID of the equipped item's slot + fn do_unequip( + issuer: T::AccountId, + item: (CollectionId, NftId), + equipper: (CollectionId, NftId), + base_id: BaseId, + slot_id: SlotId, + ) -> Result<(CollectionId, NftId, BaseId, SlotId), DispatchError> { + let item_collection_id = item.0; + let item_nft_id = item.1; + let equipper_collection_id = equipper.0; + let equipper_nft_id = equipper.1; + // Check item NFT lock status + ensure!( + !pallet_rmrk_core::Pallet::::is_locked(item_collection_id, item_nft_id), + pallet_uniques::Error::::Locked + ); + // Check equipper NFT lock status + ensure!( + !pallet_rmrk_core::Pallet::::is_locked(equipper_collection_id, equipper_nft_id), + pallet_uniques::Error::::Locked + ); + + ensure!( + Self::slot_is_equipped((equipper_collection_id, equipper_nft_id), base_id, slot_id), + Error::::SlotNotEquipped + ); + + // Check if the item already exists + let item_exists = + pallet_rmrk_core::Pallet::::nft_exists((item_collection_id, item_nft_id)); + + // If item doesn't exist, anyone can unequip it. This can happen because burn_nft can + // happen in rmrk-core, which doesn't know about rmrk-equip. + if !item_exists { + // Remove from Equippings nft/base/slot storage + Equippings::::remove(((equipper_collection_id, equipper_nft_id), base_id, slot_id)); + + // Update item's equipped property + pallet_rmrk_core::Nfts::::try_mutate_exists( + item_collection_id, + item_nft_id, + |nft| -> DispatchResult { + if let Some(nft) = nft { + nft.equipped = false; + } + Ok(()) + }, + )?; + + // Return unequip event details + return Ok((item_collection_id, item_nft_id, base_id, slot_id)) + } + + let item_owner = + pallet_rmrk_core::Pallet::::lookup_root_owner(item_collection_id, item_nft_id)?; + let equipper_owner = pallet_rmrk_core::Pallet::::lookup_root_owner( + equipper_collection_id, + equipper_nft_id, + )?; + + let issuer_owns_either_equipper_or_item = + item_owner.0 == issuer || equipper_owner.0 == issuer; + ensure!( + issuer_owns_either_equipper_or_item, + Error::::UnequipperMustOwnEitherItemOrEquipper + ); + + // Remove from Equippings nft/base/slot storage + Equippings::::remove(((equipper_collection_id, equipper_nft_id), base_id, slot_id)); + + // Update item's equipped property + pallet_rmrk_core::Nfts::::try_mutate_exists( + item_collection_id, + item_nft_id, + |nft| -> DispatchResult { + if let Some(nft) = nft { + nft.equipped = false; + } + Ok(()) + }, + )?; + Ok((item_collection_id, item_nft_id, base_id, slot_id)) + } + /// Implementation of the equippable function for the Base trait /// Called by the equippable extrinsic to update the array of Collections allowed /// to be equipped to a Base's specified Slot Part. @@ -416,7 +472,10 @@ where fn add_theme( issuer: T::AccountId, base_id: BaseId, - theme: BoundedThemeOf, + theme: Theme< + BoundedVec, + BoundedVec>, T::MaxPropertiesPerTheme>, + >, ) -> Result<(), DispatchError> { // Base must exist ensure!(Bases::::get(base_id).is_some(), Error::::BaseDoesntExist); diff --git a/pallets/rmrk-equip/src/lib.rs b/pallets/rmrk-equip/src/lib.rs index 4b250030..8100fee6 100644 --- a/pallets/rmrk-equip/src/lib.rs +++ b/pallets/rmrk-equip/src/lib.rs @@ -218,8 +218,12 @@ pub mod pallet { NoEquippableOnFixedPart, // No "default" Theme is defined, required prior to defining other themes NeedsDefaultThemeFirst, - // Equipped item cannot be equipped elsewhere (without first unequipping) - AlreadyEquipped, + // Item is trying to be equipped but is already equipped + ItemAlreadyEquipped, + // Slot to which an item is being equipped already has something equipped there + SlotAlreadyEquipped, + // Slot from which an item is being unequipped has nothing equipped there + SlotNotEquipped, // Error that should not occur // TODO is this being used? UnknownError, @@ -229,6 +233,10 @@ pub mod pallet { // Attempting to define more Properties than capacity allows // TODO confirm this is being used (after https://github.com/rmrk-team/rmrk-substrate/pull/95) TooManyProperties, + // Cannot unequip an item that isn't equipped yet + ItemNotEquipped, + // Cannot unequip an item when caller owns neither the item nor equipper + UnequipperMustOwnEitherItemOrEquipper, } #[pallet::call] @@ -265,17 +273,13 @@ pub mod pallet { Ok(()) } /// Equips a child NFT's resource to a parent's slot, if all are available. - /// Also can be called to unequip, which can be successful if - /// - Item has beeen burned - /// - Item is equipped and extrinsic called by equipping item owner - /// - Item is equipped and extrinsic called by equipper NFT owner /// Equipping operations are maintained inside the Equippings storage. /// Modeled after [equip interaction](https://github.com/rmrk-team/rmrk-spec/blob/master/standards/rmrk2.0.0/interactions/equip.md) /// /// Parameters: /// - origin: The caller of the function, not necessarily anything else - /// - item: Child NFT being equipped (or unequipped) - /// - equipper: Parent NFT which will equip (or unequip) the item + /// - item: Child NFT being equipped + /// - equipper: Parent NFT which will equip the item /// - base: ID of the base which the item and equipper must each have a resource referencing /// - slot: ID of the slot which the item and equipper must each have a resource referencing #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))] @@ -289,26 +293,54 @@ pub mod pallet { ) -> DispatchResult { let sender = ensure_signed(origin)?; - let (collection_id, nft_id, base_id, slot_id, equipped) = + let (collection_id, nft_id, base_id, slot_id) = Self::do_equip(sender, item, equipper, resource_id, base, slot)?; - if equipped { - // Send Equip event - Self::deposit_event(Event::SlotEquipped { - item_collection: collection_id, - item_nft: nft_id, - base_id, - slot_id, - }); - } else { - // Send Unequip event - Self::deposit_event(Event::SlotUnequipped { - item_collection: collection_id, - item_nft: nft_id, - base_id, - slot_id, - }); - } + // Send Equip event + Self::deposit_event(Event::SlotEquipped { + item_collection: collection_id, + item_nft: nft_id, + base_id, + slot_id, + }); + + Ok(()) + } + + /// Unequips a child NFT's resource from its parent's slot. + /// Can be successful if + /// - Item has beeen burned + /// - Item is equipped and extrinsic called by equipping item owner + /// - Item is equipped and extrinsic called by equipper NFT owner + /// Equipping operations are maintained inside the Equippings storage. + /// Modeled after [equip interaction](https://github.com/rmrk-team/rmrk-spec/blob/master/standards/rmrk2.0.0/interactions/equip.md) + /// + /// Parameters: + /// - origin: The caller of the function, not necessarily anything else + /// - item: Child NFT being unequipped + /// - unequipper: Parent NFT which will unequip the item + /// - base: ID of the base which the item and equipper must each have a resource referencing + /// - slot: ID of the slot which the item and equipper must each have a resource referencing + #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))] + pub fn unequip( + origin: OriginFor, + item: (CollectionId, NftId), + unequipper: (CollectionId, NftId), + base: BaseId, + slot: SlotId, + ) -> DispatchResult { + let sender = ensure_signed(origin)?; + + let (collection_id, nft_id, base_id, slot_id) = + Self::do_unequip(sender, item, unequipper, base, slot)?; + + Self::deposit_event(Event::SlotUnequipped { + item_collection: collection_id, + item_nft: nft_id, + base_id, + slot_id, + }); + Ok(()) } diff --git a/pallets/rmrk-equip/src/tests.rs b/pallets/rmrk-equip/src/tests.rs index 7dc3c5f8..73fe3bcf 100644 --- a/pallets/rmrk-equip/src/tests.rs +++ b/pallets/rmrk-equip/src/tests.rs @@ -240,6 +240,14 @@ fn equip_works() { AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), // Recipient )); + // Sends NFT (0, 2) [flashlight] to NFT (0, 0) [character-0] + assert_ok!(RmrkCore::send( + Origin::signed(ALICE), + 1, // Collection ID + 0, // NFT ID + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), // Recipient + )); + // Attempt to equip sword should fail as character-0 doesn't have a resource that is // associated with this base assert_noop!( @@ -304,7 +312,24 @@ fn equip_works() { sword_slot_resource_left )); - // Equipping should now work + let flashlight_slot_resource_left = SlotResource { + src: Some(stbd("ipfs://flashlight-metadata-left")), + base: 0, // BaseID + license: None, + metadata: None, + slot: 201, // SlotID + thumb: None, + }; + + // Add our flashlight left-hand resource to our flashlight NFT + assert_ok!(RmrkCore::add_slot_resource( + Origin::signed(ALICE), + 1, // collection id + 1, // nft id + flashlight_slot_resource_left + )); + + // Equipping sword should now work assert_ok!(RmrkEquip::equip( Origin::signed(ALICE), // Signer (1, 0), // item @@ -321,10 +346,23 @@ fn equip_works() { slot_id: 201, })); + // Equipping flashlight to left-hand should fail (SlotAlreadyEquipped) + assert_noop!( + RmrkEquip::equip( + Origin::signed(ALICE), // Signer + (1, 1), // item + (0, 0), // equipper + 0, // ResourceId, + 0, // BaseId + 201, // SlotId + ), + Error::::SlotAlreadyEquipped + ); + // Equipped resource ID 0 should now be associated with equippings for character-0 // on base 0, slot 201 let equipped = RmrkEquip::equippings(((0, 0), 0, 201)); - assert_eq!(equipped.clone().unwrap(), 0,); + assert_eq!(equipped.unwrap(), 0,); // Resource for equipped item should exist assert!(RmrkCore::resources((1, 0, equipped.unwrap())).is_some()); @@ -356,18 +394,29 @@ fn equip_works() { 0, // BaseId 202, // SlotId ), - Error::::AlreadyEquipped + Error::::ItemAlreadyEquipped ); - // Unequipping from left-hand should work - assert_ok!(RmrkEquip::equip( + // Equipping to left-hand should fail (ItemAlreadyEquipped) + assert_noop!( + RmrkEquip::equip( + Origin::signed(ALICE), // Signer + (1, 0), // item + (0, 0), // equipper + 0, // ResourceId + 0, // BaseId + 201, // SlotId + ), + Error::::ItemAlreadyEquipped + ); + + assert_ok!(RmrkEquip::unequip( Origin::signed(ALICE), // Signer (1, 0), // item (0, 0), // equipper - 0, // ResourceId 0, // BaseId 201, // SlotId - )); + ),); System::assert_last_event(MockEvent::RmrkEquip(crate::Event::SlotUnequipped { item_collection: 1, @@ -386,16 +435,39 @@ fn equip_works() { 201, // SlotId )); + // CHARLIE can't unequip ALICE's item + assert_noop!( + RmrkEquip::unequip( + Origin::signed(CHARLIE), // Signer + (1, 0), // item + (0, 0), // unequipper + 0, // BaseId + 201, // SlotId + ), + Error::::UnequipperMustOwnEitherItemOrEquipper + ); + // Unequipping from left-hand should work - assert_ok!(RmrkEquip::equip( + assert_ok!(RmrkEquip::unequip( Origin::signed(ALICE), // Signer (1, 0), // item (0, 0), // equipper - 0, // ResourceId 0, // BaseId 201, // SlotId )); + // Unequipping again should fail since it is no longer equipped + assert_noop!( + RmrkEquip::unequip( + Origin::signed(ALICE), // Signer + (1, 0), // item + (0, 0), // equipper + 0, // BaseId + 201, // SlotId + ), + Error::::SlotNotEquipped + ); + // Equipping to right-hand should work assert_ok!(RmrkEquip::equip( Origin::signed(ALICE), // Signer diff --git a/traits/src/base.rs b/traits/src/base.rs index 74d5bae0..72e5e94a 100644 --- a/traits/src/base.rs +++ b/traits/src/base.rs @@ -44,7 +44,16 @@ pub struct BaseInfo { } // Abstraction over a Base system. -pub trait Base { +pub trait Base< + AccountId, + CollectionId, + NftId, + BoundedString, + BoundedParts, + BoundedCollectionList, + BoundedThemeProperties, +> +{ fn base_create( issuer: AccountId, base_type: BoundedString, @@ -62,7 +71,14 @@ pub trait Base Result<(CollectionId, NftId, BaseId, SlotId, bool), DispatchError>; + ) -> Result<(CollectionId, NftId, BaseId, SlotId), DispatchError>; + fn do_unequip( + issuer: AccountId, // Maybe don't need? + item: (CollectionId, NftId), + equipper: (CollectionId, NftId), + base_id: BaseId, // Maybe BaseId ? + slot: SlotId, // Maybe SlotId ? + ) -> Result<(CollectionId, NftId, BaseId, SlotId), DispatchError>; fn do_equippable( issuer: AccountId, base_id: BaseId,