--- a/pallets/common/src/erc.rs +++ b/pallets/common/src/erc.rs @@ -28,8 +28,8 @@ 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::{ --- a/pallets/common/src/eth.rs +++ b/pallets/common/src/eth.rs @@ -80,10 +80,7 @@ if cross_account_id.is_canonical_substrate() { Self::from_sub::(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 @@ 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(&self) -> evm_coder::execution::Result where --- a/pallets/common/src/lib.rs +++ b/pallets/common/src/lib.rs @@ -100,6 +100,7 @@ PropertyValue, PropertyPermission, PropertiesError, + TokenOwnerError, PropertyKeyPermission, TokenData, TrySetProperty, @@ -2134,7 +2135,7 @@ /// 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; + fn token_owner(&self, token: TokenId) -> Result; /// Returns 10 tokens owners in no particular order. /// --- 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 @@ TokenId::default() } - fn token_owner(&self, _token: TokenId) -> Option { - None + fn token_owner(&self, _token: TokenId) -> Result { + Err(TokenOwnerError::MultipleOwners) } /// Returns 10 tokens owners in no particular order. --- a/pallets/nonfungible/src/common.rs +++ b/pallets/nonfungible/src/common.rs @@ -19,7 +19,7 @@ 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 @@ TokenId(>::get(self.id)) } - fn token_owner(&self, token: TokenId) -> Option { - >::get((self.id, token)).map(|t| t.owner) + fn token_owner(&self, token: TokenId) -> Result { + >::get((self.id, token)) + .map(|t| t.owner) + .ok_or(TokenOwnerError::NotFound) } /// Returns token owners. fn token_owners(&self, token: TokenId) -> Vec { - 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 { --- a/pallets/nonfungible/src/erc.rs +++ b/pallets/nonfungible/src/erc.rs @@ -728,7 +728,7 @@ fn cross_owner_of(&self, token_id: U256) -> Result { Self::token_owner(&self, token_id.try_into()?) .map(|o| eth::CrossAddress::from_sub_cross_account::(&o)) - .ok_or(Error::Revert("key too large".into())) + .map_err(|_| Error::Revert("token not found".into())) } /// Returns the token properties. --- a/pallets/proxy-rmrk-core/src/lib.rs +++ b/pallets/proxy-rmrk-core/src/lib.rs @@ -741,7 +741,8 @@ 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::(>::NoPermission.into())?; approval_required = cross_sender != target_nft_owner; @@ -989,7 +990,8 @@ let nft_owner = >::find_topmost_owner(collection_id, nft_id, &budget) - .map_err(|_| >::ResourceDoesntExist)?; + .map_err(|_| >::ResourceDoesntExist)? + .ok_or::(>::NoPermission.into())?; Self::try_mutate_resource_info(collection_id, nft_id, resource_id, |res| { ensure!(res.pending, >::ResourceNotPending); @@ -1044,7 +1046,8 @@ let nft_owner = >::find_topmost_owner(collection_id, nft_id, &budget) - .map_err(|_| >::ResourceDoesntExist)?; + .map_err(|_| >::ResourceDoesntExist)? + .ok_or::(>::NoPermission.into())?; ensure!(cross_sender == nft_owner, >::NoPermission); @@ -1666,7 +1669,8 @@ let budget = budget::Value::new(NESTING_BUDGET); let nft_owner = >::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::(>::NoPermission.into())?; let pending = sender != nft_owner; @@ -1720,7 +1724,8 @@ let budget = up_data_structs::budget::Value::new(NESTING_BUDGET); let topmost_owner = - >::find_topmost_owner(collection_id, nft_id, &budget)?; + >::find_topmost_owner(collection_id, nft_id, &budget)? + .ok_or::(>::NoPermission.into())?; let sender = T::CrossAccountId::from_sub(sender); if topmost_owner == sender { --- a/pallets/proxy-rmrk-core/src/rpc.rs +++ b/pallets/proxy-rmrk-core/src/rpc.rs @@ -68,7 +68,7 @@ } 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 = >::rmrk_collection_id(col)?; @@ -76,7 +76,7 @@ } None => RmrkAccountIdOrCollectionNftTuple::AccountId(owner.as_sub().clone()), }, - None => return Ok(None), + _ => return Ok(None), }; Ok(Some(RmrkInstanceInfo { --- a/pallets/refungible/src/common.rs +++ b/pallets/refungible/src/common.rs @@ -21,7 +21,7 @@ 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 @@ TokenId(>::get(self.id)) } - fn token_owner(&self, token: TokenId) -> Option { + fn token_owner(&self, token: TokenId) -> Result { >::token_owner(self.id, token) } --- a/pallets/refungible/src/erc.rs +++ b/pallets/refungible/src/erc.rs @@ -43,7 +43,7 @@ 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 @@ self.consume_store_reads(2)?; let token = token_id.try_into()?; let owner = >::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 @@ fn cross_owner_of(&self, token_id: U256) -> Result { Self::token_owner(&self, token_id.try_into()?) .map(|o| eth::CrossAddress::from_sub_cross_account::(&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. --- a/pallets/refungible/src/lib.rs +++ b/pallets/refungible/src/lib.rs @@ -107,7 +107,7 @@ 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 @@ >::remove((collection.id, token, owner)); >::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) { >::deposit_log( ERC721Events::Transfer { from: erc::ADDRESS_FOR_PARTIALLY_OWNED_TOKENS, @@ -1365,17 +1365,20 @@ Ok(()) } - fn token_owner(collection_id: CollectionId, token_id: TokenId) -> Option { + fn token_owner( + collection_id: CollectionId, + token_id: TokenId, + ) -> Result { let mut owner = None; let mut count = 0; for key in Balance::::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 { --- a/pallets/structure/src/lib.rs +++ b/pallets/structure/src/lib.rs @@ -61,7 +61,9 @@ 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 @@ 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 @@ 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,19 +208,27 @@ /// /// 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 { + ) -> Result, 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(>::DepthLimit)??; Ok(match owner { - Parent::User(v) => v, + Parent::User(v) => Some(v), + Parent::MultipleOwners => None, _ => fail!(>::TokenNotFound), }) } @@ -223,13 +236,15 @@ /// 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 { + ) -> Result, DispatchError> { // Tried to nest token in itself if Some((collection, token)) == for_nest { return Err(>::OuroborosDetected.into()); @@ -242,8 +257,9 @@ return Err(>::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(>::TokenNotFound.into()), + Parent::MultipleOwners => return Ok(None), // Continue parent chain Parent::Token(_, _) => {} } @@ -284,12 +300,17 @@ budget: &dyn Budget, ) -> Result { 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 --- a/primitives/data-structs/src/lib.rs +++ b/primitives/data-structs/src/lib.rs @@ -1099,6 +1099,13 @@ EmptyPropertyKey, } +/// Token owner error: it could be either `NotFound` ot `MultipleOwners`. +#[derive(Debug)] +pub enum TokenOwnerError { + NotFound, + MultipleOwners, +} + /// Marker for scope of property. /// /// Scoped property can't be changed by user. Used for external collections. --- a/runtime/common/runtime_apis.rs +++ b/runtime/common/runtime_apis.rs @@ -16,11 +16,11 @@ #[macro_export] macro_rules! dispatch_unique_runtime { - ($collection:ident.$method:ident($($name:ident),*)) => {{ + ($collection:ident.$method:ident($($name:ident),*) $($rest:tt)*) => {{ let collection = ::CollectionDispatch::dispatch(>::try_get($collection)?); let dispatch = collection.as_dyn(); - Ok::<_, DispatchError>(dispatch.$method($($name),*)) + Ok::<_, DispatchError>(dispatch.$method($($name),*) $($rest)*) }}; } @@ -73,7 +73,7 @@ } fn token_owner(collection: CollectionId, token: TokenId) -> Result, DispatchError> { - dispatch_unique_runtime!(collection.token_owner(token)) + dispatch_unique_runtime!(collection.token_owner(token).ok()) } fn token_owners(collection: CollectionId, token: TokenId) -> Result, DispatchError> { @@ -83,7 +83,7 @@ fn topmost_token_owner(collection: CollectionId, token: TokenId) -> Result, DispatchError> { let budget = up_data_structs::budget::Value::new(10); - Ok(Some(>::find_topmost_owner(collection, token, &budget)?)) + Ok(>::find_topmost_owner(collection, token, &budget)?) } fn token_children(collection: CollectionId, token: TokenId) -> Result, DispatchError> { Ok(>::token_children_ids(collection, token)) --- a/tests/src/nesting/nest.test.ts +++ b/tests/src/nesting/nest.test.ts @@ -15,7 +15,7 @@ // along with Unique Network. If not, see . import {IKeyringPair} from '@polkadot/types/types'; -import {expect, itSub, usingPlaygrounds} from '../util'; +import {expect, itSub, Pallets, usingPlaygrounds} from '../util'; describe('Integration Test: Composite nesting tests', () => { let alice: IKeyringPair; @@ -138,7 +138,7 @@ before(async () => { await usingPlaygrounds(async (helper, privateKey) => { const donor = await privateKey({filename: __filename}); - [alice, bob, charlie] = await helper.arrange.createAccounts([50n, 10n, 10n], donor); + [alice, bob, charlie] = await helper.arrange.createAccounts([200n, 10n, 10n], donor); }); }); @@ -288,6 +288,38 @@ await collectionFT.transfer(charlie, targetToken.nestingAccount(), 2n); expect(await collectionFT.getBalance(targetToken.nestingAccount())).to.be.equal(7n); }); + + itSub.ifWithPallets('ReFungible: getTopmostOwner works correctly with Nesting', [Pallets.ReFungible], async({helper}) => { + const collectionNFT = await helper.nft.mintCollection(alice, { + permissions: { + nesting: { + tokenOwner: true, + }, + }, + }); + const collectionRFT = await helper.rft.mintCollection(alice); + + const nft = await collectionNFT.mintToken(alice, {Substrate: alice.address}); + const rft = await collectionRFT.mintToken(alice, 100n, {Substrate: alice.address}); + + expect(await rft.getTopmostOwner()).deep.equal({Substrate: alice.address}); + + await rft.transfer(alice, nft.nestingAccount(), 40n); + + expect(await rft.getTopmostOwner()).deep.equal(null); + + await rft.transfer(alice, nft.nestingAccount(), 60n); + + expect(await rft.getTopmostOwner()).deep.equal({Substrate: alice.address}); + + await rft.transferFrom(alice, nft.nestingAccount(), {Substrate: alice.address}, 30n); + + expect(await rft.getTopmostOwner()).deep.equal(null); + + await rft.transferFrom(alice, nft.nestingAccount(), {Substrate: alice.address}, 70n); + + expect(await rft.getTopmostOwner()).deep.equal({Substrate: alice.address}); + }); }); describe('Negative Test: Nesting', () => {