From e5ae6048e96fb5df1b56a8e94689091a25e7bb56 Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Thu, 30 Jun 2022 18:38:43 +0000 Subject: [PATCH] fix: manage properties of nested tokens --- --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -1311,12 +1311,14 @@ sender: T::CrossAccountId, token_id: TokenId, property: Vec, + budget: &dyn Budget, ) -> DispatchResultWithPostInfo; fn delete_token_properties( &self, sender: T::CrossAccountId, token_id: TokenId, property_keys: Vec, + budget: &dyn Budget, ) -> DispatchResultWithPostInfo; fn set_token_property_permissions( &self, --- a/pallets/fungible/src/common.rs +++ b/pallets/fungible/src/common.rs @@ -298,6 +298,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property: Vec, + _budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -315,6 +316,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property_keys: Vec, + _budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -220,11 +220,12 @@ sender: T::CrossAccountId, token_id: TokenId, properties: Vec, + 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, false, budget), weight, ) } @@ -234,11 +235,12 @@ sender: T::CrossAccountId, token_id: TokenId, property_keys: Vec, + 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, budget), weight, ) } --- a/pallets/nonfungible/src/erc.rs +++ b/pallets/nonfungible/src/erc.rs @@ -38,6 +38,7 @@ use crate::{ AccountBalance, Config, CreateItemData, NonfungibleHandle, Pallet, TokenData, TokensMinted, SelfWeightOf, weights::WeightInfo, TokenProperties, + property_guard::PropertyGuard, }; #[solidity_interface(name = "TokenProperties")] @@ -82,14 +83,21 @@ .map_err(|_| "key too long")?; let value = value.try_into().map_err(|_| "value too long")?; - >::set_token_property( + let is_token_create = false; + let budget = self + .recorder + .weight_calls_budget(>::find_parent()); + + let mut guard = PropertyGuard::new( + &caller, self, - &caller, TokenId(token_id), - Property { key, value }, - false, - ) - .map_err(dispatch_to_evm::) + is_token_create, + &budget, + ); + + >::set_token_property(Property { key, value }, &mut guard) + .map_err(dispatch_to_evm::) } fn delete_property(&mut self, token_id: uint256, caller: caller, key: string) -> Result<()> { @@ -99,7 +107,20 @@ .try_into() .map_err(|_| "key too long")?; - >::delete_token_property(self, &caller, TokenId(token_id), key) + let is_token_create = false; + let budget = self + .recorder + .weight_calls_budget(>::find_parent()); + + let mut guard = PropertyGuard::new( + &caller, + self, + TokenId(token_id), + is_token_create, + &budget, + ); + + >::delete_token_property(key, &mut guard) .map_err(dispatch_to_evm::) } --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -52,6 +52,10 @@ pub mod erc; pub mod weights; +mod property_guard; + +use property_guard::*; + pub type CreateItemData = CreateNftExData<::CrossAccountId>; pub(crate) type SelfWeightOf = ::WeightInfo; @@ -484,30 +488,18 @@ }) } - pub fn set_token_property( - collection: &NonfungibleHandle, - sender: &T::CrossAccountId, - token_id: TokenId, - property: Property, - is_token_create: bool, - ) -> DispatchResult { - Self::check_token_change_permission( - collection, - sender, - token_id, - &property.key, - is_token_create, - )?; + pub fn set_token_property(property: Property, guard: &mut PropertyGuard<'_, T>) -> DispatchResult { + Self::check_token_change_permission(&property.key, guard)?; - >::try_mutate((collection.id, token_id), |properties| { + >::try_mutate((guard.collection.id, guard.token), |properties| { let property = property.clone(); properties.try_set(property.key, property.value) }) .map_err(>::from)?; >::deposit_event(CommonEvent::TokenPropertySet( - collection.id, - token_id, + guard.collection.id, + guard.token, property.key, )); @@ -521,57 +513,47 @@ token_id: TokenId, properties: Vec, is_token_create: bool, + nesting_budget: &dyn Budget, ) -> DispatchResult { + let mut guard = PropertyGuard::new( + sender, + collection, + token_id, + is_token_create, + nesting_budget, + ); + for property in properties { - Self::set_token_property(collection, sender, token_id, property, is_token_create)?; + Self::set_token_property(property, &mut guard)?; } Ok(()) } - pub fn delete_token_property( - collection: &NonfungibleHandle, - sender: &T::CrossAccountId, - token_id: TokenId, - property_key: PropertyKey, - ) -> DispatchResult { - Self::check_token_change_permission(collection, sender, token_id, &property_key, false)?; + pub fn delete_token_property(property_key: PropertyKey, guard: &mut PropertyGuard<'_, T>) -> DispatchResult { + Self::check_token_change_permission(&property_key, guard)?; - >::try_mutate((collection.id, token_id), |properties| { + >::try_mutate((guard.collection.id, guard.token), |properties| { properties.remove(&property_key) }) .map_err(>::from)?; >::deposit_event(CommonEvent::TokenPropertyDeleted( - collection.id, - token_id, + guard.collection.id, + guard.token, property_key, )); Ok(()) } - fn check_token_change_permission( - collection: &NonfungibleHandle, - sender: &T::CrossAccountId, - token_id: TokenId, - property_key: &PropertyKey, - is_token_create: bool, - ) -> DispatchResult { - let permission = >::property_permissions(collection.id) + fn check_token_change_permission(property_key: &PropertyKey, guard: &mut PropertyGuard<'_, T>) -> DispatchResult { + let permission = >::property_permissions(guard.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)) + let is_property_exists = TokenProperties::::get((guard.collection.id, guard.token)) .get(property_key) .is_some(); @@ -586,18 +568,18 @@ .. } => { //TODO: investigate threats during public minting. - if is_token_create && (collection_admin || token_owner) { + if guard.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); + check_result = guard.check_collection_admin(); } if token_owner { - check_result.or_else(|_| check_token_owner()) + check_result.or_else(|_| guard.check_token_owner()) } else { check_result } @@ -611,9 +593,20 @@ sender: &T::CrossAccountId, token_id: TokenId, property_keys: Vec, + nesting_budget: &dyn Budget, ) -> DispatchResult { + let is_token_create = false; + + let mut guard = PropertyGuard::new( + sender, + collection, + token_id, + is_token_create, + nesting_budget, + ); + for key in property_keys { - Self::delete_token_property(collection, sender, token_id, key)?; + Self::delete_token_property(key, &mut guard)?; } Ok(()) @@ -829,6 +822,7 @@ TokenId(token), data.properties.clone().into_inner(), 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, + _budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } @@ -331,6 +332,7 @@ _sender: T::CrossAccountId, _token_id: TokenId, _property_keys: Vec, + _budget: &dyn Budget, ) -> DispatchResultWithPostInfo { fail!(>::SettingPropertiesNotAllowed) } --- 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)] -- gitstuff