From 9d51561e5bd0199e655cae85e1e4dedd78b92dfd Mon Sep 17 00:00:00 2001 From: Daniel Shiposha Date: Fri, 27 May 2022 12:29:38 +0000 Subject: [PATCH] Revert "feat: burn children when destroying a collection" This reverts commit 4b7f4d90a16f3a5ab26bed0511dabae078429724. --- --- a/pallets/common/src/dispatch.rs +++ b/pallets/common/src/dispatch.rs @@ -6,7 +6,7 @@ weights::Pays, traits::Get, }; -use up_data_structs::{CollectionId, CreateCollectionData, budget::Budget}; +use up_data_structs::{CollectionId, CreateCollectionData}; use crate::{pallet::Config, CommonCollectionOperations, CollectionHandle}; @@ -57,11 +57,7 @@ pub trait CollectionDispatch { fn create(sender: T::AccountId, data: CreateCollectionData) -> DispatchResult; - fn destroy( - sender: T::CrossAccountId, - handle: CollectionHandle, - nesting_budget: &dyn Budget, - ) -> DispatchResult; + fn destroy(sender: T::CrossAccountId, handle: CollectionHandle) -> DispatchResult; fn dispatch(handle: CollectionHandle) -> Self; fn into_inner(self) -> CollectionHandle; --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -1169,12 +1169,6 @@ token: TokenId, amount: u128, ) -> DispatchResultWithPostInfo; - fn burn_item_unchecked( - &self, - owner: &T::CrossAccountId, - token: TokenId, - amount: u128, - ) -> DispatchResult; fn set_collection_properties( &self, sender: T::CrossAccountId, --- a/pallets/fungible/src/common.rs +++ b/pallets/fungible/src/common.rs @@ -170,17 +170,6 @@ ) } - fn burn_item_unchecked( - &self, - owner: &T::CrossAccountId, - _token: TokenId, - amount: u128, - ) -> sp_runtime::DispatchResult { - >::burn_item_unchecked(self, owner, amount)?; - - Ok(()) - } - fn transfer( &self, from: T::CrossAccountId, --- a/pallets/fungible/src/lib.rs +++ b/pallets/fungible/src/lib.rs @@ -160,36 +160,6 @@ owner: &T::CrossAccountId, amount: u128, ) -> DispatchResult { - if collection.access == AccessMode::AllowList { - collection.check_allowlist(owner)?; - } - - // ========= - - Self::burn_item_unchecked(collection, owner, amount)?; - - >::deposit_log( - ERC20Events::Transfer { - from: *owner.as_eth(), - to: H160::default(), - value: amount.into(), - } - .to_log(collection_id_to_address(collection.id)), - ); - >::deposit_event(CommonEvent::ItemDestroyed( - collection.id, - TokenId::default(), - owner.clone(), - amount, - )); - Ok(()) - } - - pub fn burn_item_unchecked( - collection: &FungibleHandle, - owner: &T::CrossAccountId, - amount: u128, - ) -> DispatchResult { let total_supply = >::get(collection.id) .checked_sub(amount) .ok_or(>::TokenValueTooLow)?; @@ -216,6 +186,20 @@ } >::insert(collection.id, total_supply); + >::deposit_log( + ERC20Events::Transfer { + from: *owner.as_eth(), + to: H160::default(), + value: amount.into(), + } + .to_log(collection_id_to_address(collection.id)), + ); + >::deposit_event(CommonEvent::ItemDestroyed( + collection.id, + TokenId::default(), + owner.clone(), + amount, + )); Ok(()) } --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -264,19 +264,6 @@ } } - fn burn_item_unchecked( - &self, - owner:& T::CrossAccountId, - token: TokenId, - amount: u128, - ) -> sp_runtime::DispatchResult { - if amount == 1 { - >::burn_item_unchecked(self, owner, token) - } else { - Ok(()) - } - } - fn transfer( &self, from: T::CrossAccountId, --- a/pallets/nonfungible/src/lib.rs +++ b/pallets/nonfungible/src/lib.rs @@ -27,7 +27,6 @@ use pallet_evm::{account::CrossAccountId, Pallet as PalletEvm}; use pallet_common::{ Error as CommonError, Pallet as PalletCommon, Event as CommonEvent, CollectionHandle, - dispatch::CollectionDispatch, eth::collection_id_to_address, }; use pallet_structure::Pallet as PalletStructure; @@ -80,8 +79,6 @@ NonfungibleItemsHaveNoAmount, /// Unable to burn NFT with children CantBurnNftWithChildren, - /// Too many children to burn when destroying a collection - TooManyChildrenToBurn, } #[pallet::config] @@ -293,14 +290,13 @@ pub fn destroy_collection( collection: NonfungibleHandle, sender: &T::CrossAccountId, - nesting_budget: &dyn Budget, ) -> DispatchResult { let id = collection.id; // ========= - Self::burn_children_in_collection(id, nesting_budget)?; PalletCommon::destroy_collection(collection.0, sender)?; + >::remove_prefix((id,), None); >::remove_prefix((id,), None); >::remove_prefix((id,), None); @@ -308,47 +304,9 @@ >::remove(id); >::remove_prefix((id,), None); >::remove_prefix((id,), None); - Ok(()) - } - - #[transactional] - fn burn_children_in_collection(collection_id: CollectionId, nesting_budget: &dyn Budget) -> DispatchResult { - for (parent_id, child) in >::drain_prefix((collection_id,)) - .map(|((parent_id, child), _)| (parent_id, child)) { - - let parent_address = T::CrossTokenAddressMapping::token_to_address(collection_id, parent_id); - Self::burn_tree(parent_address, child.0, child.1, nesting_budget)?; - } - Ok(()) } - fn burn_tree( - parent: T::CrossAccountId, - collection_id: CollectionId, - token_id: TokenId, - nesting_budget: &dyn Budget - ) -> DispatchResult { - if !nesting_budget.consume() { - return Err(>::TooManyChildrenToBurn.into()); - } - - let handle = >::try_get(collection_id)?; - let handle = T::CollectionDispatch::dispatch(handle); - let handle = handle.as_dyn(); - - let amount = handle.balance(parent.clone(), token_id); - - handle.burn_item_unchecked(&parent, token_id, amount)?; - - for child in >::drain_prefix((collection_id, token_id)).map(|(child, _)| child) { - let parent = T::CrossTokenAddressMapping::token_to_address(collection_id, token_id); - Self::burn_tree(parent, child.0, child.1, nesting_budget)?; - } - - Ok(()) - } - pub fn burn( collection: &NonfungibleHandle, sender: &T::CrossAccountId, @@ -370,11 +328,31 @@ return Err(>::CantBurnNftWithChildren.into()); } - let old_spender = >::get((collection.id, token)); + let burnt = >::get(collection.id) + .checked_add(1) + .ok_or(ArithmeticError::Overflow)?; + + let balance = >::get((collection.id, token_data.owner.clone())) + .checked_sub(1) + .ok_or(ArithmeticError::Overflow)?; + + if balance == 0 { + >::remove((collection.id, token_data.owner.clone())); + } else { + >::insert((collection.id, token_data.owner.clone()), balance); + } + + if let Some(owner) = T::CrossTokenAddressMapping::address_to_token(&token_data.owner) { + Self::unnest(owner, (collection.id, token)); + } // ========= - Self::burn_item_unchecked(collection, &token_data.owner, token)?; + >::remove((collection.id, &token_data.owner, token)); + >::insert(collection.id, burnt); + >::remove((collection.id, token)); + >::remove((collection.id, token)); + let old_spender = >::take((collection.id, token)); if let Some(old_spender) = old_spender { >::deposit_event(CommonEvent::Approved( @@ -400,40 +378,6 @@ token_data.owner, 1, )); - Ok(()) - } - - pub fn burn_item_unchecked( - collection: &NonfungibleHandle, - owner: &T::CrossAccountId, - token: TokenId, - ) -> DispatchResult { - let burnt = >::get(collection.id) - .checked_add(1) - .ok_or(ArithmeticError::Overflow)?; - - let balance = >::get((collection.id, owner.clone())) - .checked_sub(1) - .ok_or(ArithmeticError::Overflow)?; - - // ========= - - if let Some(owner) = T::CrossTokenAddressMapping::address_to_token(owner) { - Self::unnest(owner, (collection.id, token)); - } - - if balance == 0 { - >::remove((collection.id, owner.clone())); - } else { - >::insert((collection.id, owner.clone()), balance); - } - - >::remove((collection.id, owner, token)); - >::insert(collection.id, burnt); - >::remove((collection.id, token)); - >::remove((collection.id, token)); - >::remove((collection.id, token)); - Ok(()) } --- a/pallets/proxy-rmrk-core/src/lib.rs +++ b/pallets/proxy-rmrk-core/src/lib.rs @@ -179,8 +179,7 @@ ensure!(collection.total_supply() == 0, >::CollectionNotEmpty); - let empty_budget = budget::Value::new(0); - >::destroy_collection(collection, &cross_sender, &empty_budget) + >::destroy_collection(collection, &cross_sender) .map_err(Self::map_common_err_to_proxy)?; Self::deposit_event(Event::CollectionDestroyed { issuer: sender, collection_id }); --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -205,15 +205,6 @@ ) } - fn burn_item_unchecked( - &self, - owner: &T::CrossAccountId, - token: TokenId, - amount: u128, - ) -> sp_runtime::DispatchResult { - >::burn_item_unchecked(self, owner, token, amount) - } - fn transfer( &self, from: T::CrossAccountId, --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -245,25 +245,6 @@ token: TokenId, amount: u128, ) -> DispatchResult { - Self::burn_item_unchecked(collection, owner, token, amount)?; - - // TODO: ERC20 transfer event - >::deposit_event(CommonEvent::ItemDestroyed( - collection.id, - token, - owner.clone(), - amount, - )); - - Ok(()) - } - - pub fn burn_item_unchecked( - collection: &RefungibleHandle, - owner: &T::CrossAccountId, - token: TokenId, - amount: u128, - ) -> DispatchResult { let total_supply = >::get((collection.id, token)) .checked_sub(amount) .ok_or(>::TokenValueTooLow)?; @@ -318,6 +299,13 @@ >::insert((collection.id, token, owner), balance); } >::insert((collection.id, token), total_supply); + // TODO: ERC20 transfer event + >::deposit_event(CommonEvent::ItemDestroyed( + collection.id, + token, + owner.clone(), + amount, + )); Ok(()) } --- a/pallets/unique/src/lib.rs +++ b/pallets/unique/src/lib.rs @@ -332,24 +332,15 @@ /// # Arguments /// /// * collection_id: collection to destroy. - #[weight = - >::destroy_collection() - + >::burn_children_in_collection(*max_children_to_burn) - ] + #[weight = >::destroy_collection()] #[transactional] - pub fn destroy_collection( - origin, - collection_id: CollectionId, - max_children_to_burn: u32, - ) -> DispatchResult { + pub fn destroy_collection(origin, collection_id: CollectionId) -> DispatchResult { let sender = T::CrossAccountId::from_sub(ensure_signed(origin)?); let collection = >::try_get(collection_id)?; - let budget = budget::Value::new(max_children_to_burn); - // ========= - T::CollectionDispatch::destroy(sender, collection, &budget)?; + T::CollectionDispatch::destroy(sender, collection)?; >::remove_prefix(collection_id, None); >::remove_prefix(collection_id, None); --- a/pallets/unique/src/weights.rs +++ b/pallets/unique/src/weights.rs @@ -34,7 +34,6 @@ pub trait WeightInfo { fn create_collection() -> Weight; fn destroy_collection() -> Weight; - fn burn_children_in_collection(max: u32) -> Weight; fn add_to_allow_list() -> Weight; fn remove_from_allow_list() -> Weight; fn set_public_access_mode() -> Weight; @@ -74,12 +73,6 @@ .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(5 as Weight)) } - - fn burn_children_in_collection(max: u32) -> Weight { - // TODO - (50_000_000 as Weight).saturating_mul(max as Weight) - } - // Storage: Common CollectionById (r:1 w:0) // Storage: Common Allowlist (r:0 w:1) fn add_to_allow_list() -> Weight { @@ -199,12 +192,6 @@ .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(5 as Weight)) } - - fn burn_children_in_collection(max: u32) -> Weight { - // TODO - (50_000_000 as Weight).saturating_mul(max as Weight) - } - // Storage: Common CollectionById (r:1 w:0) // Storage: Common Allowlist (r:0 w:1) fn add_to_allow_list() -> Weight { --- a/runtime/common/src/dispatch.rs +++ b/runtime/common/src/dispatch.rs @@ -1,4 +1,4 @@ -use frame_support::{dispatch::{DispatchResult}, ensure}; +use frame_support::{dispatch::DispatchResult, ensure}; use pallet_evm::PrecompileResult; use sp_core::{H160, U256}; use sp_std::{borrow::ToOwned, vec::Vec}; @@ -12,7 +12,6 @@ use pallet_refungible::{Pallet as PalletRefungible, RefungibleHandle, erc::RefungibleTokenHandle}; use up_data_structs::{ CollectionMode, CreateCollectionData, MAX_DECIMAL_POINTS, mapping::TokenAddressMapping, - budget::Budget, }; pub enum CollectionDispatchT @@ -47,11 +46,7 @@ Ok(()) } - fn destroy( - sender: T::CrossAccountId, - collection: CollectionHandle, - nesting_budget: &dyn Budget, - ) -> DispatchResult { + fn destroy(sender: T::CrossAccountId, collection: CollectionHandle) -> DispatchResult { match collection.mode { CollectionMode::ReFungible => { PalletRefungible::destroy_collection(RefungibleHandle::cast(collection), &sender)? @@ -60,11 +55,7 @@ PalletFungible::destroy_collection(FungibleHandle::cast(collection), &sender)? } CollectionMode::NFT => { - PalletNonfungible::destroy_collection( - NonfungibleHandle::cast(collection), - &sender, - nesting_budget, - )? + PalletNonfungible::destroy_collection(NonfungibleHandle::cast(collection), &sender)? } } Ok(()) -- gitstuff