Merge pull request #848 from UniqueNetwork/fix/find-parent

CertainLach · web-flow · commit 24d3f67bfdff · 2023-01-19T18:59:49.000+01:00
fix: find_parent
diff --git a/pallets/common/src/erc.rs b/pallets/common/src/erc.rs
@@ -28,8 +28,8 @@ use pallet_evm_coder_substrate::dispatch_to_evm;
 use sp_std::{vec, vec::Vec};
 use sp_core::U256;
 use up_data_structs::{
-	AccessMode, CollectionMode, CollectionPermissions, OwnerRestrictedSet, Property,
-	SponsoringRateLimit, SponsorshipState,
+	CollectionMode, CollectionPermissions, OwnerRestrictedSet, Property, SponsoringRateLimit,
+	SponsorshipState,
 };
 
 use crate::{
diff --git a/pallets/common/src/eth.rs b/pallets/common/src/eth.rs
@@ -80,10 +80,7 @@ impl CrossAddress {
 		if cross_account_id.is_canonical_substrate() {
 			Self::from_sub::<T>(cross_account_id.as_sub())
 		} else {
-			Self {
-				eth: *cross_account_id.as_eth(),
-				sub: Default::default(),
-			}
+			Self::from_eth(*cross_account_id.as_eth())
 		}
 	}
 	/// Creates [`CrossAddress`] from Substrate account.
@@ -97,6 +94,13 @@ impl CrossAddress {
 			sub: U256::from_big_endian(account_id.as_ref()),
 		}
 	}
+	/// Creates [`CrossAddress`] from Ethereum account.
+	pub fn from_eth(address: Address) -> Self {
+		Self {
+			eth: address,
+			sub: Default::default(),
+		}
+	}
 	/// Converts [`CrossAddress`] to `CrossAccountId`.
 	pub fn into_sub_cross_account<T>(&self) -> evm_coder::execution::Result<T::CrossAccountId>
 	where
