git.delta.rocks / unique-network / refs/commits / bebe624eff63

difftreelog

feat prevent ouroboros creation during nest

Yaroslav Bolyukin2022-04-19parent: #07a6969.patch.diff
in: master

11 files changed

modifiedclient/rpc/src/lib.rsdiffbeforeafterboth
--- a/client/rpc/src/lib.rs
+++ b/client/rpc/src/lib.rs
@@ -77,11 +77,7 @@
 	) -> Result<Vec<u8>>;
 
 	#[rpc(name = "unique_totalSupply")]
-	fn total_supply(
-		&self,
-		collection: CollectionId,
-		at: Option<BlockHash>,
-	) -> Result<u32>;
+	fn total_supply(&self, collection: CollectionId, at: Option<BlockHash>) -> Result<u32>;
 	#[rpc(name = "unique_accountBalance")]
 	fn account_balance(
 		&self,
modifiedpallets/common/src/lib.rsdiffbeforeafterboth
--- a/pallets/common/src/lib.rs
+++ b/pallets/common/src/lib.rs
@@ -918,7 +918,7 @@
 	fn check_nesting(
 		&self,
 		sender: T::CrossAccountId,
-		from: CollectionId,
+		from: (CollectionId, TokenId),
 		under: TokenId,
 		budget: &dyn Budget,
 	) -> DispatchResult;
modifiedpallets/fungible/src/common.rsdiffbeforeafterboth
--- a/pallets/fungible/src/common.rs
+++ b/pallets/fungible/src/common.rs
@@ -237,7 +237,7 @@
 	fn check_nesting(
 		&self,
 		_sender: <T>::CrossAccountId,
-		_from: CollectionId,
+		_from: (CollectionId, TokenId),
 		_under: TokenId,
 		_budget: &dyn Budget,
 	) -> sp_runtime::DispatchResult {
modifiedpallets/fungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/fungible/src/lib.rs
+++ b/pallets/fungible/src/lib.rs
@@ -224,7 +224,12 @@
 			let dispatch = T::CollectionDispatch::dispatch(handle);
 			let dispatch = dispatch.as_dyn();
 
-			dispatch.check_nesting(from.clone(), collection.id, target.1, nesting_budget)?;
+			dispatch.check_nesting(
+				from.clone(),
+				(collection.id, TokenId::default()),
+				target.1,
+				nesting_budget,
+			)?;
 		}
 
 		// =========
@@ -293,7 +298,12 @@
 				let dispatch = T::CollectionDispatch::dispatch(handle);
 				let dispatch = dispatch.as_dyn();
 
-				dispatch.check_nesting(sender.clone(), collection.id, target.1, nesting_budget)?;
+				dispatch.check_nesting(
+					sender.clone(),
+					(collection.id, TokenId::default()),
+					target.1,
+					nesting_budget,
+				)?;
 			}
 		}
 
@@ -386,10 +396,11 @@
 		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
 			// TODO: should collection owner be allowed to perform this transfer?
 			ensure!(
-				<PalletStructure<T>>::indirectly_owned(
+				<PalletStructure<T>>::check_indirectly_owned(
 					spender.clone(),
 					source.0,
 					source.1,
+					None,
 					nesting_budget
 				)?,
 				<CommonError<T>>::ApprovedValueTooLow,
modifiedpallets/nonfungible/src/common.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/common.rs
+++ b/pallets/nonfungible/src/common.rs
@@ -251,7 +251,7 @@
 	fn check_nesting(
 		&self,
 		sender: T::CrossAccountId,
-		from: CollectionId,
+		from: (CollectionId, TokenId),
 		under: TokenId,
 		budget: &dyn Budget,
 	) -> sp_runtime::DispatchResult {
modifiedpallets/nonfungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/lib.rs
+++ b/pallets/nonfungible/src/lib.rs
@@ -296,7 +296,12 @@
 			let dispatch = T::CollectionDispatch::dispatch(handle);
 			let dispatch = dispatch.as_dyn();
 
-			dispatch.check_nesting(from.clone(), collection.id, target.1, nesting_budget)?;
+			dispatch.check_nesting(
+				from.clone(),
+				(collection.id, token),
+				target.1,
+				nesting_budget,
+			)?;
 		}
 
 		// =========
@@ -381,13 +386,18 @@
 			);
 		}
 
