git.delta.rocks / unique-network / refs/commits / fc0c96747adc

difftreelog

refactor use modify_token_properties

Daniel Shiposha2022-07-04parent: #85380b5.patch.diff
in: master

4 files changed

modifiedpallets/nonfungible/src/common.rsdiffbeforeafterboth
229 self,229 self,
230 &sender,230 &sender,
231 token_id,231 token_id,
232 properties,232 properties.into_iter(),
233 false,233 false,
234 nesting_budget,234 nesting_budget,
235 ),235 ),
251 self,251 self,
252 &sender,252 &sender,
253 token_id,253 token_id,
254 property_keys,254 property_keys.into_iter(),
255 nesting_budget,255 nesting_budget,
256 ),256 ),
257 weight,257 weight,
modifiedpallets/nonfungible/src/erc.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/erc.rs
+++ b/pallets/nonfungible/src/erc.rs
@@ -37,7 +37,7 @@
 
 use crate::{
 	AccountBalance, Config, CreateItemData, NonfungibleHandle, Pallet, TokenData, TokensMinted,
-	SelfWeightOf, weights::WeightInfo, TokenProperties, property_guard::*,
+	SelfWeightOf, weights::WeightInfo, TokenProperties,
 };
 
 #[solidity_interface(name = "TokenProperties")]
@@ -82,21 +82,18 @@
 			.map_err(|_| "key too long")?;
 		let value = value.try_into().map_err(|_| "value too long")?;
 
-		let is_token_create = false;
 		let nesting_budget = self
 			.recorder
 			.weight_calls_budget(<StructureWeight<T>>::find_parent());
 
-		let mut guard = PropertyGuard::new(PropertyGuardData {
-			sender: &caller,
-			collection: self,
-			token_id: TokenId(token_id),
-			is_token_create,
-			nesting_budget: &nesting_budget,
-		});
-
-		<Pallet<T>>::set_token_property(Property { key, value }, &mut guard)
-			.map_err(dispatch_to_evm::<T>)
+		<Pallet<T>>::set_token_property(
+			self,
+			&caller,
+			TokenId(token_id),
+			Property { key, value },
+			&nesting_budget,
+		)
+		.map_err(dispatch_to_evm::<T>)
 	}
 
 	fn delete_property(&mut self, token_id: uint256, caller: caller, key: string) -> Result<()> {
@@ -106,20 +103,12 @@
 			.try_into()
 			.map_err(|_| "key too long")?;
 
-		let is_token_create = false;
 		let nesting_budget = self
 			.recorder
 			.weight_calls_budget(<StructureWeight<T>>::find_parent());
 
-		let mut guard = PropertyGuard::new(PropertyGuardData {
-			sender: &caller,
-			collection: self,
-			token_id: TokenId(token_id),
-			is_token_create,
-			nesting_budget: &nesting_budget,
-		});
-
-		<Pallet<T>>::delete_token_property(key, &mut guard).map_err(dispatch_to_evm::<T>)
+		<Pallet<T>>::delete_token_property(self, &caller, TokenId(token_id), key, &nesting_budget)
+			.map_err(dispatch_to_evm::<T>)
 	}
 
 	/// Throws error if key not found
modifiedpallets/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::{
@@ -51,10 +52,6 @@
 pub mod common;
 pub mod erc;
 pub mod weights;
-
-mod property_guard;
-
-use property_guard::*;
 
 pub type CreateItemData<T> = CreateNftExData<<T as pallet_evm::account::Config>::CrossAccountId>;
 pub(crate) type SelfWeightOf<T> = <T as Config>::WeightInfo;
@@ -89,6 +86,8 @@
 		NonfungibleItemsHaveNoAmount,
 		/// Unable to burn NFT with children
 		CantBurnNftWithChildren,
+		/// Unable to create an empty property
+		UnableToCreateEmptyProperty,
 	}
 
 	#[pallet::config]
@@ -487,113 +486,152 @@
 			pays_fee: Pays::Yes,
 		})
 	}
-
-	pub fn set_token_property(
-		property: Property,
-		guard: &mut PropertyGuard<'_, T>,
-	) -> DispatchResult {
-		Self::check_token_change_permission(&property.key, guard)?;
 
-		<TokenProperties<T>>::try_mutate((guard.collection.id, guard.token_id), |properties| {
-			let property = property.clone();
-			properties.try_set(property.key, property.value)
-		})
-		.map_err(<CommonError<T>>::from)?;
-
-		<PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertySet(
-			guard.collection.id,
-			guard.token_id,
-			property.key,
-		));
-
-		Ok(())
-	}
-
 	#[transactional]
