git.delta.rocks / unique-network / refs/commits / 0ba1adb79fd8

difftreelog

misk: generalize 2 methods

Trubnikov Sergey2023-02-01parent: #e31d9d0.patch.diff
in: master

3 files changed

modifiedpallets/common/src/lib.rsdiffbeforeafterboth
--- a/pallets/common/src/lib.rs
+++ b/pallets/common/src/lib.rs
@@ -66,10 +66,11 @@
 	ensure,
 	traits::{Imbalance, Get, Currency, WithdrawReasons, ExistenceRequirement},
 	dispatch::Pays,
-	transactional,
+	transactional, fail,
 };
 use pallet_evm::GasWeightMapping;
 use up_data_structs::{
+	AccessMode,
 	COLLECTION_NUMBER_LIMIT,
 	Collection,
 	RpcCollection,
@@ -124,6 +125,8 @@
 pub use pallet::*;
 use sp_core::H160;
 use sp_runtime::{ArithmeticError, DispatchError, DispatchResult};
+
+use crate::erc::CollectionHelpersEvents;
 #[cfg(feature = "runtime-benchmarks")]
 pub mod benchmarking;
 pub mod dispatch;
@@ -1264,6 +1267,128 @@
 		Ok(())
 	}
 
+	/// A batch operation to add, edit or remove properties for a token.
+	/// It sets or removes a token's properties according to
+	/// `properties_updates` contents:
+	/// * sets a property under the <key> with the value provided `(<key>, Some(<value>))`
+	/// * removes a property under the <key> if the value is `None` `(<key>, None)`.
+	///
+	/// - `nesting_budget`: Limit for searching parents in-depth to check ownership.
+	/// - `is_token_create`: Indicates that method is called during token initialization.
+	///   Allows to bypass ownership check.
+	///
+	/// All affected properties should have `mutable` permission
+	/// to be **deleted** or to be **set more than once**,
+	/// and the sender should have permission to edit those properties.
+	///
+	/// This function fires an event for each property change.
+	/// In case of an error, all the changes (including the events) will be reverted
+	/// since the function is transactional.
+	pub fn modify_token_properties(
+		collection: &CollectionHandle<T>,
+		sender: &T::CrossAccountId,
+		token_id: TokenId,
+		properties_updates: impl Iterator<Item = (PropertyKey, Option<PropertyValue>)>,
+		is_token_create: bool,
+		mut stored_properties: Properties,
+		is_token_owner: impl Fn() -> Result<bool, DispatchError>,
+		set_token_properties: impl FnOnce(Properties),
+	) -> DispatchResult {
+		let is_collection_admin = collection.is_owner_or_admin(sender);
+		let permissions = Self::property_permissions(collection.id);
+
+		for (key, value) in properties_updates {
+			let permission = permissions
+				.get(&key)
+				.cloned()
+				.unwrap_or_else(PropertyPermission::none);
+
+			let is_property_exists = stored_properties.get(&key).is_some();
+
+			match permission {
+				PropertyPermission { mutable: false, .. } if is_property_exists => {
+					return Err(<Error<T>>::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!(<Error<T>>::NoPermission);
+					}
+				}
+			}
+
+			match value {
+				Some(value) => {
+					stored_properties
+						.try_set(key.clone(), value)
+						.map_err(<Error<T>>::from)?;
+
+					Self::deposit_event(Event::TokenPropertySet(collection.id, token_id, key));
+				}
+				None => {
+					stored_properties.remove(&key).map_err(<Error<T>>::from)?;
+
+					Self::deposit_event(Event::TokenPropertyDeleted(collection.id, token_id, key));
+				}
+			}
+
+			<PalletEvm<T>>::deposit_log(
+				CollectionHelpersEvents::TokenChanged {
+					collection_id: eth::collection_id_to_address(collection.id),
+					token_id: token_id.into(),
+				}
+				.to_log(T::ContractAddress::get()),
+			);
+		}
+
+		set_token_properties(stored_properties);
+
+		Ok(())
+	}
+
+	/// Sets or unsets the approval of a given operator.
+	///
+	/// The `operator` is allowed to transfer all token pieces of the `owner` on their behalf.
+	/// - `owner`: Token owner
+	/// - `operator`: Operator
+	/// - `approve`: Should operator status be granted or revoked?
+	pub fn set_allowance_for_all(
+		collection: &CollectionHandle<T>,
+		owner: &T::CrossAccountId,
+		operator: &T::CrossAccountId,
+		approve: bool,
+		set_allowance: impl FnOnce(),
+		log: evm_coder::ethereum::Log,
+	) -> DispatchResult {
+		if collection.permissions.access() == AccessMode::AllowList {
+			collection.check_allowlist(owner)?;
+			collection.check_allowlist(operator)?;
+		}
+
+		Self::ensure_correct_receiver(operator)?;
+
+		set_allowance();
+
+		<PalletEvm<T>>::deposit_log(log);
+		Self::deposit_event(Event::ApprovedForAll(
+			collection.id,
+			owner.clone(),
+			operator.clone(),
+			approve,
+		));
+		Ok(())
+	}
+
 	/// Set collection property.
 	///
 	/// * `collection` - Collection handler.