-		for (to, _) in balances.iter() {
-			if let Some(target) = T::CrossTokenAddressMapping::address_to_token(to) {
+		for (i, data) in data.iter().enumerate() {
+			let token = TokenId(first_token + i as u32 + 1);
+			if let Some(target) = T::CrossTokenAddressMapping::address_to_token(&data.owner) {
 				let handle = <CollectionHandle<T>>::try_get(target.0)?;
 				let dispatch = T::CollectionDispatch::dispatch(handle);
 				let dispatch = dispatch.as_dyn();
-
-				dispatch.check_nesting(sender.clone(), collection.id, target.1, nesting_budget)?;
+				dispatch.check_nesting(
+					sender.clone(),
+					(collection.id, token),
+					target.1,
+					nesting_budget,
+				)?;
 			}
 		}
 
@@ -535,10 +545,11 @@
 		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
 			// TODO: should collection owner be allowed to perform this transfer?
 			ensure!(
-				<PalletStructure<T>>::indirectly_owned(
+				<PalletStructure<T>>::check_indirectly_owned(
 					spender.clone(),
 					source.0,
 					source.1,
+					None,
 					nesting_budget
 				)?,
 				<CommonError<T>>::ApprovedValueTooLow,
@@ -610,31 +621,40 @@
 	pub fn check_nesting(
 		handle: &NonfungibleHandle<T>,
 		sender: T::CrossAccountId,
-		from: CollectionId,
+		from: (CollectionId, TokenId),
 		under: TokenId,
 		nesting_budget: &dyn Budget,
 	) -> DispatchResult {
 		fn ensure_sender_allowed<T: Config>(
 			collection: CollectionId,
 			token: TokenId,
+			for_nest: (CollectionId, TokenId),
 			sender: T::CrossAccountId,
 			budget: &dyn Budget,
 		) -> DispatchResult {
 			ensure!(
-				<PalletStructure<T>>::indirectly_owned(sender, collection, token, budget)?,
+				<PalletStructure<T>>::check_indirectly_owned(
+					sender,
+					collection,
+					token,
+					Some(for_nest),
+					budget
+				)?,
 				<CommonError<T>>::OnlyOwnerAllowedToNest,
 			);
 			Ok(())
 		}
 		match handle.limits.nesting_rule() {
 			NestingRule::Disabled => fail!(<CommonError<T>>::NestingIsDisabled),
-			NestingRule::Owner => ensure_sender_allowed::<T>(handle.id, under, sender, nesting_budget)?,
+			NestingRule::Owner => {
+				ensure_sender_allowed::<T>(handle.id, under, from, sender, nesting_budget)?
+			}
 			NestingRule::OwnerRestricted(whitelist) => {
 				ensure!(
-					whitelist.contains(&from),
+					whitelist.contains(&from.0),
 					<CommonError<T>>::SourceCollectionIsNotAllowedToNest
 				);
-				ensure_sender_allowed::<T>(handle.id, under, sender, nesting_budget)?
+				ensure_sender_allowed::<T>(handle.id, under, from, sender, nesting_budget)?
 			}
 		}
 		Ok(())
modifiedpallets/refungible/src/common.rsdiffbeforeafterboth
--- a/pallets/refungible/src/common.rs
+++ b/pallets/refungible/src/common.rs
@@ -260,7 +260,7 @@
 	fn check_nesting(
 		&self,
 		_sender: <T>::CrossAccountId,
-		_from: CollectionId,
+		_from: (CollectionId, TokenId),
 		_under: TokenId,
 		_budget: &dyn Budget,
 	) -> sp_runtime::DispatchResult {
modifiedpallets/refungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/refungible/src/lib.rs
+++ b/pallets/refungible/src/lib.rs
@@ -352,7 +352,12 @@
 			let dispatch = T::CollectionDispatch::dispatch(handle);
 			let dispatch = dispatch.as_dyn();
 
-			dispatch.check_nesting(from.clone(), collection.id, target.1, nesting_budget)?;
+			dispatch.check_nesting(
+				from.clone(),
+				(collection.id, token),
+				target.1,
+				nesting_budget,
+			)?;
 		}
 
 		// =========
@@ -455,7 +460,8 @@
 			}
 		}
 
-		for token in data.iter() {
+		for (i, token) in data.iter().enumerate() {
+			let token_id = TokenId(first_token_id + i as u32 + 1);
 			for (to, _) in token.users.iter() {
 				if let Some(target) = T::CrossTokenAddressMapping::address_to_token(to) {
 					let handle = <CollectionHandle<T>>::try_get(target.0)?;
@@ -464,7 +470,7 @@
 
 					dispatch.check_nesting(
 						sender.clone(),
-						collection.id,
+						(collection.id, token_id),
 						target.1,
 						nesting_budget,
 					)?;
@@ -575,10 +581,11 @@
 		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
 			// TODO: should collection owner be allowed to perform this transfer?
 			ensure!(
-				<PalletStructure<T>>::indirectly_owned(
+				<PalletStructure<T>>::check_indirectly_owned(
 					spender.clone(),
 					source.0,
 					source.1,
+					None,
 					nesting_budget
 				)?,
 				<CommonError<T>>::ApprovedValueTooLow,
modifiedpallets/structure/src/lib.rsdiffbeforeafterboth
--- a/pallets/structure/src/lib.rs
+++ b/pallets/structure/src/lib.rs
@@ -71,7 +71,7 @@
 #[derive(PartialEq)]
 pub enum Parent<CrossAccountId> {
 	/// Token owned by normal account
-	Normal(CrossAccountId),
+	User(CrossAccountId),
 	/// Passed token not found
 	TokenNotFound,
 	/// Token owner is another token (target token still may not exist)
@@ -94,7 +94,7 @@
 		Ok(match handle.token_owner(token) {
 			Some(owner) => match T::CrossTokenAddressMapping::address_to_token(&owner) {
 				Some((collection, token)) => Parent::Token(collection, token),
-				None => Parent::Normal(owner),
+				None => Parent::User(owner),
 			},
 			None => Parent::TokenNotFound,
 		})
@@ -137,29 +137,46 @@
 	) -> Result<T::CrossAccountId, DispatchError> {
 		let owner = Self::parent_chain(collection, token)
 			.take_while(|_| budget.consume())
-			.find(|p| matches!(p, Ok(Parent::Normal(_) | Parent::TokenNotFound)))
+			.find(|p| matches!(p, Ok(Parent::User(_) | Parent::TokenNotFound)))
 			.ok_or(<Error<T>>::DepthLimit)??;
 
 		Ok(match owner {
-			Parent::Normal(v) => v,
+			Parent::User(v) => v,
 			_ => fail!(<Error<T>>::TokenNotFound),
 		})
 	}
 
 	/// Check if token indirectly owned by specified user
-	pub fn indirectly_owned(
+	pub fn check_indirectly_owned(
 		user: T::CrossAccountId,
 		collection: CollectionId,
 		token: TokenId,
+		for_nest: Option<(CollectionId, TokenId)>,
 		budget: &dyn Budget,
 	) -> Result<bool, DispatchError> {
 		let target_parent = match T::CrossTokenAddressMapping::address_to_token(&user) {
 			Some((collection, token)) => Parent::Token(collection, token),
-			None => Parent::Normal(user),
+			None => Parent::User(user),
 		};
 
-		Ok(Self::parent_chain(collection, token)
-			.take_while(|_| budget.consume())
-			.any(|parent| Ok(&target_parent) == parent.as_ref()))
+		// Tried to nest token in itself
+		if Some((collection, token)) == for_nest {
+			return Err(<Error<T>>::OuroborosDetected.into());
+		}
+
+		for parent in Self::parent_chain(collection, token).take_while(|_| budget.consume()) {
+			match parent? {
+				// Tried to nest token in chain, which has this token as one of parents
+				Parent::Token(collection, token) if Some((collection, token)) == for_nest => {
+					return Err(<Error<T>>::OuroborosDetected.into())
+				}
+				// Found needed parent, token is indirecty owned
+				v if v == target_parent => return Ok(true),
+				Parent::TokenNotFound => return Ok(false),
+				_ => {}
+			}
+		}
+
+		Err(<Error<T>>::DepthLimit.into())
 	}
 }
modifiedtests/src/eth/util/helpers.tsdiffbeforeafterboth
--- a/tests/src/eth/util/helpers.ts
+++ b/tests/src/eth/util/helpers.ts
@@ -23,7 +23,7 @@
 import usingApi, {submitTransactionAsync} from '../../substrate/substrate-api';
 import {IKeyringPair} from '@polkadot/types/types';
 import {expect} from 'chai';
-import {getGenericResult, UNIQUE} from '../../util/helpers';
+import {CrossAccountId, getGenericResult, UNIQUE} from '../../util/helpers';
 import * as solc from 'solc';
 import config from '../../config';
 import privateKey from '../../substrate/privateKey';
@@ -80,6 +80,11 @@
   ]);
   return Web3.utils.toChecksumAddress('0x' + buf.toString('hex'));
 }
+export function tokenIdToCross(collection: number, token: number): CrossAccountId {
+  return {
+    Ethereum: tokenIdToAddress(collection, token),
+  };
+}
 
 export function createEthAccount(web3: Web3) {
   const account = web3.eth.accounts.create();
addedtests/src/nesting/graphs.test.tsdiffbeforeafterboth
after · tests/src/nesting/graphs.test.ts
1import {ApiPromise} from '@polkadot/api';2import {IKeyringPair} from '@polkadot/types/types';3import {expect} from 'chai';4import {tokenIdToCross} from '../eth/util/helpers';5import privateKey from '../substrate/privateKey';6import usingApi, {executeTransaction} from '../substrate/substrate-api';7import {getCreateCollectionResult, transferExpectSuccess} from '../util/helpers';89/**10 * ```dot11 * 4 -> 3 -> 2 -> 112 * 7 -> 6 -> 5 -> 213 * 8 -> 514 * ```15 */16async function buildComplexObjectGraph(api: ApiPromise, sender: IKeyringPair): Promise<number> {17  const events = await executeTransaction(api, sender, api.tx.unique.createCollectionEx({mode: 'NFT'}));18  const {collectionId} = getCreateCollectionResult(events);1920  await executeTransaction(api, sender, api.tx.unique.createMultipleItemsEx(collectionId, {NFT: Array(8).fill({owner: {Substrate: sender.address}})}));2122  await transferExpectSuccess(collectionId, 8, sender, tokenIdToCross(collectionId, 5));2324  await transferExpectSuccess(collectionId, 7, sender, tokenIdToCross(collectionId, 6));25  await transferExpectSuccess(collectionId, 6, sender, tokenIdToCross(collectionId, 5));26  await transferExpectSuccess(collectionId, 5, sender, tokenIdToCross(collectionId, 2));2728  await transferExpectSuccess(collectionId, 4, sender, tokenIdToCross(collectionId, 3));29  await transferExpectSuccess(collectionId, 3, sender, tokenIdToCross(collectionId, 2));30  await transferExpectSuccess(collectionId, 2, sender, tokenIdToCross(collectionId, 1));3132  return collectionId;33}3435describe('graphs', () => {36  it('ouroboros can\'t be created in graph', async () => {37    await usingApi(async api => {38      const alice = privateKey('//Alice');39      const collection = await buildComplexObjectGraph(api, alice);4041      // to self42      await expect(executeTransaction(api, alice, api.tx.unique.transfer(tokenIdToCross(collection, 1), collection, 1, 1)))43        .to.be.rejectedWith(/structure\.OuroborosDetected/);44      // to nested part of graph45      await expect(executeTransaction(api, alice, api.tx.unique.transfer(tokenIdToCross(collection, 5), collection, 1, 1)))46        .to.be.rejectedWith(/structure\.OuroborosDetected/);47      await expect(executeTransaction(api, alice, api.tx.unique.transfer(tokenIdToCross(collection, 8), collection, 2, 1)))48        .to.be.rejectedWith(/structure\.OuroborosDetected/);49    });50  });51});