From 44071b633884d321056113fbffbd312a7840dbdc Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Sat, 30 Sep 2023 15:52:25 +0000 Subject: [PATCH] refactor: use type-safe propertywriter to set/delete properties --- --- a/pallets/balances-adapter/src/common.rs +++ b/pallets/balances-adapter/src/common.rs @@ -172,6 +172,20 @@ fail!(>::UnsupportedOperation); } + fn get_token_properties_map(&self, _token_id: TokenId) -> up_data_structs::TokenProperties { + // No token properties are defined on fungibles + up_data_structs::TokenProperties::new() + } + + fn set_token_properties_map(&self, _token_id: TokenId, _map: up_data_structs::TokenProperties) { + // No token properties are defined on fungibles + } + + fn properties_exist(&self, _token: TokenId) -> bool { + // No token properties are defined on fungibles + false + } + fn set_token_property_permissions( &self, _sender: &::CrossAccountId, @@ -277,6 +291,15 @@ Err(up_data_structs::TokenOwnerError::MultipleOwners) } + fn check_token_indirect_owner( + &self, + _token: TokenId, + _maybe_owner: &::CrossAccountId, + _nesting_budget: &dyn up_data_structs::budget::Budget, + ) -> Result { + Ok(false) + } + fn token_owners(&self, _token: TokenId) -> Vec<::CrossAccountId> { vec![] } --- a/pallets/common/src/benchmarking.rs +++ b/pallets/common/src/benchmarking.rs @@ -22,8 +22,9 @@ use frame_benchmarking::{benchmarks, account}; use up_data_structs::{ CollectionMode, CreateCollectionData, CollectionId, Property, PropertyKey, PropertyValue, - CollectionPermissions, NestingPermissions, AccessMode, MAX_COLLECTION_NAME_LENGTH, - MAX_COLLECTION_DESCRIPTION_LENGTH, MAX_TOKEN_PREFIX_LENGTH, MAX_PROPERTIES_PER_ITEM, + CollectionPermissions, NestingPermissions, AccessMode, PropertiesPermissionMap, + MAX_COLLECTION_NAME_LENGTH, MAX_COLLECTION_DESCRIPTION_LENGTH, MAX_TOKEN_PREFIX_LENGTH, + MAX_PROPERTIES_PER_ITEM, }; use frame_support::{ traits::{Get, fungible::Balanced, Imbalance, tokens::Precision}, @@ -123,6 +124,16 @@ ) } +pub fn load_is_admin_and_property_permissions( + collection: &CollectionHandle, + sender: &T::CrossAccountId, +) -> (bool, PropertiesPermissionMap) { + ( + collection.is_owner_or_admin(sender), + >::property_permissions(collection.id), + ) +} + /// Helper macros, which handles all benchmarking preparation in semi-declarative way /// /// `name` is a substrate account @@ -215,4 +226,12 @@ assert_eq!(collection_handle.permissions.access(), AccessMode::AllowList); }: {collection_handle.check_allowlist(&sender)?;} + + init_token_properties_common { + bench_init!{ + owner: sub; collection: collection(owner); + sender: sub; + sender: cross_from_sub(sender); + }; + }: {load_is_admin_and_property_permissions(&collection, &sender);} } --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -56,6 +56,7 @@ use core::{ ops::{Deref, DerefMut}, slice::from_ref, + marker::PhantomData, }; use pallet_evm_coder_substrate::{SubstrateRecorder, WithRecorder}; use sp_std::vec::Vec; @@ -97,6 +98,9 @@ pub mod helpers; #[allow(missing_docs)] pub mod weights; + +use weights::WeightInfo; + /// Weight info. pub type SelfWeightOf = ::WeightInfo; @@ -864,19 +868,7 @@ QueryKind = OptionQuery, >; } - -/// Represents the change mode for the token property. -pub enum SetPropertyMode { - /// The token already exists. - ExistingToken, - /// New token. - NewToken { - /// The creator of the token is the recipient. - mint_target_is_sender: bool, - }, -} - /// Value representation with delayed initialization time. pub struct LazyValue T> { value: Option, @@ -892,19 +884,33 @@ } } - /// Get the value. If it call furst time the value will be initialized. + /// Get the value. If it is called the first time, the value will be initialized. pub fn value(&mut self) -> &T { - if self.value.is_none() { - self.value = Some(self.f.take().unwrap()()) - } + self.compute_value_if_not_already(); + self.value.as_ref().unwrap() + } - self.value.as_ref().unwrap() + /// Get the value. If it is called the first time, the value will be initialized. + pub fn value_mut(&mut self) -> &mut T { + self.compute_value_if_not_already(); + self.value.as_mut().unwrap() } - /// Is value initialized. + fn into_inner(mut self) -> T { + self.compute_value_if_not_already(); + self.value.unwrap() + } + + /// Is value initialized? pub fn has_value(&self) -> bool { self.value.is_some() } + + fn compute_value_if_not_already(&mut self) { + if self.value.is_none() { + self.value = Some(self.f.take().unwrap()()) + } + } } fn check_token_permissions( @@ -926,10 +932,19 @@ fail!(>::NoPermission); } - let token_certainly_exist = is_token_owner.has_value() && (*is_token_owner.value())?; - if !token_certainly_exist && !is_token_exist.value() { - fail!(>::TokenNotFound); + let token_exist_due_to_owner_check_success = + is_token_owner.has_value() && (*is_token_owner.value())?; + + // If the token owner check has occurred and succeeded, + // we know the token exists (otherwise, the owner check must fail). + if !token_exist_due_to_owner_check_success { + // If the token owner check didn't occur, + // we must check the token's existence ourselves. + if !is_token_exist.value() { + fail!(>::TokenNotFound); + } } + Ok(()) } @@ -1312,92 +1327,6 @@ 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 with the value provided `(, Some())` - /// * removes a property under the if the value is `None` `(, None)`. - /// - /// 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. - #[allow(clippy::too_many_arguments)] - pub fn modify_token_properties( - collection: &CollectionHandle, - sender: &T::CrossAccountId, - token_id: TokenId, - is_token_exist: &mut LazyValue, - properties_updates: impl Iterator)>, - mut stored_properties: TokenProperties, - is_token_owner: &mut LazyValue, FTO>, - set_token_properties: impl FnOnce(TokenProperties), - log: evm_coder::ethereum::Log, - ) -> DispatchResult - where - FTO: FnOnce() -> Result, - FTE: FnOnce() -> bool, - { - let mut is_collection_admin = LazyValue::new(|| collection.is_owner_or_admin(sender)); - let mut permissions = LazyValue::new(|| Self::property_permissions(collection.id)); - - let mut changed = false; - for (key, value) in properties_updates { - let permission = permissions - .value() - .get(&key) - .cloned() - .unwrap_or_else(PropertyPermission::none); - - let property_exists = stored_properties.get(&key).is_some(); - - match permission { - PropertyPermission { mutable: false, .. } if property_exists => { - return Err(>::NoPermission.into()); - } - - PropertyPermission { - collection_admin, - token_owner, - .. - } => check_token_permissions::( - collection_admin, - token_owner, - &mut is_collection_admin, - is_token_owner, - is_token_exist, - )?, - } - - match value { - Some(value) => { - stored_properties - .try_set(key.clone(), value) - .map_err(>::from)?; - - Self::deposit_event(Event::TokenPropertySet(collection.id, token_id, key)); - } - None => { - stored_properties.remove(&key).map_err(>::from)?; - - Self::deposit_event(Event::TokenPropertyDeleted(collection.id, token_id, key)); - } - } - - changed = true; - } - - if changed { - >::deposit_log(log); - 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. @@ -2166,6 +2095,22 @@ budget: &dyn Budget, ) -> DispatchResultWithPostInfo; + /// Get token properties raw map. + /// + /// * `token_id` - The token which properties are needed. + fn get_token_properties_map(&self, token_id: TokenId) -> TokenProperties; + + /// Set token properties raw map. + /// + /// * `token_id` - The token for which the properties are being set. + /// * `map` - The raw map containing the token's properties. + fn set_token_properties_map(&self, token_id: TokenId, map: TokenProperties); + + /// Whether the given token has properties. + /// + /// * `token_id` - The token in question. + fn properties_exist(&self, token: TokenId) -> bool; + /// Set token property permissions. /// /// * `sender` - Must be either the owner of the token or its admin. @@ -2309,6 +2254,18 @@ /// * `token` - The token for which you need to find out the owner. fn token_owner(&self, token: TokenId) -> Result; + /// Checks if the `maybe_owner` is the indirect owner of the `token`. + /// + /// * `token` - Id token to check. + /// * `maybe_owner` - The account to check. + /// * `nesting_budget` - A budget that can be spent on nesting tokens. + fn check_token_indirect_owner( + &self, + token: TokenId, + maybe_owner: &T::CrossAccountId, + nesting_budget: &dyn Budget, + ) -> Result; + /// Returns 10 tokens owners in no particular order. /// /// * `token` - The token for which you need to find out the owners. @@ -2420,6 +2377,348 @@ } } +/// A marker structure that enables the writer implementation +/// to provide the interface to write properties to **newly created** tokens. +pub struct NewTokenPropertyWriter; + +/// A marker structure that enables the writer implementation +/// to provide the interface to write properties to **already existing** tokens. +pub struct ExistingTokenPropertyWriter; + +/// The type-safe interface for writing properties (setting or deleting) to tokens. +/// It has two distinct implementations for newly created tokens and existing ones. +/// +/// This type utilizes the lazy evaluation to avoid repeating the computation +/// of several performance-heavy or PoV-heavy tasks, +/// such as checking the indirect ownership or reading the token property permissions. +pub struct PropertyWriter< + 'a, + T, + Handle, + WriterVariant, + FIsAdmin, + FPropertyPermissions, + FCheckTokenExist, + FGetProperties, +> where + T: Config, + FIsAdmin: FnOnce() -> bool, + FPropertyPermissions: FnOnce() -> PropertiesPermissionMap, +{ + collection: &'a Handle, + is_collection_admin: LazyValue, + property_permissions: LazyValue, + check_token_exist: FCheckTokenExist, + get_properties: FGetProperties, + _phantom: PhantomData<(T, WriterVariant)>, +} + +impl<'a, T, Handle, FIsAdmin, FPropertyPermissions, FCheckTokenExist, FGetProperties> + PropertyWriter< + 'a, + T, + Handle, + NewTokenPropertyWriter, + FIsAdmin, + FPropertyPermissions, + FCheckTokenExist, + FGetProperties, + > where + T: Config, + Handle: CommonCollectionOperations + Deref>, + FIsAdmin: FnOnce() -> bool, + FPropertyPermissions: FnOnce() -> PropertiesPermissionMap, + FCheckTokenExist: Copy + FnOnce(TokenId) -> bool, + FGetProperties: Copy + FnOnce(TokenId) -> TokenProperties, +{ + /// A function to write properties to a **newly created** token. + pub fn write_token_properties( + &mut self, + mint_target_is_sender: bool, + token_id: TokenId, + properties_updates: impl Iterator, + log: evm_coder::ethereum::Log, + ) -> DispatchResult { + self.internal_write_token_properties( + token_id, + properties_updates.map(|p| (p.key, Some(p.value))), + |_| Ok(mint_target_is_sender), + log, + ) + } +} + +impl<'a, T, Handle, FIsAdmin, FPropertyPermissions, FCheckTokenExist, FGetProperties> + PropertyWriter< + 'a, + T, + Handle, + ExistingTokenPropertyWriter, + FIsAdmin, + FPropertyPermissions, + FCheckTokenExist, + FGetProperties, + > where + T: Config, + Handle: CommonCollectionOperations + Deref>, + FIsAdmin: FnOnce() -> bool, + FPropertyPermissions: FnOnce() -> PropertiesPermissionMap, + FCheckTokenExist: Copy + FnOnce(TokenId) -> bool, + FGetProperties: Copy + FnOnce(TokenId) -> TokenProperties, +{ + /// A function to write properties to an **already existing** token. + pub fn write_token_properties( + &mut self, + sender: &T::CrossAccountId, + token_id: TokenId, + properties_updates: impl Iterator)>, + nesting_budget: &dyn Budget, + log: evm_coder::ethereum::Log, + ) -> DispatchResult { + self.internal_write_token_properties( + token_id, + properties_updates, + |collection| collection.check_token_indirect_owner(token_id, sender, nesting_budget), + log, + ) + } +} + +impl< + 'a, + T, + Handle, + WriterVariant, + FIsAdmin, + FPropertyPermissions, + FCheckTokenExist, + FGetProperties, + > + PropertyWriter< + 'a, + T, + Handle, + WriterVariant, + FIsAdmin, + FPropertyPermissions, + FCheckTokenExist, + FGetProperties, + > where + T: Config, + Handle: CommonCollectionOperations + Deref>, + FIsAdmin: FnOnce() -> bool, + FPropertyPermissions: FnOnce() -> PropertiesPermissionMap, + FCheckTokenExist: Copy + FnOnce(TokenId) -> bool, + FGetProperties: Copy + FnOnce(TokenId) -> TokenProperties, +{ + fn internal_write_token_properties( + &mut self, + token_id: TokenId, + properties_updates: impl Iterator)>, + check_token_owner: FCheckTokenOwner, + log: evm_coder::ethereum::Log, + ) -> DispatchResult + where + FCheckTokenOwner: FnOnce(&Handle) -> Result, + { + let get_properties = self.get_properties; + let mut stored_properties = LazyValue::new(move || get_properties(token_id)); + + let mut is_token_owner = LazyValue::new(|| check_token_owner(self.collection)); + + let check_token_exist = self.check_token_exist; + let mut is_token_exist = LazyValue::new(move || check_token_exist(token_id)); + + for (key, value) in properties_updates { + let permission = self + .property_permissions + .value() + .get(&key) + .cloned() + .unwrap_or_else(PropertyPermission::none); + + match permission { + PropertyPermission { mutable: false, .. } + if stored_properties.value().get(&key).is_some() => + { + return Err(>::NoPermission.into()); + } + + PropertyPermission { + collection_admin, + token_owner, + .. + } => check_token_permissions::( + collection_admin, + token_owner, + &mut self.is_collection_admin, + &mut is_token_owner, + &mut is_token_exist, + )?, + } + + match value { + Some(value) => { + stored_properties + .value_mut() + .try_set(key.clone(), value) + .map_err(>::from)?; + + >::deposit_event(Event::TokenPropertySet( + self.collection.id, + token_id, + key, + )); + } + None => { + stored_properties + .value_mut() + .remove(&key) + .map_err(>::from)?; + + >::deposit_event(Event::TokenPropertyDeleted( + self.collection.id, + token_id, + key, + )); + } + } + } + + let properties_changed = stored_properties.has_value(); + if properties_changed { + >::deposit_log(log); + + self.collection + .set_token_properties_map(token_id, stored_properties.into_inner()); + } + + Ok(()) + } +} + +/// Create a [`PropertyWriter`] for newly created tokens. +pub fn property_writer_for_new_token<'a, T, Handle>( + collection: &'a Handle, + sender: &'a T::CrossAccountId, +) -> PropertyWriter< + 'a, + T, + Handle, + NewTokenPropertyWriter, + impl FnOnce() -> bool + 'a, + impl FnOnce() -> PropertiesPermissionMap + 'a, + impl Copy + FnOnce(TokenId) -> bool + 'a, + impl Copy + FnOnce(TokenId) -> TokenProperties + 'a, +> +where + T: Config, + Handle: CommonCollectionOperations + Deref>, +{ + PropertyWriter { + collection, + is_collection_admin: LazyValue::new(|| collection.is_owner_or_admin(sender)), + property_permissions: LazyValue::new(|| >::property_permissions(collection.id)), + check_token_exist: |token_id| { + debug_assert!(collection.token_exists(token_id)); + true + }, + get_properties: |token_id| { + debug_assert!(!collection.properties_exist(token_id)); + TokenProperties::new() + }, + _phantom: PhantomData, + } +} + +#[cfg(feature = "runtime-benchmarks")] +/// Create a `PropertyWriter` with preloaded `is_collection_admin` and `property_permissions. +/// Also: +/// * it will return `true` for the token ownership check. +/// * it will return empty stored properties without reading them from the storage. +pub fn collection_info_loaded_property_writer( + collection: &Handle, + is_collection_admin: bool, + property_permissions: PropertiesPermissionMap, +) -> PropertyWriter< + T, + Handle, + NewTokenPropertyWriter, + impl FnOnce() -> bool, + impl FnOnce() -> PropertiesPermissionMap, + impl Copy + FnOnce(TokenId) -> bool, + impl Copy + FnOnce(TokenId) -> TokenProperties, +> +where + T: Config, + Handle: CommonCollectionOperations + Deref>, +{ + PropertyWriter { + collection, + is_collection_admin: LazyValue::new(move || is_collection_admin), + property_permissions: LazyValue::new(move || property_permissions), + check_token_exist: |_token_id| true, + get_properties: |_token_id| TokenProperties::new(), + _phantom: PhantomData, + } +} + +/// Create a [`PropertyWriter`] for already existing tokens. +pub fn property_writer_for_existing_token<'a, T, Handle>( + collection: &'a Handle, + sender: &'a T::CrossAccountId, +) -> PropertyWriter< + 'a, + T, + Handle, + ExistingTokenPropertyWriter, + impl FnOnce() -> bool + 'a, + impl FnOnce() -> PropertiesPermissionMap + 'a, + impl Copy + FnOnce(TokenId) -> bool + 'a, + impl Copy + FnOnce(TokenId) -> TokenProperties + 'a, +> +where + T: Config, + Handle: CommonCollectionOperations + Deref>, +{ + PropertyWriter { + collection, + is_collection_admin: LazyValue::new(|| collection.is_owner_or_admin(sender)), + property_permissions: LazyValue::new(|| >::property_permissions(collection.id)), + check_token_exist: |token_id| collection.token_exists(token_id), + get_properties: |token_id| collection.get_token_properties_map(token_id), + _phantom: PhantomData, + } +} + +/// Computes the weight delta for newly created tokens with properties. +/// * `properties_nums` - The properties num of each created token. +/// * `init_token_properties` - The function to obtain the weight from a token's properties num. +pub fn init_token_properties_delta Weight>( + properties_nums: impl Iterator, + init_token_properties: I, +) -> Weight { + let mut delta = properties_nums + .filter_map(|properties_num| { + if properties_num > 0 { + Some(init_token_properties(properties_num)) + } else { + None + } + }) + .fold(Weight::zero(), |a, b| a.saturating_add(b)); + + // If at least once the `init_token_properties` was called, + // it means at least one newly created token has properties. + // Becuase of that, some common collection data also was loaded and we need to add this weight. + // However, these common data was loaded only once which is guaranteed by the `PropertyWriter`. + if !delta.is_zero() { + delta = delta.saturating_add(>::init_token_properties_common()) + } + + delta +} + #[cfg(any(feature = "tests", test))] #[allow(missing_docs)] pub mod tests { --- a/pallets/fungible/src/common.rs +++ b/pallets/fungible/src/common.rs @@ -25,7 +25,7 @@ weights::WeightInfo as _, SelfWeightOf as PalletCommonWeightOf, }; use pallet_structure::Error as StructureError; -use sp_runtime::ArithmeticError; +use sp_runtime::{ArithmeticError, DispatchError}; use sp_std::{vec::Vec, vec}; use up_data_structs::{Property, PropertyKey, PropertyValue, PropertyKeyPermission}; @@ -364,6 +364,20 @@ fail!(>::SettingPropertiesNotAllowed) } + fn get_token_properties_map(&self, _token_id: TokenId) -> up_data_structs::TokenProperties { + // No token properties are defined on fungibles + up_data_structs::TokenProperties::new() + } + + fn set_token_properties_map(&self, _token_id: TokenId, _map: up_data_structs::TokenProperties) { + // No token properties are defined on fungibles + } + + fn properties_exist(&self, _token: TokenId) -> bool { + // No token properties are defined on fungibles + false + } + fn check_nesting( &self, _sender: ::CrossAccountId, @@ -402,6 +416,15 @@ Err(TokenOwnerError::MultipleOwners) } + fn check_token_indirect_owner( + &self, + _token: TokenId, + _maybe_owner: &T::CrossAccountId, + _nesting_budget: &dyn Budget, + ) -> Result { + Ok(false) + } + /// Returns 10 tokens owners in no particular order. fn token_owners(&self, token: TokenId) -> Vec { >::token_owners(self.id, token).unwrap_or_default() --- a/pallets/nonfungible/src/benchmarking.rs +++ b/pallets/nonfungible/src/benchmarking.rs @@ -20,7 +20,9 @@ use frame_benchmarking::{benchmarks, account}; use pallet_common::{ bench_init, - benchmarking::{create_collection_raw, property_key, property_value}, + benchmarking::{ + create_collection_raw, property_key, property_value, load_is_admin_and_property_permissions, + }, CommonCollectionOperations, }; use sp_std::prelude::*; @@ -198,14 +200,15 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, owner.clone())?; - }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::ExistingToken, &Unlimited)?} + }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), &Unlimited)?} - reset_token_properties { + init_token_properties { let b in 0..MAX_PROPERTIES_PER_ITEM; bench_init!{ owner: sub; collection: collection(owner); owner: cross_from_sub; }; + let perms = (0..b).map(|k| PropertyKeyPermission { key: property_key(k as usize), permission: PropertyPermission { @@ -220,8 +223,26 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, owner.clone())?; - }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::NewToken { mint_target_is_sender: true }, &Unlimited)?} + let (is_collection_admin, property_permissions) = load_is_admin_and_property_permissions(&collection, &owner); + }: { + let mut property_writer = pallet_common::collection_info_loaded_property_writer( + &collection, + is_collection_admin, + property_permissions, + ); + + property_writer.write_token_properties( + true, + item, + props.into_iter(), + crate::erc::ERC721TokenEvent::TokenChanged { + token_id: item.into(), + } + .to_log(T::ContractAddress::get()), + )? + } + delete_token_properties { let b in 0..MAX_PROPERTIES_PER_ITEM; bench_init!{ @@ -242,7 +263,7 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, owner.clone())?; - >::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::ExistingToken, &Unlimited)?; + >::set_token_properties(&collection, &owner, item, props.into_iter(), &Unlimited)?; let to_delete = (0..b).map(|k| property_key(k as usize)).collect::>(); }: {>::delete_token_properties(&collection, &owner, item, to_delete.into_iter(), &Unlimited)?} --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -23,49 +23,40 @@ }; use pallet_common::{ CommonCollectionOperations, CommonWeightInfo, RefungibleExtensions, with_weight, - weights::WeightInfo as _, SelfWeightOf as PalletCommonWeightOf, + weights::WeightInfo as _, SelfWeightOf as PalletCommonWeightOf, init_token_properties_delta, }; +use pallet_structure::Pallet as PalletStructure; use sp_runtime::DispatchError; use sp_std::{vec::Vec, vec}; use crate::{ AccountBalance, Allowance, Config, CreateItemData, Error, NonfungibleHandle, Owned, Pallet, - SelfWeightOf, TokenData, weights::WeightInfo, TokensMinted, + SelfWeightOf, TokenData, weights::WeightInfo, TokensMinted, TokenProperties, }; pub struct CommonWeights(PhantomData); impl CommonWeightInfo for CommonWeights { fn create_multiple_items_ex(data: &CreateItemExData) -> Weight { match data { - CreateItemExData::NFT(t) => { - >::create_multiple_items_ex(t.len() as u32) - + t.iter() - .filter_map(|t| { - if t.properties.len() > 0 { - Some(>::reset_token_properties( - t.properties.len() as u32, - )) - } else { - None - } - }) - .fold(Weight::zero(), |a, b| a.saturating_add(b)) - } + CreateItemExData::NFT(t) => >::create_multiple_items_ex(t.len() as u32) + .saturating_add(init_token_properties_delta::( + t.iter().map(|t| t.properties.len() as u32), + >::init_token_properties, + )), _ => Weight::zero(), } } fn create_multiple_items(data: &[up_data_structs::CreateItemData]) -> Weight { - >::create_multiple_items(data.len() as u32) - + data - .iter() - .filter_map(|t| match t { - up_data_structs::CreateItemData::NFT(n) if n.properties.len() > 0 => Some( - >::reset_token_properties(n.properties.len() as u32), - ), - _ => None, - }) - .fold(Weight::zero(), |a, b| a.saturating_add(b)) + >::create_multiple_items(data.len() as u32).saturating_add( + init_token_properties_delta::( + data.iter().map(|t| match t { + up_data_structs::CreateItemData::NFT(n) => n.properties.len() as u32, + _ => 0, + }), + >::init_token_properties, + ), + ) } fn burn_item() -> Weight { @@ -247,7 +238,6 @@ &sender, token_id, properties.into_iter(), - pallet_common::SetPropertyMode::ExistingToken, nesting_budget, ), weight, @@ -275,6 +265,14 @@ ) } + fn get_token_properties_map(&self, token_id: TokenId) -> up_data_structs::TokenProperties { + >::get((self.id, token_id)) + } + + fn set_token_properties_map(&self, token_id: TokenId, map: up_data_structs::TokenProperties) { + >::set((self.id, token_id), map) + } + fn set_token_property_permissions( &self, sender: &T::CrossAccountId, @@ -289,6 +287,10 @@ ) } + fn properties_exist(&self, token: TokenId) -> bool { + >::contains_key((self.id, token)) + } + fn burn_item( &self, sender: T::CrossAccountId, @@ -459,6 +461,21 @@ .ok_or(TokenOwnerError::NotFound) } + fn check_token_indirect_owner( + &self, + token: TokenId, + maybe_owner: &T::CrossAccountId, + nesting_budget: &dyn Budget, + ) -> Result { + >::check_indirectly_owned( + maybe_owner.clone(), + self.id, + token, + None, + nesting_budget, + ) + } + /// Returns token owners. fn token_owners(&self, token: TokenId) -> Vec { self.token_owner(token).map_or_else(|_| vec![], |t| vec![t]) --- a/pallets/nonfungible/src/erc.rs +++ b/pallets/nonfungible/src/erc.rs @@ -203,7 +203,6 @@ &caller, TokenId(token_id), properties.into_iter(), - pallet_common::SetPropertyMode::ExistingToken, &nesting_budget, ) .map_err(dispatch_to_evm::) --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -109,7 +109,7 @@ use pallet_common::{ Error as CommonError, Pallet as PalletCommon, Event as CommonEvent, CollectionHandle, eth::collection_id_to_address, SelfWeightOf as PalletCommonWeightOf, - weights::WeightInfo as CommonWeightInfo, helpers::add_weight_to_post_info, SetPropertyMode, + weights::WeightInfo as CommonWeightInfo, helpers::add_weight_to_post_info, }; use pallet_structure::{Pallet as PalletStructure, Error as StructureError}; use pallet_evm_coder_substrate::{SubstrateRecorder, WithRecorder}; @@ -598,58 +598,16 @@ sender: &T::CrossAccountId, token_id: TokenId, properties_updates: impl Iterator)>, - mode: SetPropertyMode, nesting_budget: &dyn Budget, ) -> DispatchResult { - let mut is_token_owner = pallet_common::LazyValue::new(|| { - if let SetPropertyMode::NewToken { - mint_target_is_sender, - } = mode - { - return Ok(mint_target_is_sender); - } - - let is_owned = >::check_indirectly_owned( - sender.clone(), - collection.id, - token_id, - None, - nesting_budget, - )?; - - Ok(is_owned) - }); - - let is_new_token = matches!(mode, SetPropertyMode::NewToken { .. }); - - let mut is_token_exist = pallet_common::LazyValue::new(|| { - if is_new_token { - debug_assert!(Self::token_exists(collection, token_id)); - true - } else { - Self::token_exists(collection, token_id) - } - }); - - let stored_properties = if is_new_token { - debug_assert!(!>::contains_key(( - collection.id, - token_id - ))); - TokenPropertiesT::new() - } else { - >::get((collection.id, token_id)) - }; + let mut property_writer = + pallet_common::property_writer_for_existing_token(collection, sender); - >::modify_token_properties( - collection, + property_writer.write_token_properties( sender, token_id, - &mut is_token_exist, properties_updates, - stored_properties, - &mut is_token_owner, - |properties| >::set((collection.id, token_id), properties), + nesting_budget, erc::ERC721TokenEvent::TokenChanged { token_id: token_id.into(), } @@ -680,7 +638,6 @@ sender: &T::CrossAccountId, token_id: TokenId, properties: impl Iterator, - mode: SetPropertyMode, nesting_budget: &dyn Budget, ) -> DispatchResult { Self::modify_token_properties( @@ -688,7 +645,6 @@ sender, token_id, properties.map(|p| (p.key, Some(p.value))), - mode, nesting_budget, ) } @@ -710,7 +666,6 @@ sender, token_id, [property].into_iter(), - SetPropertyMode::ExistingToken, nesting_budget, ) } @@ -732,7 +687,6 @@ sender, token_id, property_keys.into_iter().map(|key| (key, None)), - SetPropertyMode::ExistingToken, nesting_budget, ) } @@ -994,6 +948,8 @@ // ========= + let mut property_writer = pallet_common::property_writer_for_new_token(collection, sender); + with_transaction(|| { for (i, data) in data.iter().enumerate() { let token = first_token + i as u32 + 1; @@ -1006,21 +962,22 @@ }, ); + let token = TokenId(token); + >::nest_if_sent_to_token_unchecked( &data.owner, collection.id, - TokenId(token), + token, ); - if let Err(e) = Self::set_token_properties( - collection, - sender, - TokenId(token), + if let Err(e) = property_writer.write_token_properties( + sender.conv_eq(&data.owner), + token, data.properties.clone().into_iter(), - SetPropertyMode::NewToken { - mint_target_is_sender: sender.conv_eq(&data.owner), - }, - nesting_budget, + erc::ERC721TokenEvent::TokenChanged { + token_id: token.into(), + } + .to_log(T::ContractAddress::get()), ) { return TransactionOutcome::Rollback(Err(e)); } --- a/pallets/refungible/src/benchmarking.rs +++ b/pallets/refungible/src/benchmarking.rs @@ -22,7 +22,9 @@ use frame_benchmarking::{benchmarks, account}; use pallet_common::{ bench_init, - benchmarking::{create_collection_raw, property_key, property_value}, + benchmarking::{ + create_collection_raw, property_key, property_value, load_is_admin_and_property_permissions, + }, }; use sp_std::prelude::*; use up_data_structs::{ @@ -255,14 +257,15 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, [(owner.clone(), 200)])?; - }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::ExistingToken, &Unlimited)?} + }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), &Unlimited)?} - reset_token_properties { + init_token_properties { let b in 0..MAX_PROPERTIES_PER_ITEM; bench_init!{ owner: sub; collection: collection(owner); owner: cross_from_sub; }; + let perms = (0..b).map(|k| PropertyKeyPermission { key: property_key(k as usize), permission: PropertyPermission { @@ -277,8 +280,26 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, [(owner.clone(), 200)])?; - }: {>::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::NewToken { mint_target_is_sender: true }, &Unlimited)?} + let (is_collection_admin, property_permissions) = load_is_admin_and_property_permissions(&collection, &owner); + }: { + let mut property_writer = pallet_common::collection_info_loaded_property_writer( + &collection, + is_collection_admin, + property_permissions, + ); + + property_writer.write_token_properties( + true, + item, + props.into_iter(), + crate::erc::ERC721TokenEvent::TokenChanged { + token_id: item.into(), + } + .to_log(T::ContractAddress::get()), + )? + } + delete_token_properties { let b in 0..MAX_PROPERTIES_PER_ITEM; bench_init!{ @@ -299,7 +320,7 @@ value: property_value(), }).collect::>(); let item = create_max_item(&collection, &owner, [(owner.clone(), 200)])?; - >::set_token_properties(&collection, &owner, item, props.into_iter(), SetPropertyMode::ExistingToken, &Unlimited)?; + >::set_token_properties(&collection, &owner, item, props.into_iter(), &Unlimited)?; let to_delete = (0..b).map(|k| property_key(k as usize)).collect::>(); }: {>::delete_token_properties(&collection, &owner, item, to_delete.into_iter(), &Unlimited)?} --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -20,20 +20,20 @@ use frame_support::{dispatch::DispatchResultWithPostInfo, ensure, fail, weights::Weight, traits::Get}; use up_data_structs::{ CollectionId, TokenId, CreateItemExData, budget::Budget, Property, PropertyKey, PropertyValue, - PropertyKeyPermission, CollectionPropertiesVec, CreateRefungibleExMultipleOwners, - CreateRefungibleExSingleOwner, TokenOwnerError, + PropertyKeyPermission, CreateRefungibleExMultipleOwners, CreateRefungibleExSingleOwner, + TokenOwnerError, }; use pallet_common::{ CommonCollectionOperations, CommonWeightInfo, RefungibleExtensions, with_weight, - weights::WeightInfo as _, + weights::WeightInfo as _, init_token_properties_delta, }; -use pallet_structure::Error as StructureError; +use pallet_structure::{Pallet as PalletStructure, Error as StructureError}; use sp_runtime::{DispatchError}; use sp_std::{vec::Vec, vec}; use crate::{ AccountBalance, Allowance, Balance, Config, Error, Owned, Pallet, RefungibleHandle, - SelfWeightOf, weights::WeightInfo, TokensMinted, TotalSupply, CreateItemData, + SelfWeightOf, weights::WeightInfo, TokensMinted, TotalSupply, CreateItemData, TokenProperties, }; macro_rules! max_weight_of { @@ -45,26 +45,19 @@ }; } -fn properties_weight(properties: &CollectionPropertiesVec) -> Weight { - if properties.len() > 0 { - >::reset_token_properties(properties.len() as u32) - } else { - Weight::zero() - } -} - pub struct CommonWeights(PhantomData); impl CommonWeightInfo for CommonWeights { fn create_multiple_items(data: &[up_data_structs::CreateItemData]) -> Weight { >::create_multiple_items(data.len() as u32).saturating_add( - data.iter() - .map(|data| match data { + init_token_properties_delta::( + data.iter().map(|data| match data { up_data_structs::CreateItemData::ReFungible(rft_data) => { - properties_weight::(&rft_data.properties) + rft_data.properties.len() as u32 } - _ => Weight::zero(), - }) - .fold(Weight::zero(), |a, b| a.saturating_add(b)), + _ => 0, + }), + >::init_token_properties, + ), ) } @@ -72,15 +65,17 @@ match call { CreateItemExData::RefungibleMultipleOwners(i) => { >::create_multiple_items_ex_multiple_owners(i.users.len() as u32) - .saturating_add(properties_weight::(&i.properties)) + .saturating_add(init_token_properties_delta::( + [i.properties.len() as u32].into_iter(), + >::init_token_properties, + )) } CreateItemExData::RefungibleMultipleItems(i) => { >::create_multiple_items_ex_multiple_items(i.len() as u32) - .saturating_add( - i.iter() - .map(|d| properties_weight::(&d.properties)) - .fold(Weight::zero(), |a, b| a.saturating_add(b)), - ) + .saturating_add(init_token_properties_delta::( + i.iter().map(|d| d.properties.len() as u32), + >::init_token_properties, + )) } _ => Weight::zero(), } @@ -399,7 +394,6 @@ &sender, token_id, properties.into_iter(), - pallet_common::SetPropertyMode::ExistingToken, nesting_budget, ), weight, @@ -441,6 +435,18 @@ ) } + fn get_token_properties_map(&self, token_id: TokenId) -> up_data_structs::TokenProperties { + >::get((self.id, token_id)) + } + + fn set_token_properties_map(&self, token_id: TokenId, map: up_data_structs::TokenProperties) { + >::set((self.id, token_id), map) + } + + fn properties_exist(&self, token: TokenId) -> bool { + >::contains_key((self.id, token)) + } + fn check_nesting( &self, _sender: ::CrossAccountId, @@ -479,6 +485,29 @@ >::token_owner(self.id, token) } + fn check_token_indirect_owner( + &self, + token: TokenId, + maybe_owner: &T::CrossAccountId, + nesting_budget: &dyn Budget, + ) -> Result { + let balance = self.balance(maybe_owner.clone(), token); + let total_pieces: u128 = >::total_pieces(self.id, token).unwrap_or(u128::MAX); + if balance != total_pieces { + return Ok(false); + } + + let is_bundle_owner = >::check_indirectly_owned( + maybe_owner.clone(), + self.id, + token, + None, + nesting_budget, + )?; + + Ok(is_bundle_owner) + } + /// Returns 10 token in no particular order. fn token_owners(&self, token: TokenId) -> Vec { >::token_owners(self.id, token).unwrap_or_default() --- a/pallets/refungible/src/erc.rs +++ b/pallets/refungible/src/erc.rs @@ -214,7 +214,6 @@ &caller, TokenId(token_id), properties.into_iter(), - pallet_common::SetPropertyMode::ExistingToken, &nesting_budget, ) .map_err(dispatch_to_evm::) --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -96,8 +96,8 @@ 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, SetPropertyMode, + Error as CommonError, eth::collection_id_to_address, Event as CommonEvent, + Pallet as PalletCommon, }; use pallet_structure::Pallet as PalletStructure; use sp_core::{Get, H160}; @@ -533,66 +533,16 @@ sender: &T::CrossAccountId, token_id: TokenId, properties_updates: impl Iterator)>, - mode: SetPropertyMode, nesting_budget: &dyn Budget, ) -> DispatchResult { - let mut is_token_owner = - pallet_common::LazyValue::new(|| -> Result { - if let SetPropertyMode::NewToken { - mint_target_is_sender, - } = mode - { - return Ok(mint_target_is_sender); - } - - 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); - } - - let is_bundle_owner = >::check_indirectly_owned( - sender.clone(), - collection.id, - token_id, - None, - nesting_budget, - )?; - - Ok(is_bundle_owner) - }); - - let is_new_token = matches!(mode, SetPropertyMode::NewToken { .. }); - - let mut is_token_exist = pallet_common::LazyValue::new(|| { - if is_new_token { - debug_assert!(Self::token_exists(collection, token_id)); - true - } else { - Self::token_exists(collection, token_id) - } - }); - - let stored_properties = if is_new_token { - debug_assert!(!>::contains_key(( - collection.id, - token_id - ))); - TokenPropertiesT::new() - } else { - >::get((collection.id, token_id)) - }; + let mut property_writer = + pallet_common::property_writer_for_existing_token(collection, sender); - >::modify_token_properties( - collection, + property_writer.write_token_properties( sender, token_id, - &mut is_token_exist, properties_updates, - stored_properties, - &mut is_token_owner, - |properties| >::set((collection.id, token_id), properties), + nesting_budget, erc::ERC721TokenEvent::TokenChanged { token_id: token_id.into(), } @@ -618,7 +568,6 @@ sender: &T::CrossAccountId, token_id: TokenId, properties: impl Iterator, - mode: SetPropertyMode, nesting_budget: &dyn Budget, ) -> DispatchResult { Self::modify_token_properties( @@ -626,7 +575,6 @@ sender, token_id, properties.map(|p| (p.key, Some(p.value))), - mode, nesting_budget, ) } @@ -643,7 +591,6 @@ sender, token_id, [property].into_iter(), - SetPropertyMode::ExistingToken, nesting_budget, ) } @@ -660,7 +607,6 @@ sender, token_id, property_keys.into_iter().map(|key| (key, None)), - SetPropertyMode::ExistingToken, nesting_budget, ) } @@ -941,11 +887,15 @@ // ========= + let mut property_writer = pallet_common::property_writer_for_new_token(collection, sender); + with_transaction(|| { for (i, data) in data.iter().enumerate() { let token_id = first_token_id + i as u32 + 1; >::insert((collection.id, token_id), totals[i]); + let token = TokenId(token_id); + let mut mint_target_is_sender = true; for (user, amount) in data.users.iter() { if *amount == 0 { @@ -955,23 +905,22 @@ mint_target_is_sender = mint_target_is_sender && sender.conv_eq(user); >::insert((collection.id, token_id, &user), amount); - >::insert((collection.id, &user, TokenId(token_id)), true); + >::insert((collection.id, &user, token), true); >::nest_if_sent_to_token_unchecked( user, collection.id, - TokenId(token_id), + token, ); } - if let Err(e) = Self::set_token_properties( - collection, - sender, - TokenId(token_id), + if let Err(e) = property_writer.write_token_properties( + mint_target_is_sender, + token, data.properties.clone().into_iter(), - SetPropertyMode::NewToken { - mint_target_is_sender, - }, - nesting_budget, + erc::ERC721TokenEvent::TokenChanged { + token_id: token.into(), + } + .to_log(T::ContractAddress::get()), ) { return TransactionOutcome::Rollback(Err(e)); } -- gitstuff