modifiedpallets/nonfungible/src/lib.rsdiffbeforeafterboth
101};101};
102use up_data_structs::{102use up_data_structs::{
103 AccessMode, CollectionId, CollectionFlags, CustomDataLimit, TokenId, CreateCollectionData,103 AccessMode, CollectionId, CollectionFlags, CustomDataLimit, TokenId, CreateCollectionData,
104 CreateNftExData, mapping::TokenAddressMapping, budget::Budget, Property, PropertyPermission,104 CreateNftExData, mapping::TokenAddressMapping, budget::Budget, Property, PropertyKey,
105 PropertyKey, PropertyValue, PropertyKeyPermission, Properties, PropertyScope, TrySetProperty,105 PropertyValue, PropertyKeyPermission, Properties, PropertyScope, TrySetProperty, TokenChild,
106 TokenChild, AuxPropertyValue, PropertiesPermissionMap,106 AuxPropertyValue, PropertiesPermissionMap,
107};107};
108use pallet_evm::{account::CrossAccountId, Pallet as PalletEvm};108use pallet_evm::{account::CrossAccountId, Pallet as PalletEvm};
109use pallet_common::{109use pallet_common::{
110 Error as CommonError, Pallet as PalletCommon, Event as CommonEvent, CollectionHandle,110 Error as CommonError, Pallet as PalletCommon, Event as CommonEvent, CollectionHandle,
111 eth::collection_id_to_address, erc::CollectionHelpersEvents,111 eth::collection_id_to_address,
112};112};
113use pallet_structure::{Pallet as PalletStructure, Error as StructureError};113use pallet_structure::{Pallet as PalletStructure, Error as StructureError};
114use pallet_evm_coder_substrate::{SubstrateRecorder, WithRecorder};114use pallet_evm_coder_substrate::{SubstrateRecorder, WithRecorder};
115use sp_core::{H160, Get};115use sp_core::H160;
116use sp_runtime::{ArithmeticError, DispatchError, DispatchResult, TransactionOutcome};116use sp_runtime::{ArithmeticError, DispatchError, DispatchResult, TransactionOutcome};
117use sp_std::{vec::Vec, vec, collections::btree_map::BTreeMap};117use sp_std::{vec::Vec, vec, collections::btree_map::BTreeMap};
118use core::ops::Deref;118use core::ops::Deref;
578 }578 }
579579
580 /// A batch operation to add, edit or remove properties for a token.580 /// A batch operation to add, edit or remove properties for a token.
581 /// It sets or removes a token's properties according to
582 /// `properties_updates` contents:
583 /// * sets a property under the <key> with the value provided `(<key>, Some(<value>))`
584 /// * removes a property under the <key> if the value is `None` `(<key>, None)`.
585 ///581 ///
586 /// - `nesting_budget`: Limit for searching parents in-depth to check ownership.582 /// - `nesting_budget`: Limit for searching parents in-depth to check ownership.
587 /// - `is_token_create`: Indicates that method is called during token initialization.583 /// - `is_token_create`: Indicates that method is called during token initialization.
603 is_token_create: bool,599 is_token_create: bool,
604 nesting_budget: &dyn Budget,600 nesting_budget: &dyn Budget,
605 ) -> DispatchResult {601 ) -> DispatchResult {
606 let mut collection_admin_status = None;
607 let mut token_owner_result = None;
608
609 let mut is_collection_admin =
610 || *collection_admin_status.get_or_insert_with(|| collection.is_owner_or_admin(sender));
611
612 let mut is_token_owner = || {602 let is_token_owner = || {
613 *token_owner_result.get_or_insert_with(|| -> Result<bool, DispatchError> {
614 let is_owned = <PalletStructure<T>>::check_indirectly_owned(603 let is_owned = <PalletStructure<T>>::check_indirectly_owned(
615 sender.clone(),604 sender.clone(),
616 collection.id,605 collection.id,
620 )?;609 )?;
621610
622 Ok(is_owned)611 Ok(is_owned)
623 })
624 };612 };
625613
626 let mut stored_properties = <TokenProperties<T>>::get((collection.id, token_id));614 let stored_properties = <TokenProperties<T>>::get((collection.id, token_id));
615
627 let permissions = <PalletCommon<T>>::property_permissions(collection.id);616 <PalletCommon<T>>::modify_token_properties(
628
629 for (key, value) in properties_updates {617 collection,
630 let permission = permissions
631 .get(&key)
632 .cloned()
633 .unwrap_or_else(PropertyPermission::none);
634
635 let is_property_exists = stored_properties.get(&key).is_some();
636
637 match permission {
638 PropertyPermission { mutable: false, .. } if is_property_exists => {618 sender,
639 return Err(<CommonError<T>>::NoPermission.into());
640 }
641
642 PropertyPermission {
643 collection_admin,619 token_id,
644 token_owner,620 properties_updates,
645 ..
646 } => {
647 //TODO: investigate threats during public minting.
648 if is_token_create && (collection_admin || token_owner) && value.is_some() {621 is_token_create,
649 // Pass
650 } else if collection_admin && is_collection_admin() {
651 // Pass
652 } else if token_owner && is_token_owner()? {
653 // Pass
654 } else {
655 fail!(<CommonError<T>>::NoPermission);
656 }
657 }
658 }
659
660 match value {
661 Some(value) => {
662 stored_properties
663 .try_set(key.clone(), value)
664 .map_err(<CommonError<T>>::from)?;
665
666 <PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertySet(
667 collection.id,
668 token_id,
669 key,
670 ));
671 }
672 None => {
673 stored_properties622 stored_properties,
674 .remove(&key)
675 .map_err(<CommonError<T>>::from)?;
676
677 <PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertyDeleted(
678 collection.id,
679 token_id,
680 key,
681 ));
682 }
683 }
684
685 <PalletEvm<T>>::deposit_log(
686 CollectionHelpersEvents::TokenChanged {
687 collection_id: collection_id_to_address(collection.id),
688 token_id: token_id.into(),
689 }
690 .to_log(T::ContractAddress::get()),623 is_token_owner,
691 );
692 }
693
694 <TokenProperties<T>>::set((collection.id, token_id), stored_properties);624 |properties| <TokenProperties<T>>::set((collection.id, token_id), properties),
695625 )
696 Ok(())
697 }626 }
698627
699 /// Batch operation to add or edit properties for the token628 /// Batch operation to add or edit properties for the token
1418 operator: &T::CrossAccountId,1347 operator: &T::CrossAccountId,
1419 approve: bool,1348 approve: bool,
1420 ) -> DispatchResult {1349 ) -> DispatchResult {
1421 if collection.permissions.access() == AccessMode::AllowList {1350 <PalletCommon<T>>::set_allowance_for_all(
1422 collection.check_allowlist(owner)?;1351 collection,
1423 collection.check_allowlist(operator)?;1352 owner,
1424 }1353 operator,
14251354 approve,
1426 <PalletCommon<T>>::ensure_correct_receiver(operator)?;
1427
1428 // =========
1429
1430 <CollectionAllowance<T>>::insert((collection.id, owner, operator), approve);1355 || <CollectionAllowance<T>>::insert((collection.id, owner, operator), approve),
1431 <PalletEvm<T>>::deposit_log(
1432 ERC721Events::ApprovalForAll {1356 ERC721Events::ApprovalForAll {
1433 owner: *owner.as_eth(),1357 owner: *owner.as_eth(),
1434 operator: *operator.as_eth(),1358 operator: *operator.as_eth(),
1435 approved: approve,1359 approved: approve,
1436 }1360 }
1437 .to_log(collection_id_to_address(collection.id)),1361 .to_log(collection_id_to_address(collection.id)),
1438 );1362 )
1439 <PalletCommon<T>>::deposit_event(CommonEvent::ApprovedForAll(
1440 collection.id,
1441 owner.clone(),
1442 operator.clone(),
1443 approve,
1444 ));
1445 Ok(())
1446 }1363 }
14471364
1448 /// Tells whether the given `owner` approves the `operator`.1365 /// Tells whether the given `owner` approves the `operator`.
modifiedpallets/refungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/refungible/src/lib.rs
+++ b/pallets/refungible/src/lib.rs
@@ -92,22 +92,22 @@
 
 use core::ops::Deref;
 use evm_coder::ToLog;
