From 625a6732c30a10491c85d29bc7bb90b8434e578f Mon Sep 17 00:00:00 2001 From: bmacer Date: Tue, 7 Dec 2021 10:37:15 -0500 Subject: [PATCH 1/4] implement send w nested ownership --- pallets/rmrk-core/src/lib.rs | 48 ++++++++++++++++----- pallets/rmrk-core/src/tests.rs | 76 ++++++++++++++++++++++++++++------ pallets/rmrk-core/src/types.rs | 15 ++++++- 3 files changed, 114 insertions(+), 25 deletions(-) diff --git a/pallets/rmrk-core/src/lib.rs b/pallets/rmrk-core/src/lib.rs index 679167d1..bba1abc5 100644 --- a/pallets/rmrk-core/src/lib.rs +++ b/pallets/rmrk-core/src/lib.rs @@ -9,7 +9,7 @@ use frame_system::ensure_signed; use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedAdd, One, StaticLookup, Zero}; use sp_std::{convert::TryInto, vec::Vec}; -use types::{ClassInfo, InstanceInfo}; +use types::{AccountIdOrCollectionNftTuple, ClassInfo, InstanceInfo}; #[cfg(test)] mod mock; @@ -23,6 +23,8 @@ pub type ClassInfoOf = ClassInfo pub type InstanceInfoOf = InstanceInfo< ::AccountId, BoundedVec::StringLimit>, + ::CollectionId, + ::NftId, >; pub mod types; @@ -112,7 +114,12 @@ pub mod pallet { NftMinted(T::AccountId, T::CollectionId, T::NftId), NFTBurned(T::AccountId, T::NftId), CollectionBurned(T::AccountId, T::CollectionId), - NFTSent(T::AccountId, T::AccountId, T::CollectionId, T::NftId), + NFTSent( + T::AccountId, + AccountIdOrCollectionNftTuple, + T::CollectionId, + T::NftId, + ), IssuerChanged(T::AccountId, T::AccountId, T::CollectionId), PropertySet( T::CollectionId, @@ -140,7 +147,7 @@ pub mod pallet { NoAvailableNftId, NotInRange, RoyaltyNotSet, - CollectionUnknown + CollectionUnknown, } #[pallet::call] @@ -158,6 +165,7 @@ pub mod pallet { #[transactional] pub fn mint_nft( origin: OriginFor, + owner: T::AccountId, collection_id: T::CollectionId, author: Option, royalty: Option, @@ -168,8 +176,7 @@ pub mod pallet { Err(origin) => Some(ensure_signed(origin)?), }; - let _ = Self::collections(collection_id) - .ok_or(Error::::CollectionUnknown)?; + let _ = Self::collections(collection_id).ok_or(Error::::CollectionUnknown)?; if let Some(r) = royalty { ensure!(r < 100, Error::::NotInRange); @@ -196,10 +203,13 @@ pub mod pallet { let author = author.ok_or(Error::::AuthorNotSet)?; let royalty = royalty.ok_or(Error::::RoyaltyNotSet)?; + let rootowner = owner.clone(); + let owner = AccountIdOrCollectionNftTuple::AccountId(owner.clone()); + NFTs::::insert( collection_id, nft_id, - InstanceInfo { author, royalty, metadata: metadata_bounded }, + InstanceInfo { owner, rootowner, author, royalty, metadata: metadata_bounded }, ); Self::deposit_event(Event::NftMinted( @@ -211,7 +221,7 @@ pub mod pallet { Ok(()) } - /// Mint a collection + /// Create a collection #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))] #[transactional] pub fn create_collection(origin: OriginFor, metadata: Vec) -> DispatchResult { @@ -289,14 +299,31 @@ pub mod pallet { origin: OriginFor, collection_id: T::CollectionId, nft_id: T::NftId, - dest: ::Source, + dest: AccountIdOrCollectionNftTuple, ) -> DispatchResult { let sender = match T::ProtocolOrigin::try_origin(origin) { Ok(_) => None, Err(origin) => Some(ensure_signed(origin)?), }; - let dest = T::Lookup::lookup(dest)?; - // TODO + + //TODO checks... + // Does NFT exist? + // Is sender the owner? + // If dest is tuple, does that NFT exist? + + let mut z = NFTs::::get(collection_id, nft_id).unwrap(); + // TODO if Alice sends to Bob's NFT, does Bob become the rootowner? + match dest.clone() { + AccountIdOrCollectionNftTuple::AccountId(account_id) => { + z.rootowner = account_id.clone(); + } + _ => (), + }; + z.owner = dest.clone(); + + NFTs::::remove(collection_id, nft_id); + NFTs::::insert(collection_id, nft_id, z); + Self::deposit_event(Event::NFTSent( sender.unwrap_or_default(), dest, @@ -379,7 +406,6 @@ pub mod pallet { Self::deposit_event(Event::ResourceAdded(nft_id, resource_id)); Ok(()) } - /// accept the addition of a new resource to an existing NFT #[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 bdd9bb42..a95ecfcb 100644 --- a/pallets/rmrk-core/src/tests.rs +++ b/pallets/rmrk-core/src/tests.rs @@ -1,4 +1,5 @@ use frame_support::{assert_noop, assert_ok, error::BadOrigin}; +// use sp_runtime::AccountId32; // use crate::types::ClassType; @@ -21,7 +22,7 @@ fn stv(s: &str) -> Vec { #[test] fn create_collection_works() { ExtBuilder::default().build().execute_with(|| { - let metadata = stv("testing"); + let metadata = stv("testing"); assert_ok!(RMRKCore::create_collection(Origin::signed(ALICE), metadata.clone())); assert_noop!( RMRKCore::create_collection( @@ -34,7 +35,7 @@ fn create_collection_works() { assert_noop!( RMRKCore::create_collection(Origin::signed(ALICE), metadata.clone()), Error::::NoAvailableCollectionId - ); + ); }); } @@ -44,6 +45,7 @@ fn mint_nft_works() { assert_ok!(RMRKCore::create_collection(Origin::signed(ALICE), b"metadata".to_vec())); assert_ok!(RMRKCore::mint_nft( Origin::signed(ALICE), + ALICE, 0, Some(ALICE), Some(0), @@ -51,27 +53,77 @@ fn mint_nft_works() { )); assert_ok!(RMRKCore::mint_nft( Origin::signed(ALICE), + ALICE, COLLECTION_ID_0, Some(ALICE), Some(20), Some(b"metadata".to_vec()) - )); - assert_ok!(RMRKCore::mint_nft( - Origin::signed(BOB), - COLLECTION_ID_0, - Some(CHARLIE), - Some(20), - Some(b"metadata".to_vec()) - )); + )); + assert_ok!(RMRKCore::mint_nft( + Origin::signed(BOB), + BOB, + COLLECTION_ID_0, + Some(CHARLIE), + Some(20), + Some(b"metadata".to_vec()) + )); assert_noop!( RMRKCore::mint_nft( Origin::signed(ALICE), + ALICE, NOT_EXISTING_CLASS_ID, Some(CHARLIE), Some(20), Some(b"metadata".to_vec()) ), Error::::CollectionUnknown - ); + ); + }); +} + +#[test] +fn send_nft_to_minted_nft_works() { + ExtBuilder::default().build().execute_with(|| { + let collection_metadata = stv("testing"); + let nft_metadata = stv("testing"); + assert_ok!(RMRKCore::create_collection(Origin::signed(ALICE), collection_metadata)); + // Alice mints NFT (0, 0) [will be the parent] + assert_ok!(RMRKCore::mint_nft( + Origin::signed(ALICE), + ALICE, + 0, + Some(ALICE), + Some(0), + Some(nft_metadata.clone()) + )); + // Alice mints NFT (0, 1) [will be the child] + assert_ok!(RMRKCore::mint_nft( + Origin::signed(ALICE), + ALICE, + 0, + Some(ALICE), + Some(0), + Some(nft_metadata) + )); + // Alice sends NFT (0, 0) [parent] to Bob + let x = RMRKCore::send( + Origin::signed(ALICE), + 0, + 0, + AccountIdOrCollectionNftTuple::AccountId(BOB), + ); + // Alice sends NFT (0, 1) [parent] to NFT (0, 0) [child] + let x = RMRKCore::send( + Origin::signed(ALICE), + 0, + 1, + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), + ); + + // Check that + assert_eq!( + RMRKCore::nfts(0, 1).unwrap().owner, + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), + ) }); -} \ No newline at end of file +} diff --git a/pallets/rmrk-core/src/types.rs b/pallets/rmrk-core/src/types.rs index 031f2c27..52647234 100644 --- a/pallets/rmrk-core/src/types.rs +++ b/pallets/rmrk-core/src/types.rs @@ -5,6 +5,13 @@ use serde::{Deserialize, Serialize}; use scale_info::TypeInfo; +#[derive(Encode, Decode, Eq, PartialEq, Copy, Clone, RuntimeDebug, TypeInfo)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub enum AccountIdOrCollectionNftTuple { + AccountId(AccountId), + CollectionAndNftTuple(CollectionId, NftId), +} + #[derive(Encode, Decode, Eq, Copy, PartialEq, Clone, RuntimeDebug, TypeInfo)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct ClassInfo { @@ -14,11 +21,15 @@ pub struct ClassInfo { #[derive(Encode, Decode, Eq, Copy, PartialEq, Clone, RuntimeDebug, TypeInfo)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] -pub struct InstanceInfo { +pub struct InstanceInfo { + /// The rootowner of the account, must be an account + pub rootowner: AccountId, + /// The owner of the NFT, can be either an Account or a tuple (CollectionId, NftId) + pub owner: AccountIdOrCollectionNftTuple, /// The user account which receives the royalty pub author: AccountId, /// Royalty in percent in range 0-99 pub royalty: u8, /// Arbitrary data about an instance, e.g. IPFS hash pub metadata: BoundedString, -} +} \ No newline at end of file From 60e8b4e9729a802ca8cdd840ba6a903bb24bfaa7 Mon Sep 17 00:00:00 2001 From: bmacer Date: Tue, 7 Dec 2021 10:57:29 -0500 Subject: [PATCH 2/4] update rootowner if sent isnt rootowned by recip --- pallets/rmrk-core/src/lib.rs | 12 +++++++++--- pallets/rmrk-core/src/tests.rs | 9 ++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pallets/rmrk-core/src/lib.rs b/pallets/rmrk-core/src/lib.rs index bba1abc5..86f03726 100644 --- a/pallets/rmrk-core/src/lib.rs +++ b/pallets/rmrk-core/src/lib.rs @@ -307,17 +307,23 @@ pub mod pallet { }; //TODO checks... - // Does NFT exist? + // Does sending NFT exist? + // Does recipient NFT exist? // Is sender the owner? // If dest is tuple, does that NFT exist? let mut z = NFTs::::get(collection_id, nft_id).unwrap(); - // TODO if Alice sends to Bob's NFT, does Bob become the rootowner? + match dest.clone() { AccountIdOrCollectionNftTuple::AccountId(account_id) => { z.rootowner = account_id.clone(); } - _ => (), + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(cid, nid) => { + let recipient_nft = NFTs::::get(cid, nid).unwrap(); + if z.rootowner != recipient_nft.rootowner { + z.rootowner = recipient_nft.rootowner + } + } }; z.owner = dest.clone(); diff --git a/pallets/rmrk-core/src/tests.rs b/pallets/rmrk-core/src/tests.rs index a95ecfcb..1daf9176 100644 --- a/pallets/rmrk-core/src/tests.rs +++ b/pallets/rmrk-core/src/tests.rs @@ -112,7 +112,7 @@ fn send_nft_to_minted_nft_works() { 0, AccountIdOrCollectionNftTuple::AccountId(BOB), ); - // Alice sends NFT (0, 1) [parent] to NFT (0, 0) [child] + // Alice sends NFT (0, 1) [child] to NFT (0, 0) [parent] let x = RMRKCore::send( Origin::signed(ALICE), 0, @@ -120,10 +120,13 @@ fn send_nft_to_minted_nft_works() { AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), ); - // Check that + // Check that NFT (0,1) [child] is owned by NFT (0,0) [parent] assert_eq!( RMRKCore::nfts(0, 1).unwrap().owner, AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), - ) + ); + + // Check that Bob now root-owns NFT (0, 1) [child] since he wasn't originally rootowner + assert_eq!(RMRKCore::nfts(0, 1).unwrap().rootowner, BOB) }); } From 297f30693cecde7755c6627030c83f2bb4efcf81 Mon Sep 17 00:00:00 2001 From: bmacer Date: Wed, 8 Dec 2021 08:35:09 -0500 Subject: [PATCH 3/4] variable renames --- pallets/rmrk-core/src/lib.rs | 18 +++++++++--------- pallets/rmrk-core/src/tests.rs | 8 ++++---- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pallets/rmrk-core/src/lib.rs b/pallets/rmrk-core/src/lib.rs index 86f03726..d8196ad8 100644 --- a/pallets/rmrk-core/src/lib.rs +++ b/pallets/rmrk-core/src/lib.rs @@ -299,7 +299,7 @@ pub mod pallet { origin: OriginFor, collection_id: T::CollectionId, nft_id: T::NftId, - dest: AccountIdOrCollectionNftTuple, + new_owner: AccountIdOrCollectionNftTuple, ) -> DispatchResult { let sender = match T::ProtocolOrigin::try_origin(origin) { Ok(_) => None, @@ -312,27 +312,27 @@ pub mod pallet { // Is sender the owner? // If dest is tuple, does that NFT exist? - let mut z = NFTs::::get(collection_id, nft_id).unwrap(); + let mut sending_nft = NFTs::::get(collection_id, nft_id).unwrap(); - match dest.clone() { + match new_owner.clone() { AccountIdOrCollectionNftTuple::AccountId(account_id) => { - z.rootowner = account_id.clone(); + sending_nft.rootowner = account_id.clone(); } AccountIdOrCollectionNftTuple::CollectionAndNftTuple(cid, nid) => { let recipient_nft = NFTs::::get(cid, nid).unwrap(); - if z.rootowner != recipient_nft.rootowner { - z.rootowner = recipient_nft.rootowner + if sending_nft.rootowner != recipient_nft.rootowner { + sending_nft.rootowner = recipient_nft.rootowner } } }; - z.owner = dest.clone(); + sending_nft.owner = new_owner.clone(); NFTs::::remove(collection_id, nft_id); - NFTs::::insert(collection_id, nft_id, z); + NFTs::::insert(collection_id, nft_id, sending_nft); Self::deposit_event(Event::NFTSent( sender.unwrap_or_default(), - dest, + new_owner, collection_id, nft_id, )); diff --git a/pallets/rmrk-core/src/tests.rs b/pallets/rmrk-core/src/tests.rs index 1daf9176..6a7c0c38 100644 --- a/pallets/rmrk-core/src/tests.rs +++ b/pallets/rmrk-core/src/tests.rs @@ -106,19 +106,19 @@ fn send_nft_to_minted_nft_works() { Some(nft_metadata) )); // Alice sends NFT (0, 0) [parent] to Bob - let x = RMRKCore::send( + assert_ok!(RMRKCore::send( Origin::signed(ALICE), 0, 0, AccountIdOrCollectionNftTuple::AccountId(BOB), - ); + )); // Alice sends NFT (0, 1) [child] to NFT (0, 0) [parent] - let x = RMRKCore::send( + assert_ok!(RMRKCore::send( Origin::signed(ALICE), 0, 1, AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), - ); + )); // Check that NFT (0,1) [child] is owned by NFT (0,0) [parent] assert_eq!( From db9b3b0ba33ec7f134ada06054c11336dd02c282 Mon Sep 17 00:00:00 2001 From: bmacer Date: Wed, 8 Dec 2021 08:52:43 -0500 Subject: [PATCH 4/4] adds existence tests --- pallets/rmrk-core/src/lib.rs | 19 ++++++++------- pallets/rmrk-core/src/tests.rs | 43 +++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/pallets/rmrk-core/src/lib.rs b/pallets/rmrk-core/src/lib.rs index d8196ad8..ac40b94f 100644 --- a/pallets/rmrk-core/src/lib.rs +++ b/pallets/rmrk-core/src/lib.rs @@ -148,6 +148,7 @@ pub mod pallet { NotInRange, RoyaltyNotSet, CollectionUnknown, + NoPermission, } #[pallet::call] @@ -292,7 +293,7 @@ pub mod pallet { Ok(()) } - /// transfer NFT from account A to account B + /// transfer NFT from account A to (account B or NFT) #[pallet::weight(10_000 + T::DbWeight::get().reads_writes(1,1))] #[transactional] pub fn send( @@ -306,20 +307,20 @@ pub mod pallet { Err(origin) => Some(ensure_signed(origin)?), }; - //TODO checks... - // Does sending NFT exist? - // Does recipient NFT exist? - // Is sender the owner? - // If dest is tuple, does that NFT exist? - - let mut sending_nft = NFTs::::get(collection_id, nft_id).unwrap(); + let mut sending_nft = + NFTs::::get(collection_id, nft_id).ok_or(Error::::NoAvailableNftId)?; + ensure!( + sending_nft.rootowner == sender.clone().unwrap_or_default(), + Error::::NoPermission + ); match new_owner.clone() { AccountIdOrCollectionNftTuple::AccountId(account_id) => { sending_nft.rootowner = account_id.clone(); } AccountIdOrCollectionNftTuple::CollectionAndNftTuple(cid, nid) => { - let recipient_nft = NFTs::::get(cid, nid).unwrap(); + let recipient_nft = + NFTs::::get(cid, nid).ok_or(Error::::NoAvailableNftId)?; if sending_nft.rootowner != recipient_nft.rootowner { sending_nft.rootowner = recipient_nft.rootowner } diff --git a/pallets/rmrk-core/src/tests.rs b/pallets/rmrk-core/src/tests.rs index 6a7c0c38..fea321ca 100644 --- a/pallets/rmrk-core/src/tests.rs +++ b/pallets/rmrk-core/src/tests.rs @@ -127,6 +127,47 @@ fn send_nft_to_minted_nft_works() { ); // Check that Bob now root-owns NFT (0, 1) [child] since he wasn't originally rootowner - assert_eq!(RMRKCore::nfts(0, 1).unwrap().rootowner, BOB) + assert_eq!(RMRKCore::nfts(0, 1).unwrap().rootowner, BOB); + + // Error if sender doesn't root-own sending NFT + assert_noop!( + RMRKCore::send( + Origin::signed(CHARLIE), + 0, + 0, + AccountIdOrCollectionNftTuple::AccountId(BOB) + ), + Error::::NoPermission + ); + + // Error if sending NFT doesn't exist + assert_noop!( + RMRKCore::send( + Origin::signed(ALICE), + 666, + 666, + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0) + ), + Error::::NoAvailableNftId + ); + + // BOB can send back child NFT to ALICE + assert_ok!(RMRKCore::send( + Origin::signed(BOB), + 0, + 1, + AccountIdOrCollectionNftTuple::AccountId(ALICE) + )); + + // Error if recipient is NFT and that NFT doesn't exist + assert_noop!( + RMRKCore::send( + Origin::signed(ALICE), + 0, + 1, + AccountIdOrCollectionNftTuple::CollectionAndNftTuple(666, 666) + ), + Error::::NoAvailableNftId + ); }); }