-	pub fn set_token_properties(
+	fn modify_token_properties(
 		collection: &NonfungibleHandle<T>,
 		sender: &T::CrossAccountId,
 		token_id: TokenId,
-		properties: Vec<Property>,
+		properties: impl Iterator<Item = (PropertyKey, Option<PropertyValue>)>,
 		is_token_create: bool,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
-		let mut guard = PropertyGuard::new(PropertyGuardData {
-			sender,
-			collection,
-			token_id,
-			is_token_create,
-			nesting_budget,
-		});
+		let mut collection_admin_result = None;
+		let mut token_owner_result = None;
 
-		for property in properties {
-			Self::set_token_property(property, &mut guard)?;
-		}
+		let mut check_collection_admin = || {
+			*collection_admin_result
+				.get_or_insert_with(|| collection.check_is_owner_or_admin(sender))
+		};
 
-		Ok(())
-	}
+		let mut check_token_owner = || {
+			*token_owner_result.get_or_insert_with(|| {
+				let is_owned = <PalletStructure<T>>::check_indirectly_owned(
+					sender.clone(),
+					collection.id,
+					token_id,
+					None,
+					nesting_budget,
+				)?;
 
-	pub fn delete_token_property(
-		property_key: PropertyKey,
-		guard: &mut PropertyGuard<'_, T>,
-	) -> DispatchResult {
-		Self::check_token_change_permission(&property_key, guard)?;
+				if is_owned {
+					Ok(())
+				} else {
+					Err(<CommonError<T>>::NoPermission.into())
+				}
+			})
+		};
 
-		<TokenProperties<T>>::try_mutate((guard.collection.id, guard.token_id), |properties| {
-			properties.remove(&property_key)
-		})
-		.map_err(<CommonError<T>>::from)?;
+		for (key, value) in properties {
+			let permission = <PalletCommon<T>>::property_permissions(collection.id)
+				.get(&key)
+				.cloned()
+				.unwrap_or_else(PropertyPermission::none);
 
-		<PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertyDeleted(
-			guard.collection.id,
-			guard.token_id,
-			property_key,
-		));
+			let is_property_exists = TokenProperties::<T>::get((collection.id, token_id))
+				.get(&key)
+				.is_some();
 
-		Ok(())
-	}
+			match permission {
+				PropertyPermission { mutable: false, .. } if is_property_exists => {
+					return Err(<CommonError<T>>::NoPermission.into());
+				}
 
-	fn check_token_change_permission(
-		property_key: &PropertyKey,
-		guard: &mut PropertyGuard<'_, T>,
-	) -> DispatchResult {
-		let permission = <PalletCommon<T>>::property_permissions(guard.collection.id)
-			.get(property_key)
-			.cloned()
-			.unwrap_or_else(PropertyPermission::none);
+				PropertyPermission {
+					collection_admin,
+					token_owner,
+					..
+				} => {
+					//TODO: investigate threats during public minting.
+					if is_token_create && (collection_admin || token_owner) {
+						if value.is_some() {
+							return Ok(());
+						} else {
+							return Err(<Error<T>>::UnableToCreateEmptyProperty.into());
+						}
+					}
 
-		let is_property_exists = TokenProperties::<T>::get((guard.collection.id, guard.token_id))
-			.get(property_key)
-			.is_some();
+					let mut check_result = Err(<CommonError<T>>::NoPermission.into());
 
-		match permission {
-			PropertyPermission { mutable: false, .. } if is_property_exists => {
-				Err(<CommonError<T>>::NoPermission.into())
-			}
+					if collection_admin {
+						check_result = check_collection_admin();
+					}
 
-			PropertyPermission {
-				collection_admin,
-				token_owner,
-				..
-			} => {
-				//TODO: investigate threats during public minting.
-				if guard.is_token_create && (collection_admin || token_owner) {
-					return Ok(());
+					if token_owner {
+						check_result = check_result.or_else(|_| check_token_owner())
+					}
+
+					check_result?;
 				}
+			}
 
-				let mut check_result = Err(<CommonError<T>>::NoPermission.into());
+			match value {
+				Some(value) => {
+					<TokenProperties<T>>::try_mutate((collection.id, token_id), |properties| {
+						properties.try_set(key.clone(), value)
+					})
+					.map_err(<CommonError<T>>::from)?;
 
-				if collection_admin {
-					check_result = guard.check_collection_admin();
+					<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)?;
 
-				if token_owner {
-					check_result.or_else(|_| guard.check_token_owner())
-				} else {
-					check_result
+					<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: impl Iterator<Item = Property>,
+		is_token_create: bool,
+		nesting_budget: &dyn Budget,
+	) -> DispatchResult {
+		Self::modify_token_properties(
+			collection,
+			sender,
+			token_id,
+			properties.map(|p| (p.key, Some(p.value))),
+			is_token_create,
+			nesting_budget,
+		)
+	}
+
+	pub fn set_token_property(
+		collection: &NonfungibleHandle<T>,
+		sender: &T::CrossAccountId,
+		token_id: TokenId,
+		property: Property,
+		nesting_budget: &dyn Budget,
+	) -> DispatchResult {
+		let is_token_create = false;
+
+		Self::set_token_properties(
+			collection,
+			sender,
+			token_id,
+			[property].into_iter(),
+			is_token_create,
+			nesting_budget,
+		)
 	}
 
 	#[transactional]
@@ -601,24 +639,35 @@
 		collection: &NonfungibleHandle<T>,
 		sender: &T::CrossAccountId,
 		token_id: TokenId,
-		property_keys: Vec<PropertyKey>,
+		property_keys: impl Iterator<Item = PropertyKey>,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
 		let is_token_create = false;
 
-		let mut guard = PropertyGuard::new(PropertyGuardData {
+		Self::modify_token_properties(
+			collection,
 			sender,
-			collection,
 			token_id,
+			property_keys.into_iter().map(|key| (key, None)),
 			is_token_create,
 			nesting_budget,
-		});
+		)
+	}
 
-		for key in property_keys {
-			Self::delete_token_property(key, &mut guard)?;
-		}
-
-		Ok(())
+	pub fn delete_token_property(
+		collection: &NonfungibleHandle<T>,
+		sender: &T::CrossAccountId,
+		token_id: TokenId,
+		property_key: PropertyKey,
+		nesting_budget: &dyn Budget,
+	) -> DispatchResult {
+		Self::delete_token_properties(
+			collection,
+			sender,
+			token_id,
+			[property_key].into_iter(),
+			nesting_budget,
+		)
 	}
 
 	pub fn set_collection_properties(
@@ -829,7 +878,7 @@
 					collection,
 					sender,
 					TokenId(token),
-					data.properties.clone().into_inner(),
+					data.properties.clone().into_iter(),
 					true,
 					nesting_budget,
 				) {
deletedpallets/nonfungible/src/property_guard.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/property_guard.rs
+++ /dev/null
@@ -1,59 +0,0 @@
-use super::*;
-
-pub struct PropertyGuard<'a, T: Config> {
-	pub sender: &'a T::CrossAccountId,
-	pub collection: &'a NonfungibleHandle<T>,
-	pub token_id: TokenId,
-	pub is_token_create: bool,
-	nesting_budget: &'a dyn Budget,
-
-	collection_admin_result: Option<DispatchResult>,
-	token_owner_result: Option<DispatchResult>,
-}
-
-pub struct PropertyGuardData<'a, T: Config> {
-	pub sender: &'a T::CrossAccountId,
-	pub collection: &'a NonfungibleHandle<T>,
-	pub token_id: TokenId,
-	pub is_token_create: bool,
-	pub nesting_budget: &'a dyn Budget,
-}
-
-impl<'a, T: Config> PropertyGuard<'a, T> {
-	pub fn new(data: PropertyGuardData<'a, T>) -> Self {
-		Self {
-			sender: data.sender,
-			collection: data.collection,
-			token_id: data.token_id,
-			is_token_create: data.is_token_create,
-			nesting_budget: data.nesting_budget,
-
-			collection_admin_result: None,
-			token_owner_result: None,
-		}
-	}
-
-	pub fn check_collection_admin(&mut self) -> DispatchResult {
-		*self
-			.collection_admin_result
-			.get_or_insert_with(|| self.collection.check_is_owner_or_admin(self.sender))
-	}
-
-	pub fn check_token_owner(&mut self) -> DispatchResult {
-		*self.token_owner_result.get_or_insert_with(|| {
-			let is_owned = <PalletStructure<T>>::check_indirectly_owned(
-				self.sender.clone(),
-				self.collection.id,
-				self.token_id,
-				None,
-				self.nesting_budget,
-			)?;
-
-			if is_owned {
-				Ok(())
-			} else {
-				Err(<CommonError<T>>::NoPermission.into())
-			}
-		})
-	}
-}