-use frame_support::{ensure, fail, storage::with_transaction, transactional};
+use frame_support::{ensure, storage::with_transaction, transactional};
 use pallet_evm::{account::CrossAccountId, Pallet as PalletEvm};
 use pallet_evm_coder_substrate::WithRecorder;
 use pallet_common::{
 	CommonCollectionOperations, Error as CommonError, eth::collection_id_to_address,
-	Event as CommonEvent, Pallet as PalletCommon, erc::CollectionHelpersEvents,
+	Event as CommonEvent, Pallet as PalletCommon,
 };
 use pallet_structure::Pallet as PalletStructure;
-use sp_core::{Get, H160};
+use sp_core::H160;
 use sp_runtime::{ArithmeticError, DispatchError, DispatchResult, TransactionOutcome};
 use sp_std::{vec::Vec, vec, collections::btree_map::BTreeMap};
 use up_data_structs::{
 	AccessMode, budget::Budget, CollectionId, CollectionFlags, CreateCollectionData,
 	mapping::TokenAddressMapping, MAX_REFUNGIBLE_PIECES, Property, PropertyKey,
-	PropertyKeyPermission, PropertyPermission, PropertyScope, PropertyValue, TokenId,
-	TrySetProperty, PropertiesPermissionMap, CreateRefungibleExMultipleOwners, TokenOwnerError,
+	PropertyKeyPermission, PropertyScope, PropertyValue, TokenId, TrySetProperty,
+	PropertiesPermissionMap, CreateRefungibleExMultipleOwners, TokenOwnerError,
 };
 
 pub use pallet::*;
