From 0427b9aac8a21112835725f412985b96f44f1252 Mon Sep 17 00:00:00 2001 From: Trubnikov Sergey Date: Thu, 07 Jul 2022 09:56:31 +0000 Subject: [PATCH] CORE-410 Adapt to prop check root owner --- --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -327,12 +327,19 @@ sender: T::CrossAccountId, token_id: TokenId, properties: Vec, - _nesting_budget: &dyn Budget, + 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, ) } @@ -356,12 +363,18 @@ sender: T::CrossAccountId, token_id: TokenId, property_keys: Vec, - _nesting_budget: &dyn Budget, + 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, ) } --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -87,14 +87,18 @@ #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::{ensure, BoundedVec, transactional, storage::with_transaction}; +use frame_support::{ensure, fail, BoundedVec, transactional, storage::with_transaction}; use up_data_structs::{ AccessMode, CollectionId, CustomDataLimit, MAX_REFUNGIBLE_PIECES, TokenId, CreateCollectionData, CreateRefungibleExData, mapping::TokenAddressMapping, budget::Budget, - Property, PropertyScope, TrySetProperty, PropertyKey, PropertyPermission, PropertyKeyPermission + Property, PropertyScope, TrySetProperty, PropertyKey, PropertyValue, PropertyPermission, + PropertyKeyPermission, }; use pallet_evm::account::CrossAccountId; -use pallet_common::{Error as CommonError, Event as CommonEvent, Pallet as PalletCommon, CommonCollectionOperations as _}; +use pallet_common::{ + Error as CommonError, Event as CommonEvent, Pallet as PalletCommon, + CommonCollectionOperations as _, +}; use pallet_structure::Pallet as PalletStructure; use sp_runtime::{ArithmeticError, DispatchError, DispatchResult, TransactionOutcome}; use sp_std::{vec::Vec, vec, collections::btree_map::BTreeMap}; @@ -466,140 +470,168 @@ Ok(()) } - pub fn set_token_property( + #[transactional] + fn modify_token_properties( collection: &RefungibleHandle, 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 is_collection_admin = || collection.is_owner_or_admin(sender); + let is_token_owner = || -> Result { + let balance = collection.balance(sender.clone(), token_id); + let total_pieces: u128 = + Self::total_pieces(collection.id, token_id).unwrap_or(u128::MAX); + if balance != total_pieces { + return Ok(false); + } - >::try_mutate((collection.id, token_id), |properties| { - let property = property.clone(); - properties.try_set(property.key, property.value) - }) - .map_err(>::from)?; + let is_bundle_owner = >::check_indirectly_owned( + sender.clone(), + collection.id, + token_id, + None, + nesting_budget, + )?; + + Ok(is_bundle_owner) + }; + + for (key, value) in properties { + let permission = >::property_permissions(collection.id) + .get(&key) + .cloned() + .unwrap_or_else(PropertyPermission::none); - >::deposit_event(CommonEvent::TokenPropertySet( - collection.id, - token_id, - property.key, - )); + 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. + let is_token_create = + is_token_create && (collection_admin || token_owner) && value.is_some(); + if !(is_token_create + || (collection_admin && is_collection_admin()) + || (token_owner && is_token_owner()?)) + { + 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: &RefungibleHandle, 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: &RefungibleHandle, 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)?; + let is_token_create = false; - >::try_mutate((collection.id, token_id), |properties| { - properties.remove(&property_key) - }) - .map_err(>::from)?; - - >::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: &RefungibleHandle, 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 is_token_create = false; - // Not "try_fold" because total count of pieces is limited by 'MAX_REFUNGIBLE_PIECES'. - let total_pieces: u128 = >::iter_prefix((collection.id, token_id,)).fold(0, |total, piece| total + piece.1); - let balance = collection.balance(sender.clone(), token_id); - - let check_token_owner = || -> DispatchResult { - ensure!(balance == total_pieces, >::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()) - } - - 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: &RefungibleHandle, 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, + ) } /// Transfer RFT token pieces from one account to another. @@ -833,8 +865,9 @@ collection, sender, TokenId(token_id), - data.properties.clone().into_inner(), + data.properties.clone().into_iter(), true, + nesting_budget, ) { return TransactionOutcome::Rollback(Err(e)); } @@ -854,7 +887,7 @@ for (user, amount) in token.users.into_iter() { if amount == 0 { continue; - } + } // TODO: ERC20 transfer event >::deposit_event(CommonEvent::ItemCreated( @@ -1049,10 +1082,10 @@ ); ensure!(amount > 0, >::TokenValueTooLow); // Ensure user owns all pieces - let total_supply = >::get((collection.id, token)); + let total_pieces = Self::total_pieces(collection.id, token).unwrap_or(u128::MAX); let balance = >::get((collection.id, token, owner)); ensure!( - total_supply == balance, + total_pieces == balance, >::RepartitionWhileNotOwningAllPieces ); @@ -1064,7 +1097,7 @@ fn total_pieces(collection_id: CollectionId, token_id: TokenId) -> Option { >::try_get((collection_id, token_id)).ok() } - + pub fn set_collection_properties( collection: &RefungibleHandle, sender: &T::CrossAccountId, --- a/primitives/data-structs/src/lib.rs +++ b/primitives/data-structs/src/lib.rs @@ -534,9 +534,9 @@ #[cfg_attr(feature = "serde1", serde(with = "bounded::vec_serde"))] #[derivative(Debug(format_with = "bounded::vec_debug"))] pub const_data: BoundedVec, - + pub pieces: u128, - + #[cfg_attr(feature = "serde1", serde(with = "bounded::vec_serde"))] #[derivative(Debug(format_with = "bounded::vec_debug"))] pub properties: CollectionPropertiesVec, -- gitstuff