difftreelog
Merge pull request #414 from UniqueNetwork/feature/prop-check-root-owner
in: master
Feature/prop check root owner
9 files changed
pallets/common/src/lib.rsdiffbeforeafterboth--- a/pallets/common/src/lib.rs
+++ b/pallets/common/src/lib.rs
@@ -1311,12 +1311,14 @@
sender: T::CrossAccountId,
token_id: TokenId,
property: Vec<Property>,
+ nesting_budget: &dyn Budget,
) -> DispatchResultWithPostInfo;
fn delete_token_properties(
&self,
sender: T::CrossAccountId,
token_id: TokenId,
property_keys: Vec<PropertyKey>,
+ 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));
pallets/fungible/src/common.rsdiffbeforeafterboth--- a/pallets/fungible/src/common.rs
+++ b/pallets/fungible/src/common.rs
@@ -298,6 +298,7 @@
_sender: T::CrossAccountId,
_token_id: TokenId,
_property: Vec<Property>,
+ _nesting_budget: &dyn Budget,
) -> DispatchResultWithPostInfo {
fail!(<Error<T>>::SettingPropertiesNotAllowed)
}
@@ -315,6 +316,7 @@
_sender: T::CrossAccountId,
_token_id: TokenId,
_property_keys: Vec<PropertyKey>,
+ _nesting_budget: &dyn Budget,
) -> DispatchResultWithPostInfo {
fail!(<Error<T>>::SettingPropertiesNotAllowed)
}
@@ -324,7 +326,7 @@
_sender: <T>::CrossAccountId,
_from: (CollectionId, TokenId),
_under: TokenId,
- _budget: &dyn Budget,
+ _nesting_budget: &dyn Budget,
) -> sp_runtime::DispatchResult {
fail!(<Error<T>>::FungibleDisallowsNesting)
}
pallets/nonfungible/src/benchmarking.rsdiffbeforeafterboth--- a/pallets/nonfungible/src/benchmarking.rs
+++ b/pallets/nonfungible/src/benchmarking.rs
@@ -183,7 +183,7 @@
value: property_value(),
}).collect::<Vec<_>>();
let item = create_max_item(&collection, &owner, owner.clone())?;
- }: {<Pallet<T>>::set_token_properties(&collection, &owner, item, props, false)?}
+ }: {<Pallet<T>>::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::<Vec<_>>();
let item = create_max_item(&collection, &owner, owner.clone())?;
- <Pallet<T>>::set_token_properties(&collection, &owner, item, props, false)?;
+ <Pallet<T>>::set_token_properties(&collection, &owner, item, props, false, &Unlimited)?;
let to_delete = (0..b).map(|k| property_key(k as usize)).collect::<Vec<_>>();
- }: {<Pallet<T>>::delete_token_properties(&collection, &owner, item, to_delete)?}
+ }: {<Pallet<T>>::delete_token_properties(&collection, &owner, item, to_delete, &Unlimited)?}
}
pallets/nonfungible/src/common.rsdiffbeforeafterboth--- a/pallets/nonfungible/src/common.rs
+++ b/pallets/nonfungible/src/common.rs
@@ -220,11 +220,19 @@
sender: T::CrossAccountId,
token_id: TokenId,
properties: Vec<Property>,
+ nesting_budget: &dyn Budget,
) -> DispatchResultWithPostInfo {
let weight = <CommonWeights<T>>::set_token_properties(properties.len() as u32);
with_weight(
- <Pallet<T>>::set_token_properties(self, &sender, token_id, properties, false),
+ <Pallet<T>>::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<PropertyKey>,
+ nesting_budget: &dyn Budget,
) -> DispatchResultWithPostInfo {
let weight = <CommonWeights<T>>::delete_token_properties(property_keys.len() as u32);
with_weight(
- <Pallet<T>>::delete_token_properties(self, &sender, token_id, property_keys),
+ <Pallet<T>>::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 {
- <Pallet<T>>::check_nesting(self, sender, from, under, budget)
+ <Pallet<T>>::check_nesting(self, sender, from, under, nesting_budget)
}
fn nest(&self, under: TokenId, to_nest: (CollectionId, TokenId)) {
pallets/nonfungible/src/erc.rsdiffbeforeafterboth--- 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(<StructureWeight<T>>::find_parent());
+
<Pallet<T>>::set_token_property(
self,
&caller,
TokenId(token_id),
Property { key, value },
- false,
+ &nesting_budget,
)
.map_err(dispatch_to_evm::<T>)
}
@@ -99,7 +103,11 @@
.try_into()
.map_err(|_| "key too long")?;
- <Pallet<T>>::delete_token_property(self, &caller, TokenId(token_id), key)
+ let nesting_budget = self
+ .recorder
+ .weight_calls_budget(<StructureWeight<T>>::find_parent());
+
+ <Pallet<T>>::delete_token_property(self, &caller, TokenId(token_id), key, &nesting_budget)
.map_err(dispatch_to_evm::<T>)
}
pallets/nonfungible/src/lib.rsdiffbeforeafterboth--- 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<T>,
sender: &T::CrossAccountId,
token_id: TokenId,
- property: Property,
+ properties: impl Iterator<Item = (PropertyKey, Option<PropertyValue>)>,
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;
- <TokenProperties<T>>::try_mutate((collection.id, token_id), |properties| {
- let property = property.clone();
- properties.try_set(property.key, property.value)
- })
- .map_err(<CommonError<T>>::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<bool, DispatchError> {
+ let is_owned = <PalletStructure<T>>::check_indirectly_owned(
+ sender.clone(),
+ collection.id,
+ token_id,
+ None,
+ nesting_budget,
+ )?;
+
+ Ok(is_owned)
+ })
+ };
- <PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertySet(
- collection.id,
- token_id,
- property.key,
- ));
+ for (key, value) in properties {
+ let permission = <PalletCommon<T>>::property_permissions(collection.id)
+ .get(&key)
+ .cloned()
+ .unwrap_or_else(PropertyPermission::none);
+
+ let is_property_exists = TokenProperties::<T>::get((collection.id, token_id))
+ .get(&key)
+ .is_some();
+ match permission {
+ PropertyPermission { mutable: false, .. } if is_property_exists => {
+ return Err(<CommonError<T>>::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!(<CommonError<T>>::NoPermission);
+ }
+ }
+ }
+
+ match value {
+ Some(value) => {
+ <TokenProperties<T>>::try_mutate((collection.id, token_id), |properties| {
+ properties.try_set(key.clone(), value)
+ })
+ .map_err(<CommonError<T>>::from)?;
+
+ <PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertySet(
+ collection.id,
+ token_id,
+ key,
+ ));
+ }
+ None => {
+ <TokenProperties<T>>::try_mutate((collection.id, token_id), |properties| {
+ properties.remove(&key)
+ })
+ .map_err(<CommonError<T>>::from)?;
+
+ <PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertyDeleted(
+ collection.id,
+ token_id,
+ key,
+ ));
+ }
+ }
+ }
+
Ok(())
}
- #[transactional]
pub fn set_token_properties(
collection: &NonfungibleHandle<T>,
sender: &T::CrossAccountId,
token_id: TokenId,
- properties: Vec<Property>,
+ properties: impl Iterator<Item = Property>,
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<T>,
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)?;
-
- <TokenProperties<T>>::try_mutate((collection.id, token_id), |properties| {
- properties.remove(&property_key)
- })
- .map_err(<CommonError<T>>::from)?;
+ let is_token_create = false;
- <PalletCommon<T>>::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<T>,
sender: &T::CrossAccountId,
token_id: TokenId,
- property_key: &PropertyKey,
- is_token_create: bool,
+ property_keys: impl Iterator<Item = PropertyKey>,
+ nesting_budget: &dyn Budget,
) -> DispatchResult {
- let permission = <PalletCommon<T>>::property_permissions(collection.id)
- .get(property_key)
- .cloned()
- .unwrap_or_else(PropertyPermission::none);
-
- let token_data = <TokenData<T>>::get((collection.id, token_id))
- .ok_or(<CommonError<T>>::TokenNotFound)?;
-
- let check_token_owner = || -> DispatchResult {
- ensure!(&token_data.owner == sender, <CommonError<T>>::NoPermission);
- Ok(())
- };
-
- let is_property_exists = TokenProperties::<T>::get((collection.id, token_id))
- .get(property_key)
- .is_some();
-
- match permission {
- PropertyPermission { mutable: false, .. } if is_property_exists => {
- Err(<CommonError<T>>::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(<CommonError<T>>::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<T>,
sender: &T::CrossAccountId,
token_id: TokenId,
- property_keys: Vec<PropertyKey>,
+ 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));
}
pallets/refungible/src/common.rsdiffbeforeafterboth314 _sender: T::CrossAccountId,314 _sender: T::CrossAccountId,315 _token_id: TokenId,315 _token_id: TokenId,316 _property: Vec<Property>,316 _property: Vec<Property>,317 _nesting_budget: &dyn Budget,317 ) -> DispatchResultWithPostInfo {318 ) -> DispatchResultWithPostInfo {318 fail!(<Error<T>>::SettingPropertiesNotAllowed)319 fail!(<Error<T>>::SettingPropertiesNotAllowed)319 }320 }331 _sender: T::CrossAccountId,332 _sender: T::CrossAccountId,332 _token_id: TokenId,333 _token_id: TokenId,333 _property_keys: Vec<PropertyKey>,334 _property_keys: Vec<PropertyKey>,335 _nesting_budget: &dyn Budget,334 ) -> DispatchResultWithPostInfo {336 ) -> DispatchResultWithPostInfo {335 fail!(<Error<T>>::SettingPropertiesNotAllowed)337 fail!(<Error<T>>::SettingPropertiesNotAllowed)336 }338 }340 _sender: <T>::CrossAccountId,342 _sender: <T>::CrossAccountId,341 _from: (CollectionId, TokenId),343 _from: (CollectionId, TokenId),342 _under: TokenId,344 _under: TokenId,343 _budget: &dyn Budget,345 _nesting_budget: &dyn Budget,344 ) -> sp_runtime::DispatchResult {346 ) -> sp_runtime::DispatchResult {345 fail!(<Error<T>>::RefungibleDisallowsNesting)347 fail!(<Error<T>>::RefungibleDisallowsNesting)346 }348 }pallets/unique/src/lib.rsdiffbeforeafterboth--- a/pallets/unique/src/lib.rs
+++ b/pallets/unique/src/lib.rs
@@ -654,8 +654,9 @@
ensure!(!properties.is_empty(), Error::<T>::EmptyArgument);
let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?);
+ let budget = budget::Value::new(NESTING_BUDGET);
- dispatch_tx::<T, _>(collection_id, |d| d.set_token_properties(sender, token_id, properties))
+ dispatch_tx::<T, _>(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::<T>::EmptyArgument);
let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?);
+ let budget = budget::Value::new(NESTING_BUDGET);
- dispatch_tx::<T, _>(collection_id, |d| d.delete_token_properties(sender, token_id, property_keys))
+ dispatch_tx::<T, _>(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)]
tests/src/nesting/properties.test.tsdiffbeforeafterboth--- 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
+});