git.delta.rocks / unique-network / refs/commits / 6ad82a288c79

difftreelog

fix disallow nesting under non-existing tokens

Yaroslav Bolyukin2023-01-18parent: #d3f529b.patch.diff
in: master

3 files changed

modifiedpallets/nonfungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/lib.rs
+++ b/pallets/nonfungible/src/lib.rs
@@ -1332,7 +1332,10 @@
 		let permissive = nesting.permissive;
 
 		if permissive {
-			// Pass
+			ensure!(
+				<TokenData<T>>::contains_key((handle.id, under)),
+				<CommonError<T>>::TokenNotFound
+			);
 		} else if nesting.token_owner
 			&& <PalletStructure<T>>::check_indirectly_owned(
 				sender.clone(),
@@ -1341,9 +1344,12 @@
 				Some(from),
 				nesting_budget,
 			)? {
-			// Pass
+			// Pass, token existence is checked in `check_indirectly_owned`
 		} else if nesting.collection_admin && handle.is_owner_or_admin(&sender) {
-			// Pass
+			ensure!(
+				<TokenData<T>>::contains_key((handle.id, under)),
+				<CommonError<T>>::TokenNotFound
+			);
 		} else {
 			fail!(<CommonError<T>>::UserIsNotAllowedToNest);
 		}
modifiedpallets/structure/Cargo.tomldiffbeforeafterboth
17] }17] }
18up-data-structs = { path = "../../primitives/data-structs", default-features = false }18up-data-structs = { path = "../../primitives/data-structs", default-features = false }
19pallet-evm = { default-features = false, git = "https://github.com/uniquenetwork/frontier", branch = "unique-polkadot-v0.9.36" }19pallet-evm = { default-features = false, git = "https://github.com/uniquenetwork/frontier", branch = "unique-polkadot-v0.9.36" }
20log = { version = "0.4.17", default-features = false }
2021
21[features]22[features]
22default = ["std"]23default = ["std"]
modifiedpallets/structure/src/lib.rsdiffbeforeafterboth
--- a/pallets/structure/src/lib.rs
+++ b/pallets/structure/src/lib.rs
@@ -54,6 +54,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use pallet_common::CommonCollectionOperations;
+use pallet_common::{erc::CrossAccountId, eth::is_collection};
 use sp_std::collections::btree_set::BTreeSet;
 
 use frame_support::dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo};
@@ -86,6 +87,8 @@
 		BreadthLimit,
 		/// Couldn't find the token owner that is itself a token.
 		TokenNotFound,
+		/// Tried to nest token under collection contract address, instead of token address
+		CantNestTokenUnderCollection,
 	}
 
 	#[pallet::event]
@@ -302,7 +305,7 @@
 		token_id: TokenId,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
-		Self::try_exec_if_owner_is_valid_nft(under, |collection, parent_id| {
+		Self::try_exec_if_token(under, |collection, parent_id| {
 			collection.check_nesting(from, (collection_id, token_id), parent_id, nesting_budget)
 		})
 	}
@@ -319,7 +322,7 @@
 		token_id: TokenId,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
-		Self::try_exec_if_owner_is_valid_nft(under, |collection, parent_id| {
+		Self::try_exec_if_token(under, |collection, parent_id| {
 			collection.check_nesting(from, (collection_id, token_id), parent_id, nesting_budget)?;
 
 			collection.nest(parent_id, (collection_id, token_id));
@@ -336,7 +339,7 @@
 		collection_id: CollectionId,
 		token_id: TokenId,
 	) {
-		Self::exec_if_owner_is_valid_nft(owner, |collection, parent_id| {
+		Self::exec_if_token(owner, |collection, parent_id| {
 			collection.nest(parent_id, (collection_id, token_id))
 		});
 	}
@@ -347,45 +350,45 @@
 		collection_id: CollectionId,
 		token_id: TokenId,
 	) {
-		Self::exec_if_owner_is_valid_nft(owner, |collection, parent_id| {
-			collection.unnest(parent_id, (collection_id, token_id))
-		});
+		if let Err(e) = Self::try_exec_if_token(owner, |collection, parent_id| {
+			collection.unnest(parent_id, (collection_id, token_id));
+			Ok(())
+		}) {
+			log::warn!("unnest precondition failed: {e:?}")
+		}
 	}
 
-	fn exec_if_owner_is_valid_nft(
+	/// # Panics
+	/// If [`Self::try_exec_if_token`] fails
+	fn exec_if_token(
 		account: &T::CrossAccountId,
 		action: impl FnOnce(&dyn CommonCollectionOperations<T>, TokenId),
 	) {
-		Self::try_exec_if_owner_is_valid_nft(account, |collection, id| {
+		Self::try_exec_if_token(account, |collection, id| {
 			action(collection, id);
 			Ok(())
 		})
 		.unwrap();
 	}
 
-	fn try_exec_if_owner_is_valid_nft(
+	/// If `account` is a token address, execute `action` providing found collection as an argument
+	/// Token may not exist, it is expected it will be checked in the callback.
+	fn try_exec_if_token(
 		account: &T::CrossAccountId,
 		action: impl FnOnce(&dyn CommonCollectionOperations<T>, TokenId) -> DispatchResult,
 	) -> DispatchResult {
-		let account = T::CrossTokenAddressMapping::address_to_token(account);
-
-		if account.is_none() {
-			return Ok(());
+		if is_collection(&account.as_eth()) {
+			fail!(<Error<T>>::CantNestTokenUnderCollection);
 		}
+		let Some((collection, token)) = T::CrossTokenAddressMapping::address_to_token(account) else {
+			return Ok(())
+		};
 
-		let account = account.unwrap();
-
-		let handle = <CollectionHandle<T>>::try_get(account.0);
-
-		if handle.is_err() {
-			return Ok(());
-		}
-
-		let handle = handle.unwrap();
+		let handle = <CollectionHandle<T>>::try_get(collection)?;
 
 		let dispatch = T::CollectionDispatch::dispatch(handle);
 		let dispatch = dispatch.as_dyn();
 
-		action(dispatch, account.1)
+		action(dispatch, token)
 	}
 }