--- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -1311,12 +1311,14 @@ sender: T::CrossAccountId, token_id: TokenId, property: Vec, + nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo; fn delete_token_properties( &self, sender: T::CrossAccountId, token_id: TokenId, property_keys: Vec, + nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo; fn set_token_property_permissions( &self, @@ -1361,7 +1363,7 @@ sender: T::CrossAccountId, from: (CollectionId, TokenId), under: TokenId, - budget: &dyn Budget, + nesting_budget: &dyn Budget, ) -> DispatchResult; fn nest(&self, under: TokenId, to_nest: (CollectionId, TokenId)); --- a/pallets/fungible/src/common.rs +++ b/pallets/fungible/src/common.rs @@ -298,6 +298,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property: Vec, + _nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -315,6 +316,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property_keys: Vec, + _nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -324,7 +326,7 @@ _sender: ::CrossAccountId, _from: (CollectionId, TokenId), _under: TokenId, - _budget: &dyn Budget, + _nesting_budget: &dyn Budget, ) -> sp_runtime::DispatchResult { fail!(>::FungibleDisallowsNesting) } --- a/pallets/nonfungible/src/benchmarking.rs +++ b/pallets/nonfungible/src/benchmarking.rs @@ -183,7 +183,7 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, owner.clone())?; - }: {>::set_token_properties(&collection, &owner, item, props, false)?} + }: {>::set_token_properties(&collection, &owner, item, props, false, &Unlimited)?} delete_token_properties { let b in 0..MAX_PROPERTIES_PER_ITEM; @@ -205,7 +205,7 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, owner.clone())?; - >::set_token_properties(&collection, &owner, item, props, false)?; + >::set_token_properties(&collection, &owner, item, props, false, &Unlimited)?; let to_delete = (0..b).map(|k| property_key(k as usize)).collect::>(); - }: {>::delete_token_properties(&collection, &owner, item, to_delete)?} + }: {>::delete_token_properties(&collection, &owner, item, to_delete, &Unlimited)?} } --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -220,11 +220,19 @@ sender: T::CrossAccountId, token_id: TokenId, properties: Vec, + nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { let weight = >::set_token_properties(properties.len() as u32); with_weight( - >::set_token_properties(self, &sender, token_id, properties, false), + >::set_token_properties( + self, + &sender, + token_id, + properties.into_iter(), + false, + nesting_budget, + ), weight, ) } @@ -234,11 +242,18 @@ sender: T::CrossAccountId, token_id: TokenId, property_keys: Vec, + nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { let weight = >::delete_token_properties(property_keys.len() as u32); with_weight( - >::delete_token_properties(self, &sender, token_id, property_keys), + >::delete_token_properties( + self, + &sender, + token_id, + property_keys.into_iter(), + nesting_budget, + ), weight, ) } @@ -368,9 +383,9 @@ sender: T::CrossAccountId, from: (CollectionId, TokenId), under: TokenId, - budget: &dyn Budget, + nesting_budget: &dyn Budget, ) -> sp_runtime::DispatchResult { - >::check_nesting(self, sender, from, under, budget) + >::check_nesting(self, sender, from, under, nesting_budget) } fn nest(&self, under: TokenId, to_nest: (CollectionId, TokenId)) { --- a/pallets/nonfungible/src/erc.rs +++ b/pallets/nonfungible/src/erc.rs @@ -82,12 +82,16 @@ .map_err(|_| "key too long")?; let value = value.try_into().map_err(|_| "value too long")?; + let nesting_budget = self + .recorder + .weight_calls_budget(>::find_parent()); + >::set_token_property( self, &caller, TokenId(token_id), Property { key, value }, - false, + &nesting_budget, ) .map_err(dispatch_to_evm::) } @@ -99,7 +103,11 @@ .try_into() .map_err(|_| "key too long")?; - >::delete_token_property(self, &caller, TokenId(token_id), key) + let nesting_budget = self + .recorder + .weight_calls_budget(>::find_parent()); + + >::delete_token_property(self, &caller, TokenId(token_id), key, &nesting_budget) .map_err(dispatch_to_evm::) } --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -28,7 +28,8 @@ use up_data_structs::{ AccessMode, CollectionId, CustomDataLimit, TokenId, CreateCollectionData, CreateNftExData, mapping::TokenAddressMapping, budget::Budget, Property, PropertyPermission, PropertyKey, - PropertyKeyPermission, Properties, PropertyScope, TrySetProperty, TokenChild, AuxPropertyValue, + PropertyValue, PropertyKeyPermission, Properties, PropertyScope, TrySetProperty, TokenChild, + AuxPropertyValue, }; use pallet_evm::{account::CrossAccountId, Pallet as PalletEvm}; use pallet_common::{ @@ -480,139 +481,169 @@ }) } - pub fn set_token_property( + #[transactional] + fn modify_token_properties( collection: &NonfungibleHandle, sender: &T::CrossAccountId, token_id: TokenId, - property: Property, + properties: impl Iterator)>, is_token_create: bool, + nesting_budget: &dyn Budget, ) -> DispatchResult { - Self::check_token_change_permission( - collection, - sender, - token_id, - &property.key, - is_token_create, - )?; + let mut collection_admin_status = None; + let mut token_owner_result = None; - >::try_mutate((collection.id, token_id), |properties| { - let property = property.clone(); - properties.try_set(property.key, property.value) - }) - .map_err(>::from)?; + let mut is_collection_admin = + || *collection_admin_status.get_or_insert_with(|| collection.is_owner_or_admin(sender)); + + let mut is_token_owner = || { + *token_owner_result.get_or_insert_with(|| -> Result { + let is_owned = >::check_indirectly_owned( + sender.clone(), + collection.id, + token_id, + None, + nesting_budget, + )?; + + Ok(is_owned) + }) + }; - >::deposit_event(CommonEvent::TokenPropertySet( - collection.id, - token_id, - property.key, - )); + for (key, value) in properties { + let permission = >::property_permissions(collection.id) + .get(&key) + .cloned() + .unwrap_or_else(PropertyPermission::none); + + let is_property_exists = TokenProperties::::get((collection.id, token_id)) + .get(&key) + .is_some(); + match permission { + PropertyPermission { mutable: false, .. } if is_property_exists => { + return Err(>::NoPermission.into()); + } + + PropertyPermission { + collection_admin, + token_owner, + .. + } => { + //TODO: investigate threats during public minting. + if is_token_create && (collection_admin || token_owner) && value.is_some() { + // Pass + } else if collection_admin && is_collection_admin() { + // Pass + } else if token_owner && is_token_owner()? { + // Pass + } else { + fail!(>::NoPermission); + } + } + } + + match value { + Some(value) => { + >::try_mutate((collection.id, token_id), |properties| { + properties.try_set(key.clone(), value) + }) + .map_err(>::from)?; + + >::deposit_event(CommonEvent::TokenPropertySet( + collection.id, + token_id, + key, + )); + } + None => { + >::try_mutate((collection.id, token_id), |properties| { + properties.remove(&key) + }) + .map_err(>::from)?; + + >::deposit_event(CommonEvent::TokenPropertyDeleted( + collection.id, + token_id, + key, + )); + } + } + } + Ok(()) } - #[transactional] pub fn set_token_properties( collection: &NonfungibleHandle, sender: &T::CrossAccountId, token_id: TokenId, - properties: Vec, + properties: impl Iterator, is_token_create: bool, + nesting_budget: &dyn Budget, ) -> DispatchResult { - for property in properties { - Self::set_token_property(collection, sender, token_id, property, is_token_create)?; - } - - Ok(()) + Self::modify_token_properties( + collection, + sender, + token_id, + properties.map(|p| (p.key, Some(p.value))), + is_token_create, + nesting_budget, + ) } - pub fn delete_token_property( + pub fn set_token_property( collection: &NonfungibleHandle, sender: &T::CrossAccountId, token_id: TokenId, - property_key: PropertyKey, + property: Property, + nesting_budget: &dyn Budget, ) -> DispatchResult { - Self::check_token_change_permission(collection, sender, token_id, &property_key, false)?; - - >::try_mutate((collection.id, token_id), |properties| { - properties.remove(&property_key) - }) - .map_err(>::from)?; + let is_token_create = false; - >::deposit_event(CommonEvent::TokenPropertyDeleted( - collection.id, + Self::set_token_properties( + collection, + sender, token_id, - property_key, - )); - - Ok(()) + [property].into_iter(), + is_token_create, + nesting_budget, + ) } - fn check_token_change_permission( + pub fn delete_token_properties( collection: &NonfungibleHandle, sender: &T::CrossAccountId, token_id: TokenId, - property_key: &PropertyKey, - is_token_create: bool, + property_keys: impl Iterator, + nesting_budget: &dyn Budget, ) -> DispatchResult { - let permission = >::property_permissions(collection.id) - .get(property_key) - .cloned() - .unwrap_or_else(PropertyPermission::none); - - let token_data = >::get((collection.id, token_id)) - .ok_or(>::TokenNotFound)?; - - let check_token_owner = || -> DispatchResult { - ensure!(&token_data.owner == sender, >::NoPermission); - Ok(()) - }; - - let is_property_exists = TokenProperties::::get((collection.id, token_id)) - .get(property_key) - .is_some(); - - match permission { - PropertyPermission { mutable: false, .. } if is_property_exists => { - Err(>::NoPermission.into()) - } + let is_token_create = false; - PropertyPermission { - collection_admin, - token_owner, - .. - } => { - //TODO: investigate threats during public minting. - if is_token_create && (collection_admin || token_owner) { - return Ok(()); - } - - let mut check_result = Err(>::NoPermission.into()); - - if collection_admin { - check_result = collection.check_is_owner_or_admin(sender); - } - - if token_owner { - check_result.or_else(|_| check_token_owner()) - } else { - check_result - } - } - } + Self::modify_token_properties( + collection, + sender, + token_id, + property_keys.into_iter().map(|key| (key, None)), + is_token_create, + nesting_budget, + ) } - #[transactional] - pub fn delete_token_properties( + pub fn delete_token_property( collection: &NonfungibleHandle, sender: &T::CrossAccountId, token_id: TokenId, - property_keys: Vec, + property_key: PropertyKey, + nesting_budget: &dyn Budget, ) -> DispatchResult { - for key in property_keys { - Self::delete_token_property(collection, sender, token_id, key)?; - } - - Ok(()) + Self::delete_token_properties( + collection, + sender, + token_id, + [property_key].into_iter(), + nesting_budget, + ) } pub fn set_collection_properties( @@ -818,8 +849,9 @@ collection, sender, TokenId(token), - data.properties.clone().into_inner(), + data.properties.clone().into_iter(), true, + nesting_budget, ) { return TransactionOutcome::Rollback(Err(e)); } --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -314,6 +314,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property: Vec, + _nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -331,6 +332,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property_keys: Vec, + _nesting_budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -340,7 +342,7 @@ _sender: ::CrossAccountId, _from: (CollectionId, TokenId), _under: TokenId, - _budget: &dyn Budget, + _nesting_budget: &dyn Budget, ) -> sp_runtime::DispatchResult { fail!(>::RefungibleDisallowsNesting) } --- a/pallets/unique/src/lib.rs +++ b/pallets/unique/src/lib.rs @@ -654,8 +654,9 @@ ensure!(!properties.is_empty(), Error::::EmptyArgument); let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); + let budget = budget::Value::new(NESTING_BUDGET); - dispatch_tx::(collection_id, |d| d.set_token_properties(sender, token_id, properties)) + dispatch_tx::(collection_id, |d| d.set_token_properties(sender, token_id, properties, &budget)) } #[weight = T::CommonWeightInfo::delete_token_properties(property_keys.len() as u32)] @@ -669,8 +670,9 @@ ensure!(!property_keys.is_empty(), Error::::EmptyArgument); let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); + let budget = budget::Value::new(NESTING_BUDGET); - dispatch_tx::(collection_id, |d| d.delete_token_properties(sender, token_id, property_keys)) + dispatch_tx::(collection_id, |d| d.delete_token_properties(sender, token_id, property_keys, &budget)) } #[weight = T::CommonWeightInfo::set_token_property_permissions(property_permissions.len() as u32)] --- a/tests/src/nesting/properties.test.ts +++ b/tests/src/nesting/properties.test.ts @@ -3,11 +3,13 @@ import { addCollectionAdminExpectSuccess, createCollectionExpectSuccess, + setCollectionPermissionsExpectSuccess, createItemExpectSuccess, getCreateCollectionResult, transferExpectSuccess, } from '../util/helpers'; import {IKeyringPair} from '@polkadot/types/types'; +import {tokenIdToAddress} from '../eth/util/helpers'; let alice: IKeyringPair; let bob: IKeyringPair; @@ -522,6 +524,7 @@ describe('Integration Test: Token Properties', () => { let collection: number; let token: number; + let nestedToken: number; let permissions: {permission: any, signers: IKeyringPair[]}[]; before(async () => { @@ -544,7 +547,11 @@ beforeEach(async () => { await usingApi(async () => { collection = await createCollectionExpectSuccess(); + await setCollectionPermissionsExpectSuccess(alice, collection, {nesting: {tokenOwner: true}}); + token = await createItemExpectSuccess(alice, collection, 'NFT'); + nestedToken = await createItemExpectSuccess(alice, collection, 'NFT', {Ethereum: tokenIdToAddress(collection, token)}); + await addCollectionAdminExpectSuccess(alice, collection, bob.address); await transferExpectSuccess(collection, token, alice, charlie); }); @@ -681,6 +688,124 @@ expect((await api.query.nonfungible.tokenProperties(collection, token)).toJSON().consumedSpace).to.be.equal(0); }); }); + + it('Assigns properties to a nested token according to permissions', async () => { + await usingApi(async api => { + const propertyKeys: string[] = []; + let i = 0; + for (const permission of permissions) { + for (const signer of permission.signers) { + const key = i + '_' + signer.address; + propertyKeys.push(key); + + await expect(executeTransaction( + api, + alice, + api.tx.unique.setTokenPropertyPermissions(collection, [{key: key, permission: permission.permission}]), + ), `on setting permission ${i} by ${signer.address}`).to.not.be.rejected; + + await expect(executeTransaction( + api, + signer, + api.tx.unique.setTokenProperties(collection, nestedToken, [{key: key, value: 'Serotonin increase'}]), + ), `on adding property ${i} by ${signer.address}`).to.not.be.rejected; + } + + i++; + } + + const properties = (await api.rpc.unique.tokenProperties(collection, nestedToken, propertyKeys)).toHuman() as any[]; + const tokensData = (await api.rpc.unique.tokenData(collection, nestedToken, propertyKeys)).toHuman().properties as any[]; + for (let i = 0; i < properties.length; i++) { + expect(properties[i].value).to.be.equal('Serotonin increase'); + expect(tokensData[i].value).to.be.equal('Serotonin increase'); + } + }); + }); + + it('Changes properties of a nested token according to permissions', async () => { + await usingApi(async api => { + const propertyKeys: string[] = []; + let i = 0; + for (const permission of permissions) { + if (!permission.permission.mutable) continue; + + for (const signer of permission.signers) { + const key = i + '_' + signer.address; + propertyKeys.push(key); + + await expect(executeTransaction( + api, + alice, + api.tx.unique.setTokenPropertyPermissions(collection, [{key: key, permission: permission.permission}]), + ), `on setting permission ${i} by ${signer.address}`).to.not.be.rejected; + + await expect(executeTransaction( + api, + signer, + api.tx.unique.setTokenProperties(collection, nestedToken, [{key: key, value: 'Serotonin increase'}]), + ), `on adding property ${i} by ${signer.address}`).to.not.be.rejected; + + await expect(executeTransaction( + api, + signer, + api.tx.unique.setTokenProperties(collection, nestedToken, [{key: key, value: 'Serotonin stable'}]), + ), `on changing property ${i} by ${signer.address}`).to.not.be.rejected; + } + + i++; + } + + const properties = (await api.rpc.unique.tokenProperties(collection, nestedToken, propertyKeys)).toHuman() as any[]; + const tokensData = (await api.rpc.unique.tokenData(collection, nestedToken, propertyKeys)).toHuman().properties as any[]; + for (let i = 0; i < properties.length; i++) { + expect(properties[i].value).to.be.equal('Serotonin stable'); + expect(tokensData[i].value).to.be.equal('Serotonin stable'); + } + }); + }); + + it('Deletes properties of a nested token according to permissions', async () => { + await usingApi(async api => { + const propertyKeys: string[] = []; + let i = 0; + + for (const permission of permissions) { + if (!permission.permission.mutable) continue; + + for (const signer of permission.signers) { + const key = i + '_' + signer.address; + propertyKeys.push(key); + + await expect(executeTransaction( + api, + alice, + api.tx.unique.setTokenPropertyPermissions(collection, [{key: key, permission: permission.permission}]), + ), `on setting permission ${i} by ${signer.address}`).to.not.be.rejected; + + await expect(executeTransaction( + api, + signer, + api.tx.unique.setTokenProperties(collection, nestedToken, [{key: key, value: 'Serotonin increase'}]), + ), `on adding property ${i} by ${signer.address}`).to.not.be.rejected; + + await expect(executeTransaction( + api, + signer, + api.tx.unique.deleteTokenProperties(collection, nestedToken, [key]), + ), `on deleting property ${i} by ${signer.address}`).to.not.be.rejected; + } + + i++; + } + + const properties = (await api.rpc.unique.tokenProperties(collection, nestedToken, propertyKeys)).toJSON() as any[]; + expect(properties).to.be.empty; + const tokensData = (await api.rpc.unique.tokenData(collection, nestedToken, propertyKeys)).toJSON().properties as any[]; + expect(tokensData).to.be.empty; + expect((await api.query.nonfungible.tokenProperties(collection, nestedToken)).toJSON().consumedSpace).to.be.equal(0); + }); + }); }); describe('Negative Integration Test: Token Properties', () => { @@ -848,4 +973,4 @@ expect(propertiesMap.consumedSpace).to.be.equal(originalSpace); }); }); -}); \ No newline at end of file +});