diff --git a/pallets/common/src/lib.rs b/pallets/common/src/lib.rs
@@ -100,6 +100,7 @@ use up_data_structs::{
 	PropertyValue,
 	PropertyPermission,
 	PropertiesError,
+	TokenOwnerError,
 	PropertyKeyPermission,
 	TokenData,
 	TrySetProperty,
@@ -2134,7 +2135,7 @@ pub trait CommonCollectionOperations<T: Config> {
 	/// Get the owner of the token.
 	///
 	/// * `token` - The token for which you need to find out the owner.
-	fn token_owner(&self, token: TokenId) -> Option<T::CrossAccountId>;
+	fn token_owner(&self, token: TokenId) -> Result<T::CrossAccountId, TokenOwnerError>;
 
 	/// Returns 10 tokens owners in no particular order.
 	///
diff --git a/pallets/fungible/src/common.rs b/pallets/fungible/src/common.rs
@@ -17,7 +17,9 @@
 use core::marker::PhantomData;
 
 use frame_support::{dispatch::DispatchResultWithPostInfo, ensure, fail, weights::Weight, traits::Get};
-use up_data_structs::{TokenId, CollectionId, CreateItemExData, budget::Budget, CreateItemData};
+use up_data_structs::{
+	TokenId, CollectionId, CreateItemExData, budget::Budget, CreateItemData, TokenOwnerError,
+};
 use pallet_common::{
 	CommonCollectionOperations, CommonWeightInfo, RefungibleExtensions, with_weight,
 	weights::WeightInfo as _,
@@ -404,8 +406,8 @@ impl<T: Config> CommonCollectionOperations<T> for FungibleHandle<T> {
 		TokenId::default()
 	}
 
-	fn token_owner(&self, _token: TokenId) -> Option<T::CrossAccountId> {
-		None
+	fn token_owner(&self, _token: TokenId) -> Result<T::CrossAccountId, TokenOwnerError> {
+		Err(TokenOwnerError::MultipleOwners)
 	}
 
 	/// Returns 10 tokens owners in no particular order.
diff --git a/pallets/nonfungible/src/common.rs b/pallets/nonfungible/src/common.rs
@@ -19,7 +19,7 @@ use core::marker::PhantomData;
 use frame_support::{dispatch::DispatchResultWithPostInfo, ensure, fail, weights::Weight};
 use up_data_structs::{
 	TokenId, CreateItemExData, CollectionId, budget::Budget, Property, PropertyKey,
-	PropertyKeyPermission, PropertyValue,
+	PropertyKeyPermission, PropertyValue, TokenOwnerError,
 };
 use pallet_common::{
 	CommonCollectionOperations, CommonWeightInfo, RefungibleExtensions, with_weight,
@@ -460,13 +460,15 @@ impl<T: Config> CommonCollectionOperations<T> for NonfungibleHandle<T> {
 		TokenId(<TokensMinted<T>>::get(self.id))
 	}
 
-	fn token_owner(&self, token: TokenId) -> Option<T::CrossAccountId> {
-		<TokenData<T>>::get((self.id, token)).map(|t| t.owner)
+	fn token_owner(&self, token: TokenId) -> Result<T::CrossAccountId, TokenOwnerError> {
+		<TokenData<T>>::get((self.id, token))
+			.map(|t| t.owner)
+			.ok_or(TokenOwnerError::NotFound)
 	}
 
 	/// Returns token owners.
 	fn token_owners(&self, token: TokenId) -> Vec<T::CrossAccountId> {
-		self.token_owner(token).map_or_else(|| vec![], |t| vec![t])
+		self.token_owner(token).map_or_else(|_| vec![], |t| vec![t])
 	}
 
 	fn token_property(&self, token_id: TokenId, key: &PropertyKey) -> Option<PropertyValue> {
diff --git a/pallets/nonfungible/src/erc.rs b/pallets/nonfungible/src/erc.rs
@@ -728,7 +728,7 @@ where
 	fn cross_owner_of(&self, token_id: U256) -> Result<eth::CrossAddress> {
 		Self::token_owner(&self, token_id.try_into()?)
 			.map(|o| eth::CrossAddress::from_sub_cross_account::<T>(&o))
-			.ok_or(Error::Revert("key too large".into()))
+			.map_err(|_| Error::Revert("token not found".into()))
 	}
 
 	/// Returns the token properties.
diff --git a/pallets/proxy-rmrk-core/src/lib.rs b/pallets/proxy-rmrk-core/src/lib.rs
@@ -741,7 +741,8 @@ pub mod pallet {
 						Some((collection_id, nft_id)),
 						&target_nft_budget,
 					)
-					.map_err(Self::map_unique_err_to_proxy)?;
+					.map_err(Self::map_unique_err_to_proxy)?
+					.ok_or::<DispatchError>(<Error<T>>::NoPermission.into())?;
 
 					approval_required = cross_sender != target_nft_owner;
 
@@ -989,7 +990,8 @@ pub mod pallet {
 
 			let nft_owner =
 				<PalletStructure<T>>::find_topmost_owner(collection_id, nft_id, &budget)
-					.map_err(|_| <Error<T>>::ResourceDoesntExist)?;
+					.map_err(|_| <Error<T>>::ResourceDoesntExist)?
+					.ok_or::<DispatchError>(<Error<T>>::NoPermission.into())?;
 
 			Self::try_mutate_resource_info(collection_id, nft_id, resource_id, |res| {
 				ensure!(res.pending, <Error<T>>::ResourceNotPending);
@@ -1044,7 +1046,8 @@ pub mod pallet {
 
 			let nft_owner =
 				<PalletStructure<T>>::find_topmost_owner(collection_id, nft_id, &budget)
-					.map_err(|_| <Error<T>>::ResourceDoesntExist)?;
+					.map_err(|_| <Error<T>>::ResourceDoesntExist)?
+					.ok_or::<DispatchError>(<Error<T>>::NoPermission.into())?;
 
 			ensure!(cross_sender == nft_owner, <Error<T>>::NoPermission);
 
@@ -1666,7 +1669,8 @@ impl<T: Config> Pallet<T> {
 		let budget = budget::Value::new(NESTING_BUDGET);
 
 		let nft_owner = <PalletStructure<T>>::find_topmost_owner(collection_id, nft_id, &budget)
-			.map_err(Self::map_unique_err_to_proxy)?;
+			.map_err(Self::map_unique_err_to_proxy)?
+			.ok_or::<DispatchError>(<Error<T>>::NoPermission.into())?;
 
 		let pending = sender != nft_owner;
 
@@ -1720,7 +1724,8 @@ impl<T: Config> Pallet<T> {
 
 		let budget = up_data_structs::budget::Value::new(NESTING_BUDGET);
 		let topmost_owner =
-			<PalletStructure<T>>::find_topmost_owner(collection_id, nft_id, &budget)?;
+			<PalletStructure<T>>::find_topmost_owner(collection_id, nft_id, &budget)?
+				.ok_or::<DispatchError>(<Error<T>>::NoPermission.into())?;
 
 		let sender = T::CrossAccountId::from_sub(sender);
 		if topmost_owner == sender {
diff --git a/pallets/proxy-rmrk-core/src/rpc.rs b/pallets/proxy-rmrk-core/src/rpc.rs
@@ -68,15 +68,15 @@ pub fn nft_by_id<T: Config>(
 	}
 
 	let owner = match collection.token_owner(nft_id) {
-		Some(owner) => match T::CrossTokenAddressMapping::address_to_token(&owner) {
+		Ok(owner) => match T::CrossTokenAddressMapping::address_to_token(&owner) {
 			Some((col, tok)) => {
 				let rmrk_collection = <Pallet<T>>::rmrk_collection_id(col)?;
 
 				RmrkAccountIdOrCollectionNftTuple::CollectionAndNftTuple(rmrk_collection, tok.0)
 			}
 			None => RmrkAccountIdOrCollectionNftTuple::AccountId(owner.as_sub().clone()),
 		},
-		None => return Ok(None),
+		_ => return Ok(None),
 	};
 
 	Ok(Some(RmrkInstanceInfo {
diff --git a/pallets/refungible/src/common.rs b/pallets/refungible/src/common.rs
@@ -21,7 +21,7 @@ use frame_support::{dispatch::DispatchResultWithPostInfo, ensure, fail, weights:
 use up_data_structs::{
 	CollectionId, TokenId, CreateItemExData, budget::Budget, Property, PropertyKey, PropertyValue,
 	PropertyKeyPermission, CollectionPropertiesVec, CreateRefungibleExMultipleOwners,
-	CreateRefungibleExSingleOwner,
+	CreateRefungibleExSingleOwner, TokenOwnerError,
 };
 use pallet_common::{
 	CommonCollectionOperations, CommonWeightInfo, RefungibleExtensions, with_weight,
@@ -478,7 +478,7 @@ impl<T: Config> CommonCollectionOperations<T> for RefungibleHandle<T> {
 		TokenId(<TokensMinted<T>>::get(self.id))
 	}
 
-	fn token_owner(&self, token: TokenId) -> Option<T::CrossAccountId> {
+	fn token_owner(&self, token: TokenId) -> Result<T::CrossAccountId, TokenOwnerError> {
 		<Pallet<T>>::token_owner(self.id, token)
 	}
 
diff --git a/pallets/refungible/src/erc.rs b/pallets/refungible/src/erc.rs
@@ -43,7 +43,7 @@ use sp_core::{H160, U256, Get};
 use sp_std::{collections::btree_map::BTreeMap, vec::Vec, vec};
 use up_data_structs::{
 	CollectionId, CollectionPropertiesVec, mapping::TokenAddressMapping, Property, PropertyKey,
-	PropertyKeyPermission, PropertyPermission, TokenId,
+	PropertyKeyPermission, PropertyPermission, TokenId, TokenOwnerError,
 };
 
 use crate::{
@@ -411,9 +411,12 @@ impl<T: Config> RefungibleHandle<T> {
 		self.consume_store_reads(2)?;
 		let token = token_id.try_into()?;
 		let owner = <Pallet<T>>::token_owner(self.id, token);
-		Ok(owner
+		owner
 			.map(|address| *address.as_eth())
-			.unwrap_or_else(|| ADDRESS_FOR_PARTIALLY_OWNED_TOKENS))
+			.or_else(|err| match err {
+				TokenOwnerError::NotFound => Err(Error::Revert("token not found".into())),
+				TokenOwnerError::MultipleOwners => Ok(ADDRESS_FOR_PARTIALLY_OWNED_TOKENS),
+			})
 	}
 
 	/// @dev Not implemented
@@ -766,7 +769,12 @@ where
 	fn cross_owner_of(&self, token_id: U256) -> Result<eth::CrossAddress> {
 		Self::token_owner(&self, token_id.try_into()?)
 			.map(|o| eth::CrossAddress::from_sub_cross_account::<T>(&o))
-			.ok_or(Error::Revert("key too large".into()))
+			.or_else(|err| match err {
+				TokenOwnerError::NotFound => Err(Error::Revert("token not found".into())),
+				TokenOwnerError::MultipleOwners => Ok(eth::CrossAddress::from_eth(
+					ADDRESS_FOR_PARTIALLY_OWNED_TOKENS,
+				)),
+			})
 	}
 
 	/// Returns the token properties.
diff --git a/pallets/refungible/src/lib.rs b/pallets/refungible/src/lib.rs
@@ -107,7 +107,7 @@ use up_data_structs::{
 	AccessMode, budget::Budget, CollectionId, CollectionFlags, CreateCollectionData,
 	mapping::TokenAddressMapping, MAX_REFUNGIBLE_PIECES, Property, PropertyKey,
 	PropertyKeyPermission, PropertyPermission, PropertyScope, PropertyValue, TokenId,
-	TrySetProperty, PropertiesPermissionMap, CreateRefungibleExMultipleOwners,
+	TrySetProperty, PropertiesPermissionMap, CreateRefungibleExMultipleOwners, TokenOwnerError,
 };
 
 pub use pallet::*;
@@ -480,7 +480,7 @@ impl<T: Config> Pallet<T> {
 			<Balance<T>>::remove((collection.id, token, owner));
 			<AccountBalance<T>>::insert((collection.id, owner), account_balance);
 
-			if let Some(user) = Self::token_owner(collection.id, token) {
+			if let Ok(user) = Self::token_owner(collection.id, token) {
 				<PalletEvm<T>>::deposit_log(
 					ERC721Events::Transfer {
 						from: erc::ADDRESS_FOR_PARTIALLY_OWNED_TOKENS,
@@ -1365,17 +1365,20 @@ impl<T: Config> Pallet<T> {
 		Ok(())
 	}
 
-	fn token_owner(collection_id: CollectionId, token_id: TokenId) -> Option<T::CrossAccountId> {
+	fn token_owner(
+		collection_id: CollectionId,
+		token_id: TokenId,
+	) -> Result<T::CrossAccountId, TokenOwnerError> {
 		let mut owner = None;
 		let mut count = 0;
 		for key in Balance::<T>::iter_key_prefix((collection_id, token_id)) {
 			count += 1;
 			if count > 1 {
-				return None;
+				return Err(TokenOwnerError::MultipleOwners);
 			}
 			owner = Some(key);
 		}
-		owner
+		owner.ok_or(TokenOwnerError::NotFound)
 	}
 
 	fn total_pieces(collection_id: CollectionId, token_id: TokenId) -> Option<u128> {
diff --git a/pallets/structure/src/lib.rs b/pallets/structure/src/lib.rs
@@ -61,7 +61,9 @@ use frame_support::dispatch::{DispatchError, DispatchResult, DispatchResultWithP
 use frame_support::fail;
 pub use pallet::*;
 use pallet_common::{dispatch::CollectionDispatch, CollectionHandle};
-use up_data_structs::{CollectionId, TokenId, mapping::TokenAddressMapping, budget::Budget};
+use up_data_structs::{
+	CollectionId, TokenId, mapping::TokenAddressMapping, budget::Budget, TokenOwnerError,
+};
 
 #[cfg(feature = "runtime-benchmarks")]
 pub mod benchmarking;
@@ -135,6 +137,8 @@ pub enum Parent<CrossAccountId> {
 	User(CrossAccountId),
 	/// Could not find the token provided as the owner.
 	TokenNotFound,
+	/// Nested token has multiple owners.
+	MultipleOwners,
 	/// Token owner is another token (still, the target token may not exist).
 	Token(CollectionId, TokenId),
 }
@@ -159,11 +163,12 @@ impl<T: Config> Pallet<T> {
 		let handle = handle.as_dyn();
 
 		Ok(match handle.token_owner(token) {
-			Some(owner) => match T::CrossTokenAddressMapping::address_to_token(&owner) {
+			Ok(owner) => match T::CrossTokenAddressMapping::address_to_token(&owner) {
 				Some((collection, token)) => Parent::Token(collection, token),
 				None => Parent::User(owner),
 			},
-			None => Parent::TokenNotFound,
+			Err(TokenOwnerError::MultipleOwners) => Parent::MultipleOwners,
+			Err(TokenOwnerError::NotFound) => Parent::TokenNotFound,
 		})
 	}
 
@@ -203,33 +208,43 @@ impl<T: Config> Pallet<T> {
 	///
 	/// May return token address if parent token not yet exists
 	///
+	/// Returns `None` if the token has multiple owners.
+	///
 	/// - `budget`: Limit for searching parents in depth.
 	pub fn find_topmost_owner(
 		collection: CollectionId,
 		token: TokenId,
 		budget: &dyn Budget,
-	) -> Result<T::CrossAccountId, DispatchError> {
+	) -> Result<Option<T::CrossAccountId>, DispatchError> {
 		let owner = Self::parent_chain(collection, token)
 			.take_while(|_| budget.consume())
-			.find(|p| matches!(p, Ok(Parent::User(_) | Parent::TokenNotFound)))
+			.find(|p| {
+				matches!(
+					p,
+					Ok(Parent::User(_) | Parent::TokenNotFound | Parent::MultipleOwners)
+				)
+			})
 			.ok_or(<Error<T>>::DepthLimit)??;
 
 		Ok(match owner {
-			Parent::User(v) => v,
+			Parent::User(v) => Some(v),
+			Parent::MultipleOwners => None,
 			_ => fail!(<Error<T>>::TokenNotFound),
 		})
 	}
 
 	/// Find the topmost parent and check that assigning `for_nest` token as a child for
 	/// `token` wouldn't create a cycle.
 	///
+	/// Returns `None` if the token has multiple owners.
+	///
 	/// - `budget`: Limit for searching parents in depth.
 	pub fn get_checked_topmost_owner(
 		collection: CollectionId,
 		token: TokenId,
 		for_nest: Option<(CollectionId, TokenId)>,
 		budget: &dyn Budget,
-	) -> Result<T::CrossAccountId, DispatchError> {
+	) -> Result<Option<T::CrossAccountId>, DispatchError> {
 		// Tried to nest token in itself
 		if Some((collection, token)) == for_nest {
 			return Err(<Error<T>>::OuroborosDetected.into());
@@ -242,8 +257,9 @@ impl<T: Config> Pallet<T> {
 					return Err(<Error<T>>::OuroborosDetected.into())
 				}
 				// Token is owned by other user
-				Parent::User(user) => return Ok(user),
+				Parent::User(user) => return Ok(Some(user)),
 				Parent::TokenNotFound => return Err(<Error<T>>::TokenNotFound.into()),
+				Parent::MultipleOwners => return Ok(None),
 				// Continue parent chain
 				Parent::Token(_, _) => {}
 			}
@@ -284,12 +300,17 @@ impl<T: Config> Pallet<T> {
 		budget: &dyn Budget,
 	) -> Result<bool, DispatchError> {
 		let target_parent = match T::CrossTokenAddressMapping::address_to_token(&user) {
-			Some((collection, token)) => Self::find_topmost_owner(collection, token, budget)?,
+			Some((collection, token)) => match Self::find_topmost_owner(collection, token, budget)?
+			{
+				Some(topmost_owner) => topmost_owner,
+				None => return Ok(false),
+			},
 			None => user,
 		};
 
-		Self::get_checked_topmost_owner(collection, token, for_nest, budget)
-			.map(|indirect_owner| indirect_owner == target_parent)
+		Self::get_checked_topmost_owner(collection, token, for_nest, budget).map(|indirect_owner| {
+			indirect_owner.map_or(false, |indirect_owner| indirect_owner == target_parent)
+		})
 	}
 
 	/// Checks that `under` is valid token and that `token_id` could be nested under it
diff --git a/primitives/data-structs/src/lib.rs b/primitives/data-structs/src/lib.rs
diff --git a/runtime/common/runtime_apis.rs b/runtime/common/runtime_apis.rs
diff --git a/tests/src/nesting/nest.test.ts b/tests/src/nesting/nest.test.ts

Original file line number	Diff line number	Diff line change
`@@ -728,7 +728,7 @@ where`
`728`	`728`	`fn cross_owner_of(&self, token_id: U256) -> Result<eth::CrossAddress> {`
`729`	`729`	`Self::token_owner(&self, token_id.try_into()?)`
`730`	`730`	`.map(\|o\| eth::CrossAddress::from_sub_cross_account::<T>(&o))`
`731`		`- .ok_or(Error::Revert("key too large".into()))`
	`731`	`+ .map_err(\|_\| Error::Revert("token not found".into()))`
`732`	`732`	`}`
`733`	`733`
`734`	`734`	`/// Returns the token properties.`