From 8ccb2682a57d013ccbd865ebe3edf50ce519441a Mon Sep 17 00:00:00 2001 From: Yaroslav Bolyukin Date: Thu, 04 Nov 2021 14:25:22 +0000 Subject: [PATCH] refactor: make collection limits fields optional --- --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -105,10 +105,10 @@ Ok(()) } pub fn ignores_allowance(&self, user: &T::CrossAccountId) -> Result { - Ok(self.limits.owner_can_transfer && self.is_owner_or_admin(user)?) + Ok(self.limits.owner_can_transfer() && self.is_owner_or_admin(user)?) } pub fn ignores_owned_amount(&self, user: &T::CrossAccountId) -> Result { - Ok(self.limits.owner_can_transfer && self.is_owner_or_admin(user)?) + Ok(self.limits.owner_can_transfer() && self.is_owner_or_admin(user)?) } pub fn check_allowlist(&self, user: &T::CrossAccountId) -> DispatchResult { self.consume_sload()?; @@ -405,9 +405,10 @@ collection: CollectionHandle, sender: &T::CrossAccountId, ) -> DispatchResult { - if !collection.limits.owner_can_destroy { - fail!(Error::::NoPermission); - } + ensure!( + collection.limits.owner_can_destroy(), + >::NoPermission, + ); collection.check_is_owner(&sender)?; let destroyed_collections = >::get() --- a/pallets/fungible/src/lib.rs +++ b/pallets/fungible/src/lib.rs @@ -157,8 +157,8 @@ amount: u128, ) -> DispatchResult { ensure!( - collection.transfers_enabled, - >::TransferNotAllowed + collection.limits.transfers_enabled(), + >::TransferNotAllowed, ); if collection.access == AccessMode::WhiteList { --- a/pallets/nft/src/eth/sponsoring.rs +++ b/pallets/nft/src/eth/sponsoring.rs @@ -43,11 +43,8 @@ let token_id: u32 = token_id.try_into().map_err(|_| AnyError)?; let block_number = >::block_number() as T::BlockNumber; let collection_limits = &collection.limits; - let limit: u32 = if collection_limits.sponsor_transfer_timeout > 0 { - collection_limits.sponsor_transfer_timeout - } else { - NFT_SPONSOR_TRANSFER_TIMEOUT - }; + let limit = + collection_limits.sponsor_transfer_timeout(NFT_SPONSOR_TRANSFER_TIMEOUT); let mut sponsor = true; if >::contains_key(collection_id, token_id) { @@ -74,11 +71,8 @@ UniqueFungibleCall::ERC20(ERC20Call::Transfer { .. }) => { let who = T::CrossAccountId::from_eth(*caller); let collection_limits = &collection.limits; - let limit: u32 = if collection_limits.sponsor_transfer_timeout > 0 { - collection_limits.sponsor_transfer_timeout - } else { - FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT - }; + let limit = collection_limits + .sponsor_transfer_timeout(FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT); let block_number = >::block_number() as T::BlockNumber; let mut sponsored = true; --- a/pallets/nft/src/lib.rs +++ b/pallets/nft/src/lib.rs @@ -37,8 +37,9 @@ use nft_data_structs::{ MAX_DECIMAL_POINTS, MAX_SPONSOR_TIMEOUT, MAX_TOKEN_OWNERSHIP, CUSTOM_DATA_LIMIT, VARIABLE_ON_CHAIN_SCHEMA_LIMIT, CONST_ON_CHAIN_SCHEMA_LIMIT, COLLECTION_ADMINS_LIMIT, - OFFCHAIN_SCHEMA_LIMIT, AccessMode, Collection, CreateItemData, CollectionLimits, CollectionId, - CollectionMode, TokenId, SchemaVersion, SponsorshipState, MetaUpdatePermission, + OFFCHAIN_SCHEMA_LIMIT, FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, + NFT_SPONSOR_TRANSFER_TIMEOUT, AccessMode, Collection, CreateItemData, CollectionLimits, + CollectionId, CollectionMode, TokenId, SchemaVersion, SponsorshipState, MetaUpdatePermission, }; use pallet_common::{ account::CrossAccountId, CollectionHandle, IsAdmin, Pallet as PalletCommon, @@ -188,11 +189,6 @@ // Anyone can create a collection let who = ensure_signed(origin)?; - - let limits = CollectionLimits:: { - sponsored_data_size: CUSTOM_DATA_LIMIT, - ..Default::default() - }; // Create new collection let new_collection = Collection:: { @@ -208,8 +204,7 @@ sponsorship: SponsorshipState::Disabled, variable_on_chain_schema: Vec::new(), const_on_chain_schema: Vec::new(), - limits, - transfers_enabled: true, + limits: Default::default(), meta_update_permission: Default::default(), }; @@ -582,7 +577,7 @@ // ========= - target_collection.transfers_enabled = value; + target_collection.limits.transfers_enabled = Some(value); target_collection.save() } @@ -888,30 +883,63 @@ pub fn set_collection_limits( origin, collection_id: CollectionId, - new_limits: CollectionLimits, + new_limit: CollectionLimits, ) -> DispatchResult { + let mut new_limit = new_limit; let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; target_collection.check_is_owner(&sender)?; - let old_limits = &target_collection.limits; + let old_limit = &target_collection.limits; - // collection bounds - ensure!(new_limits.sponsor_transfer_timeout <= MAX_SPONSOR_TIMEOUT && - new_limits.account_token_ownership_limit.unwrap_or(0) <= MAX_TOKEN_OWNERSHIP && - new_limits.sponsored_data_size <= CUSTOM_DATA_LIMIT, - Error::::CollectionLimitBoundsExceeded); + macro_rules! limit_default { + ($old:ident, $new:ident, $($field:ident $(($arg:expr))? => $check:expr),* $(,)?) => {{ + $( + if let Some($new) = $new.$field { + let $old = $old.$field($($arg)?); + let _ = $new; + let _ = $old; + $check + } else { + $new.$field = $old.$field + } + )* + }}; + } - // token_limit check prev - ensure!(old_limits.token_limit >= new_limits.token_limit, >::CollectionTokenLimitExceeded); - ensure!(new_limits.token_limit > 0, >::CollectionTokenLimitExceeded); - - ensure!( - (old_limits.owner_can_transfer || !new_limits.owner_can_transfer) && - (old_limits.owner_can_destroy || !new_limits.owner_can_destroy), - Error::::OwnerPermissionsCantBeReverted, + limit_default!(old_limit, new_limit, + account_token_ownership_limit => ensure!( + new_limit <= MAX_TOKEN_OWNERSHIP, + >::CollectionLimitBoundsExceeded, + ), + sponsor_transfer_timeout(match target_collection.mode { + CollectionMode::NFT => NFT_SPONSOR_TRANSFER_TIMEOUT, + CollectionMode::Fungible(_) => FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, + CollectionMode::ReFungible => REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, + }) => ensure!( + new_limit <= MAX_SPONSOR_TIMEOUT, + >::CollectionLimitBoundsExceeded, + ), + sponsored_data_size => ensure!( + new_limit <= CUSTOM_DATA_LIMIT, + >::CollectionLimitBoundsExceeded, + ), + token_limit => ensure!( + old_limit >= new_limit && new_limit > 0, + >::CollectionTokenLimitExceeded + ), + owner_can_transfer => ensure!( + old_limit || !new_limit, + >::OwnerPermissionsCantBeReverted, + ), + owner_can_destroy => ensure!( + old_limit || !new_limit, + >::OwnerPermissionsCantBeReverted, + ), + sponsored_data_rate_limit => {}, + transfers_enabled => {}, ); - target_collection.limits = new_limits; + target_collection.limits = new_limit; target_collection.save() } --- a/pallets/nft/src/sponsorship.rs +++ b/pallets/nft/src/sponsorship.rs @@ -26,7 +26,13 @@ // sponsor timeout let block_number = >::block_number() as T::BlockNumber; - let limit = collection.limits.sponsor_transfer_timeout; + let limit = collection + .limits + .sponsor_transfer_timeout(match _properties { + CreateItemData::NFT(_) => NFT_SPONSOR_TRANSFER_TIMEOUT, + CreateItemData::Fungible(_) => FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, + CreateItemData::ReFungible(_) => REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT, + }); if CreateItemBasket::::contains_key((collection_id, &who)) { let last_tx_block = CreateItemBasket::::get((collection_id, &who)); let limit_time = last_tx_block + limit.into(); @@ -37,7 +43,7 @@ CreateItemBasket::::insert((collection_id, who.clone()), block_number); // check free create limit - if collection.limits.sponsored_data_size >= (_properties.data_size() as u32) { + if collection.limits.sponsored_data_size() >= (_properties.data_size() as u32) { collection.sponsorship.sponsor().cloned() } else { None @@ -61,11 +67,8 @@ sponsor_transfer = match collection_mode { CollectionMode::NFT => { // get correct limit - let limit: u32 = if collection_limits.sponsor_transfer_timeout > 0 { - collection_limits.sponsor_transfer_timeout - } else { - NFT_SPONSOR_TRANSFER_TIMEOUT - }; + let limit = + collection_limits.sponsor_transfer_timeout(NFT_SPONSOR_TRANSFER_TIMEOUT); let mut sponsored = true; if NftTransferBasket::::contains_key(collection_id, item_id) { @@ -83,11 +86,8 @@ } CollectionMode::Fungible(_) => { // get correct limit - let limit: u32 = if collection_limits.sponsor_transfer_timeout > 0 { - collection_limits.sponsor_transfer_timeout - } else { - FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT - }; + let limit = collection_limits + .sponsor_transfer_timeout(FUNGIBLE_SPONSOR_TRANSFER_TIMEOUT); let block_number = >::block_number() as T::BlockNumber; let mut sponsored = true; @@ -106,11 +106,8 @@ } CollectionMode::ReFungible => { // get correct limit - let limit: u32 = if collection_limits.sponsor_transfer_timeout > 0 { - collection_limits.sponsor_transfer_timeout - } else { - REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT - }; + let limit = collection_limits + .sponsor_transfer_timeout(REFUNGIBLE_SPONSOR_TRANSFER_TIMEOUT); let mut sponsored = true; if ReFungibleTransferBasket::::contains_key(collection_id, item_id) { @@ -150,13 +147,13 @@ // Can't sponsor fungible collection, this tx will be rejected // as invalid !matches!(collection.mode, CollectionMode::Fungible(_)) && - data.len() <= collection.limits.sponsored_data_size as usize + data.len() <= collection.limits.sponsored_data_size() as usize { - if let Some(rate_limit) = collection.limits.sponsored_data_rate_limit { + if let Some(rate_limit) = collection.limits.sponsored_data_rate_limit() { let block_number = >::block_number() as T::BlockNumber; if VariableMetaDataBasket::::get(collection_id, item_id) - .map(|last_block| block_number - last_block > rate_limit) + .map(|last_block| block_number - last_block > rate_limit.into()) .unwrap_or(true) { sponsor_metadata_changes = true; --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -164,7 +164,7 @@ .ok_or_else(|| >::TokenNotFound)?; ensure!( &token_data.owner == sender - || (collection.limits.owner_can_transfer + || (collection.limits.owner_can_transfer() && collection.is_owner_or_admin(sender)?), >::NoPermission ); @@ -215,7 +215,7 @@ token: TokenId, ) -> DispatchResult { ensure!( - collection.transfers_enabled, + collection.limits.transfers_enabled(), >::TransferNotAllowed ); @@ -223,7 +223,8 @@ .ok_or_else(|| >::TokenNotFound)?; ensure!( &token_data.owner == from - || (collection.limits.owner_can_transfer && collection.is_owner_or_admin(from)?), + || (collection.limits.owner_can_transfer() + && collection.is_owner_or_admin(from)?), >::NoPermission ); @@ -327,7 +328,7 @@ .checked_add(data.len() as u32) .ok_or(ArithmeticError::Overflow)?; ensure!( - tokens_minted < collection.limits.token_limit, + tokens_minted < collection.limits.token_limit(), >::CollectionTokenLimitExceeded ); collection.consume_sstore()?; --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -268,7 +268,7 @@ amount: u128, ) -> DispatchResult { ensure!( - collection.transfers_enabled, + collection.limits.transfers_enabled(), >::TransferNotAllowed ); @@ -404,7 +404,7 @@ .checked_add(data.len() as u32) .ok_or(ArithmeticError::Overflow)?; ensure!( - tokens_minted < collection.limits.token_limit, + tokens_minted < collection.limits.token_limit(), >::CollectionTokenLimitExceeded ); --- a/primitives/nft/src/lib.rs +++ b/primitives/nft/src/lib.rs @@ -42,6 +42,7 @@ 10 }; pub const COLLECTION_ADMINS_LIMIT: u64 = 5; +pub const COLLECTION_TOKEN_LIMIT: u32 = u32::MAX; pub const ACCOUNT_TOKEN_OWNERSHIP_LIMIT: u32 = if cfg!(not(feature = "limit-testing")) { 1000000 } else { @@ -217,11 +218,10 @@ pub offchain_schema: Vec, pub schema_version: SchemaVersion, pub sponsorship: SponsorshipState, - pub limits: CollectionLimits, // Collection private restrictions - pub variable_on_chain_schema: Vec, // - pub const_on_chain_schema: Vec, // + pub limits: CollectionLimits, // Collection private restrictions + pub variable_on_chain_schema: Vec, // + pub const_on_chain_schema: Vec, // pub meta_update_permission: MetaUpdatePermission, - pub transfers_enabled: bool, } #[derive(Encode, Decode, Debug, Clone, PartialEq, TypeInfo)] @@ -246,42 +246,57 @@ pub variable_data: Vec, } -#[derive(Encode, Decode, Debug, Clone, PartialEq, TypeInfo)] +#[derive(Encode, Decode, Debug, Default, Clone, PartialEq, TypeInfo)] #[cfg_attr(feature = "serde1", derive(Serialize, Deserialize))] -pub struct CollectionLimits { +pub struct CollectionLimits { pub account_token_ownership_limit: Option, - pub sponsored_data_size: u32, + pub sponsored_data_size: Option, /// None - setVariableMetadata is not sponsored /// Some(v) - setVariableMetadata is sponsored /// if there is v block between txs - pub sponsored_data_rate_limit: Option, - pub token_limit: u32, + pub sponsored_data_rate_limit: Option, + pub token_limit: Option, // Timeouts for item types in passed blocks - pub sponsor_transfer_timeout: u32, - pub owner_can_transfer: bool, - pub owner_can_destroy: bool, + pub sponsor_transfer_timeout: Option, + pub owner_can_transfer: Option, + pub owner_can_destroy: Option, + pub transfers_enabled: Option, } -impl CollectionLimits { +impl CollectionLimits { pub fn account_token_ownership_limit(&self) -> u32 { self.account_token_ownership_limit .unwrap_or(ACCOUNT_TOKEN_OWNERSHIP_LIMIT) - .min(ACCOUNT_TOKEN_OWNERSHIP_LIMIT) + .min(MAX_TOKEN_OWNERSHIP) } -} - -impl Default for CollectionLimits { - fn default() -> Self { - Self { - account_token_ownership_limit: Some(10_000_000), - token_limit: u32::max_value(), - sponsored_data_size: u32::MAX, - sponsored_data_rate_limit: None, - sponsor_transfer_timeout: 14400, - owner_can_transfer: true, - owner_can_destroy: true, - } + pub fn sponsored_data_size(&self) -> u32 { + self.sponsored_data_size + .unwrap_or(CUSTOM_DATA_LIMIT) + .min(CUSTOM_DATA_LIMIT) + } + pub fn token_limit(&self) -> u32 { + self.token_limit + .unwrap_or(COLLECTION_TOKEN_LIMIT) + .min(COLLECTION_TOKEN_LIMIT) + } + pub fn sponsor_transfer_timeout(&self, default: u32) -> u32 { + self.sponsor_transfer_timeout + .unwrap_or(default) + .min(MAX_SPONSOR_TIMEOUT) + } + pub fn owner_can_transfer(&self) -> bool { + self.owner_can_transfer.unwrap_or(true) + } + pub fn owner_can_destroy(&self) -> bool { + self.owner_can_destroy.unwrap_or(true) + } + pub fn transfers_enabled(&self) -> bool { + self.transfers_enabled.unwrap_or(true) + } + pub fn sponsored_data_rate_limit(&self) -> Option { + self.sponsored_data_rate_limit + .map(|v| v.min(MAX_SPONSOR_TIMEOUT)) } } -- gitstuff