--- a/pallets/common/src/dispatch.rs +++ b/pallets/common/src/dispatch.rs @@ -36,6 +36,13 @@ }, error, })?; + handle.check_is_internal().map_err(|error| DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(dispatch_weight::()), + pays_fee: Pays::Yes, + }, + error, + })?; let dispatched = T::CollectionDispatch::dispatch(handle); let mut result = call(dispatched.as_dyn()); match &mut result { --- a/pallets/common/src/erc.rs +++ b/pallets/common/src/erc.rs @@ -316,8 +316,9 @@ } fn save(collection: &CollectionHandle) -> Result { + // TODO possibly delete for the lack of transaction collection - .check_is_mutable() + .check_is_internal() .map_err(dispatch_to_evm::)?; >::insert(collection.id, collection.collection.clone()); Ok(()) --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -149,20 +149,16 @@ )) } pub fn save(self) -> Result<(), DispatchError> { - self.check_is_mutable()?; >::insert(self.id, self.collection); Ok(()) } pub fn set_sponsor(&mut self, sponsor: T::AccountId) -> DispatchResult { - self.check_is_mutable()?; self.collection.sponsorship = SponsorshipState::Unconfirmed(sponsor); Ok(()) } pub fn confirm_sponsorship(&mut self, sender: &T::AccountId) -> Result { - self.check_is_mutable()?; - if self.collection.sponsorship.pending_sponsor() != Some(sender) { return Ok(false); } @@ -171,11 +167,21 @@ Ok(true) } - /// Checks that collection is can be mutate. - /// Now check only `external_collection` flag and if it **true**, than return `CollectionIsReadOnly` error. - pub fn check_is_mutable(&self) -> DispatchResult { + /// Checks that the collection was created with, and must be operated upon through **Unique API**. + /// Now check only the `external_collection` flag and if it's **true**, then return `CollectionIsExternal` error. + pub fn check_is_internal(&self) -> DispatchResult { if self.external_collection { - return Err(>::CollectionIsReadOnly)?; + return Err(>::CollectionIsExternal)?; + } + + Ok(()) + } + + /// Checks that the collection was created with, and must be operated upon through an **assimilated API**. + /// Now check only the `external_collection` flag and if it's **false**, then return `CollectionIsInternal` error. + pub fn check_is_external(&self) -> DispatchResult { + if !self.external_collection { + return Err(>::CollectionIsInternal)?; } Ok(()) @@ -449,8 +455,11 @@ /// Empty property keys are forbidden EmptyPropertyKey, - /// Collection is read only - CollectionIsReadOnly, + /// Tried to access an external collection with an internal API + CollectionIsExternal, + + /// Tried to access an internal collection with an external API + CollectionIsInternal, } #[pallet::storage] @@ -754,6 +763,7 @@ pub fn init_collection( owner: T::CrossAccountId, data: CreateCollectionData, + is_external: bool, ) -> Result { { ensure!( @@ -797,7 +807,7 @@ Self::clamp_permissions(data.mode.clone(), &Default::default(), permissions) }) .unwrap_or_else(|| Ok(CollectionPermissions::default()))?, - external_collection: false, + external_collection: is_external, }; let mut collection_properties = up_data_structs::CollectionProperties::get(); @@ -854,7 +864,6 @@ collection: CollectionHandle, sender: &T::CrossAccountId, ) -> DispatchResult { - collection.check_is_mutable()?; ensure!( collection.limits.owner_can_destroy(), >::NoPermission, @@ -884,7 +893,6 @@ sender: &T::CrossAccountId, property: Property, ) -> DispatchResult { - collection.check_is_mutable()?; collection.check_is_owner_or_admin(sender)?; CollectionProperties::::try_mutate(collection.id, |properties| { @@ -930,8 +938,6 @@ sender: &T::CrossAccountId, properties: Vec, ) -> DispatchResult { - collection.check_is_mutable()?; - for property in properties { Self::set_collection_property(collection, sender, property)?; } @@ -944,7 +950,6 @@ sender: &T::CrossAccountId, property_key: PropertyKey, ) -> DispatchResult { - collection.check_is_mutable()?; collection.check_is_owner_or_admin(sender)?; CollectionProperties::::try_mutate(collection.id, |properties| { @@ -966,8 +971,6 @@ sender: &T::CrossAccountId, property_keys: Vec, ) -> DispatchResult { - collection.check_is_mutable()?; - for key in property_keys { Self::delete_collection_property(collection, sender, key)?; } @@ -992,7 +995,6 @@ sender: &T::CrossAccountId, property_permission: PropertyKeyPermission, ) -> DispatchResult { - collection.check_is_mutable()?; collection.check_is_owner_or_admin(sender)?; let all_permissions = CollectionPropertyPermissions::::get(collection.id); @@ -1024,8 +1026,6 @@ sender: &T::CrossAccountId, property_permissions: Vec, ) -> DispatchResult { - collection.check_is_mutable()?; - for prop_pemission in property_permissions { Self::set_property_permission(collection, sender, prop_pemission)?; } @@ -1113,7 +1113,6 @@ user: &T::CrossAccountId, allowed: bool, ) -> DispatchResult { - collection.check_is_mutable()?; collection.check_is_owner_or_admin(sender)?; // ========= @@ -1133,7 +1132,6 @@ user: &T::CrossAccountId, admin: bool, ) -> DispatchResult { - collection.check_is_mutable()?; collection.check_is_owner_or_admin(sender)?; let was_admin = >::get((collection.id, user)); --- a/pallets/fungible/src/lib.rs +++ b/pallets/fungible/src/lib.rs @@ -137,7 +137,7 @@ owner: T::CrossAccountId, data: CreateCollectionData, ) -> Result { - >::init_collection(owner, data) + >::init_collection(owner, data, false) } pub fn destroy_collection( collection: FungibleHandle, @@ -168,8 +168,6 @@ owner: &T::CrossAccountId, amount: u128, ) -> DispatchResult { - collection.check_is_mutable()?; - let total_supply = >::get(collection.id) .checked_sub(amount) .ok_or(>::TokenValueTooLow)?; @@ -216,8 +214,6 @@ amount: u128, nesting_budget: &dyn Budget, ) -> DispatchResult { - collection.check_is_mutable()?; - ensure!( collection.limits.transfers_enabled(), >::TransferNotAllowed, @@ -287,8 +283,6 @@ data: BTreeMap, nesting_budget: &dyn Budget, ) -> DispatchResult { - collection.check_is_mutable()?; - if !collection.is_owner_or_admin(sender) { ensure!( collection.permissions.mint_mode(), @@ -390,7 +384,6 @@ spender: &T::CrossAccountId, amount: u128, ) -> DispatchResult { - collection.check_is_mutable()?; if collection.permissions.access() == AccessMode::AllowList { collection.check_allowlist(owner)?; collection.check_allowlist(spender)?; --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -304,8 +304,9 @@ pub fn init_collection( owner: T::CrossAccountId, data: CreateCollectionData, + is_external: bool, ) -> Result { - >::init_collection(owner, data) + >::init_collection(owner, data, is_external) } pub fn destroy_collection( collection: NonfungibleHandle, @@ -336,8 +337,6 @@ sender: &T::CrossAccountId, token: TokenId, ) -> DispatchResult { - collection.check_is_mutable()?; - let token_data = >::get((collection.id, token)).ok_or(>::TokenNotFound)?; ensure!( @@ -458,7 +457,6 @@ &property.key, is_token_create, )?; - collection.check_is_mutable()?; >::try_mutate((collection.id, token_id), |properties| { let property = property.clone(); @@ -496,7 +494,6 @@ token_id: TokenId, property_key: PropertyKey, ) -> DispatchResult { - collection.check_is_mutable()?; Self::check_token_change_permission(collection, sender, token_id, &property_key, false)?; >::try_mutate((collection.id, token_id), |properties| { @@ -574,8 +571,6 @@ token_id: TokenId, property_keys: Vec, ) -> DispatchResult { - collection.check_is_mutable()?; - for key in property_keys { Self::delete_token_property(collection, sender, token_id, key)?; } @@ -622,8 +617,6 @@ token: TokenId, nesting_budget: &dyn Budget, ) -> DispatchResult { - collection.check_is_mutable()?; - ensure!( collection.limits.transfers_enabled(), >::TransferNotAllowed @@ -902,8 +895,6 @@ token: TokenId, spender: Option<&T::CrossAccountId>, ) -> DispatchResult { - collection.check_is_mutable()?; - if collection.permissions.access() == AccessMode::AllowList { collection.check_allowlist(sender)?; if let Some(spender) = spender { --- a/pallets/proxy-rmrk-core/src/lib.rs +++ b/pallets/proxy-rmrk-core/src/lib.rs @@ -235,6 +235,7 @@ Self::unique_collection_id(collection_id)?, misc::CollectionType::Regular, )?; + collection.check_is_external()?; >::destroy_collection(collection, &cross_sender) .map_err(Self::map_unique_err_to_proxy)?; @@ -256,6 +257,9 @@ ) -> DispatchResult { let sender = ensure_signed(origin)?; + let collection = Self::get_nft_collection(Self::unique_collection_id(collection_id)?)?; + collection.check_is_external()?; + let new_issuer = T::Lookup::lookup(new_issuer)?; Self::change_collection_owner( @@ -287,6 +291,7 @@ Self::unique_collection_id(collection_id)?, misc::CollectionType::Regular, )?; + collection.check_is_external()?; Self::check_collection_owner(&collection, &cross_sender)?; @@ -318,17 +323,18 @@ let sender = ensure_signed(origin)?; let sender = T::CrossAccountId::from_sub(sender); let cross_owner = T::CrossAccountId::from_sub(owner.clone()); - - let royalty_info = royalty_amount.map(|amount| rmrk_traits::RoyaltyInfo { - recipient: recipient.unwrap_or_else(|| owner.clone()), - amount, - }); let collection = Self::get_typed_nft_collection( Self::unique_collection_id(collection_id)?, misc::CollectionType::Regular, )?; + collection.check_is_external()?; + let royalty_info = royalty_amount.map(|amount| rmrk_traits::RoyaltyInfo { + recipient: recipient.unwrap_or_else(|| owner.clone()), + amount, + }); + let nft_id = Self::create_nft( &sender, &cross_owner, @@ -382,6 +388,12 @@ let sender = ensure_signed(origin)?; let cross_sender = T::CrossAccountId::from_sub(sender.clone()); + let collection = Self::get_typed_nft_collection( + Self::unique_collection_id(collection_id)?, + misc::CollectionType::Regular, + )?; + collection.check_is_external()?; + Self::destroy_nft( cross_sender, Self::unique_collection_id(collection_id)?, @@ -411,13 +423,14 @@ let collection_id = Self::unique_collection_id(rmrk_collection_id)?; let nft_id = rmrk_nft_id.into(); + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let token_data = >::get((collection_id, nft_id)).ok_or(>::NoAvailableNftId)?; let from = token_data.owner; - - let collection = - Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; ensure!( Self::get_nft_property_decoded(collection_id, nft_id, RmrkProperty::Transferable)?, @@ -516,6 +529,7 @@ let collection = Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; let new_cross_owner = match new_owner { RmrkAccountIdOrCollectionNftTuple::AccountId(ref account_id) => { @@ -581,6 +595,10 @@ let collection_id = Self::unique_collection_id(rmrk_collection_id)?; let nft_id = rmrk_nft_id.into(); + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + Self::destroy_nft(cross_sender, collection_id, nft_id).map_err(|err| { if err == >::NoPermission.into() || err == >::ApprovedValueTooLow.into() @@ -613,6 +631,9 @@ let collection_id = Self::unique_collection_id(rmrk_collection_id) .map_err(|_| >::ResourceDoesntExist)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; let nft_id = rmrk_nft_id.into(); let resource_id = rmrk_resource_id.into(); @@ -666,6 +687,9 @@ let collection_id = Self::unique_collection_id(rmrk_collection_id) .map_err(|_| >::ResourceDoesntExist)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; let nft_id = rmrk_nft_id.into(); let resource_id = rmrk_resource_id.into(); @@ -720,6 +744,10 @@ let sender = T::CrossAccountId::from_sub(sender); let collection_id = Self::unique_collection_id(rmrk_collection_id)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let budget = budget::Value::new(NESTING_BUDGET); match maybe_nft_id { @@ -775,6 +803,11 @@ let collection_id = Self::unique_collection_id(rmrk_collection_id)?; let nft_id = rmrk_nft_id.into(); + + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let budget = budget::Value::new(NESTING_BUDGET); Self::ensure_nft_type(collection_id, nft_id, NftType::Regular)?; @@ -799,15 +832,20 @@ #[transactional] pub fn add_basic_resource( origin: OriginFor, - collection_id: RmrkCollectionId, + rmrk_collection_id: RmrkCollectionId, nft_id: RmrkNftId, resource: RmrkBasicResource, ) -> DispatchResult { let sender = ensure_signed(origin.clone())?; + let collection_id = Self::unique_collection_id(rmrk_collection_id)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let resource_id = Self::resource_add( sender, - Self::unique_collection_id(collection_id)?, + collection_id, nft_id.into(), [ Self::rmrk_property(TokenType, &NftType::Resource)?, @@ -831,16 +869,21 @@ #[transactional] pub fn add_composable_resource( origin: OriginFor, - collection_id: RmrkCollectionId, + rmrk_collection_id: RmrkCollectionId, nft_id: RmrkNftId, _resource_id: RmrkBoundedResource, resource: RmrkComposableResource, ) -> DispatchResult { let sender = ensure_signed(origin.clone())?; + let collection_id = Self::unique_collection_id(rmrk_collection_id)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let resource_id = Self::resource_add( sender, - Self::unique_collection_id(collection_id)?, + collection_id, nft_id.into(), [ Self::rmrk_property(TokenType, &NftType::Resource)?, @@ -866,15 +909,20 @@ #[transactional] pub fn add_slot_resource( origin: OriginFor, - collection_id: RmrkCollectionId, + rmrk_collection_id: RmrkCollectionId, nft_id: RmrkNftId, resource: RmrkSlotResource, ) -> DispatchResult { let sender = ensure_signed(origin.clone())?; + let collection_id = Self::unique_collection_id(rmrk_collection_id)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + let resource_id = Self::resource_add( sender, - Self::unique_collection_id(collection_id)?, + collection_id, nft_id.into(), [ Self::rmrk_property(TokenType, &NftType::Resource)?, @@ -900,18 +948,18 @@ #[transactional] pub fn remove_resource( origin: OriginFor, - collection_id: RmrkCollectionId, + rmrk_collection_id: RmrkCollectionId, nft_id: RmrkNftId, resource_id: RmrkResourceId, ) -> DispatchResult { let sender = ensure_signed(origin.clone())?; - Self::resource_remove( - sender, - Self::unique_collection_id(collection_id)?, - nft_id.into(), - resource_id.into(), - )?; + let collection_id = Self::unique_collection_id(rmrk_collection_id)?; + let collection = + Self::get_typed_nft_collection(collection_id, misc::CollectionType::Regular)?; + collection.check_is_external()?; + + Self::resource_remove(sender, collection_id, nft_id.into(), resource_id.into())?; Self::deposit_event(Event::ResourceRemoval { nft_id, @@ -968,7 +1016,7 @@ data: CreateCollectionData, properties: impl Iterator, ) -> Result { - let collection_id = >::init_collection(sender, data); + let collection_id = >::init_collection(sender, data, true); if let Err(DispatchError::Arithmetic(_)) = &collection_id { return Err(>::NoAvailableCollectionId.into()); --- a/pallets/proxy-rmrk-equip/src/lib.rs +++ b/pallets/proxy-rmrk-equip/src/lib.rs @@ -94,7 +94,8 @@ ..Default::default() }; - let collection_id_res = >::init_collection(cross_sender.clone(), data); + let collection_id_res = + >::init_collection(cross_sender.clone(), data, true); if let Err(DispatchError::Arithmetic(_)) = &collection_id_res { return Err(>::NoAvailableBaseId.into()); @@ -155,6 +156,7 @@ misc::CollectionType::Base, ) .map_err(|_| >::BaseDoesntExist)?; + collection.check_is_external()?; if theme.name.as_slice() == b"default" { >::insert(collection_id, true); --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -200,7 +200,7 @@ owner: T::CrossAccountId, data: CreateCollectionData, ) -> Result { - >::init_collection(owner, data) + >::init_collection(owner, data, false) } pub fn destroy_collection( collection: RefungibleHandle, @@ -234,7 +234,6 @@ } pub fn burn_token(collection: &RefungibleHandle, token_id: TokenId) -> DispatchResult { - collection.check_is_mutable()?; let burnt = >::get(collection.id) .checked_add(1) .ok_or(ArithmeticError::Overflow)?; @@ -254,7 +253,6 @@ token: TokenId, amount: u128, ) -> DispatchResult { - collection.check_is_mutable()?; let total_supply = >::get((collection.id, token)) .checked_sub(amount) .ok_or(>::TokenValueTooLow)?; @@ -327,7 +325,6 @@ amount: u128, nesting_budget: &dyn Budget, ) -> DispatchResult { - collection.check_is_mutable()?; ensure!( collection.limits.transfers_enabled(), >::TransferNotAllowed @@ -576,7 +573,6 @@ token: TokenId, amount: u128, ) -> DispatchResult { - collection.check_is_mutable()?; if collection.permissions.access() == AccessMode::AllowList { collection.check_allowlist(sender)?; collection.check_allowlist(spender)?; --- a/pallets/unique/src/eth/mod.rs +++ b/pallets/unique/src/eth/mod.rs @@ -92,8 +92,9 @@ ..Default::default() }; - let collection_id = >::init_collection(caller.clone(), data) - .map_err(pallet_evm_coder_substrate::dispatch_to_evm::)?; + let collection_id = + >::init_collection(caller.clone(), data, false) + .map_err(pallet_evm_coder_substrate::dispatch_to_evm::)?; let address = pallet_common::eth::collection_id_to_address(collection_id); Ok(address) --- a/pallets/unique/src/lib.rs +++ b/pallets/unique/src/lib.rs @@ -304,7 +304,7 @@ pub fn destroy_collection(origin, collection_id: CollectionId) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; - collection.check_is_mutable()?; + collection.check_is_internal()?; // ========= @@ -339,6 +339,7 @@ let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; + collection.check_is_internal()?; >::toggle_allowlist( &collection, @@ -373,6 +374,7 @@ let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; + collection.check_is_internal()?; >::toggle_allowlist( &collection, @@ -407,7 +409,7 @@ let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; - target_collection.check_is_mutable()?; + target_collection.check_is_internal()?; target_collection.check_is_owner(&sender)?; target_collection.owner = new_owner.clone(); @@ -437,6 +439,7 @@ pub fn add_collection_admin(origin, collection_id: CollectionId, new_admin_id: T::CrossAccountId) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; + collection.check_is_internal()?; >::deposit_event(Event::::CollectionAdminAdded( collection_id, @@ -463,6 +466,7 @@ pub fn remove_collection_admin(origin, collection_id: CollectionId, account_id: T::CrossAccountId) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; + collection.check_is_internal()?; >::deposit_event(Event::::CollectionAdminRemoved( collection_id, @@ -488,6 +492,7 @@ let mut target_collection = >::try_get(collection_id)?; target_collection.check_is_owner(&sender)?; + target_collection.check_is_internal()?; target_collection.set_sponsor(new_sponsor.clone())?; @@ -512,6 +517,7 @@ let sender = ensure_signed(origin)?; let mut target_collection = >::try_get(collection_id)?; + target_collection.check_is_internal()?; ensure!( target_collection.confirm_sponsorship(&sender)?, Error::::ConfirmUnsetSponsorFail @@ -540,6 +546,7 @@ let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; + target_collection.check_is_internal()?; target_collection.check_is_owner(&sender)?; target_collection.sponsorship = SponsorshipState::Disabled; @@ -704,6 +711,7 @@ pub fn set_transfers_enabled_flag(origin, collection_id: CollectionId, value: bool) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; + target_collection.check_is_internal()?; target_collection.check_is_owner(&sender)?; // ========= @@ -858,6 +866,7 @@ ) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; + target_collection.check_is_internal()?; target_collection.check_is_owner(&sender)?; let old_limit = &target_collection.limits; @@ -879,6 +888,7 @@ ) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let mut target_collection = >::try_get(collection_id)?; + target_collection.check_is_internal()?; target_collection.check_is_owner(&sender)?; let old_limit = &target_collection.permissions; --- a/runtime/common/src/dispatch.rs +++ b/runtime/common/src/dispatch.rs @@ -35,7 +35,7 @@ data: CreateCollectionData, ) -> DispatchResult { let _id = match data.mode { - CollectionMode::NFT => >::init_collection(sender, data)?, + CollectionMode::NFT => >::init_collection(sender, data, false)?, CollectionMode::Fungible(decimal_points) => { // check params ensure!(