@@ -245,8 +245,8 @@
 	pub type CollectionAllowance<T: Config> = StorageNMap<
 		Key = (
 			Key<Twox64Concat, CollectionId>,
-			Key<Blake2_128Concat, T::CrossAccountId>,
-			Key<Blake2_128Concat, T::CrossAccountId>,
+			Key<Blake2_128Concat, T::CrossAccountId>, // Owner
+			Key<Blake2_128Concat, T::CrossAccountId>, // Operator
 		),
 		Value = bool,
 		QueryKind = ValueQuery,
@@ -541,7 +541,6 @@
 		is_token_create: bool,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
-		let is_collection_admin = || collection.is_owner_or_admin(sender);
 		let is_token_owner = || -> Result<bool, DispatchError> {
 			let balance = collection.balance(sender.clone(), token_id);
 			let total_pieces: u128 =
@@ -560,77 +559,19 @@
 
 			Ok(is_bundle_owner)
 		};
-
-		let mut stored_properties = <TokenProperties<T>>::get((collection.id, token_id));
-		let permissions = <PalletCommon<T>>::property_permissions(collection.id);
-
-		for (key, value) in properties_updates {
-			let permission = permissions
-				.get(&key)
-				.cloned()
-				.unwrap_or_else(PropertyPermission::none);
-
-			let is_property_exists = stored_properties.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.
-					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!(<CommonError<T>>::NoPermission);
-					}
-				}
-			}
-
-			match value {
-				Some(value) => {
-					stored_properties
-						.try_set(key.clone(), value)
-						.map_err(<CommonError<T>>::from)?;
-
-					<PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertySet(
-						collection.id,
-						token_id,
-						key,
-					));
-				}
-				None => {
-					stored_properties
-						.remove(&key)
-						.map_err(<CommonError<T>>::from)?;
+		let stored_properties = <TokenProperties<T>>::get((collection.id, token_id));
 
-					<PalletCommon<T>>::deposit_event(CommonEvent::TokenPropertyDeleted(
-						collection.id,
-						token_id,
-						key,
-					));
-				}
-			}
-
-			<PalletEvm<T>>::deposit_log(
-				CollectionHelpersEvents::TokenChanged {
-					collection_id: collection_id_to_address(collection.id),
-					token_id: token_id.into(),
-				}
-				.to_log(T::ContractAddress::get()),
-			);
-		}
-
-		<TokenProperties<T>>::set((collection.id, token_id), stored_properties);
-
-		Ok(())
+		<PalletCommon<T>>::modify_token_properties(
+			collection,
+			sender,
+			token_id,
+			properties_updates,
+			is_token_create,
+			stored_properties,
+			is_token_owner,
+			|properties| <TokenProperties<T>>::set((collection.id, token_id), properties),
+		)
 	}
 
 	pub fn set_token_properties(
@@ -1465,31 +1406,19 @@
 		operator: &T::CrossAccountId,
 		approve: bool,
 	) -> DispatchResult {
-		if collection.permissions.access() == AccessMode::AllowList {
-			collection.check_allowlist(owner)?;
-			collection.check_allowlist(operator)?;
-		}
-
-		<PalletCommon<T>>::ensure_correct_receiver(operator)?;
-
-		// =========
-
-		<CollectionAllowance<T>>::insert((collection.id, owner, operator), approve);
-		<PalletEvm<T>>::deposit_log(
+		<PalletCommon<T>>::set_allowance_for_all(
+			collection,
+			owner,
+			operator,
+			approve,
+			|| <CollectionAllowance<T>>::insert((collection.id, owner, operator), approve),
 			ERC721Events::ApprovalForAll {
 				owner: *owner.as_eth(),
 				operator: *operator.as_eth(),
 				approved: approve,
 			}
 			.to_log(collection_id_to_address(collection.id)),
-		);
-		<PalletCommon<T>>::deposit_event(CommonEvent::ApprovedForAll(
-			collection.id,
-			owner.clone(),
-			operator.clone(),
-			approve,
-		));
-		Ok(())
+		)
 	}
 
 	/// Tells whether the given `owner` approves the `operator`.