git.delta.rocks / unique-network / refs/commits / 713b8d223b2f

difftreelog

Merge pull request #831 from UniqueNetwork/fix/rft-collection-admin-permissions

Yaroslav Bolyukin2023-01-16parents: #5c9ffb7 #c7a326d.patch.diff
in: master

5 files changed

modifiedpallets/common/src/lib.rsdiffbeforeafterboth
--- a/pallets/common/src/lib.rs
+++ b/pallets/common/src/lib.rs
@@ -390,8 +390,10 @@
 		Ok(())
 	}
 
-	/// Return **true** if `user` was not allowed to have tokens, and he can ignore such restrictions.
-	pub fn ignores_allowance(&self, user: &T::CrossAccountId) -> bool {
+	/// Returns **true** if
+	/// * the `user`is a collection owner or admin
+	/// * the collection limits allow the owner/admins to transfer/burn any collection token
+	pub fn ignores_token_restrictions(&self, user: &T::CrossAccountId) -> bool {
 		self.limits.owner_can_transfer() && self.is_owner_or_admin(user)
 	}
modifiedpallets/fungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/fungible/src/lib.rs
+++ b/pallets/fungible/src/lib.rs
@@ -677,8 +677,14 @@
 			// `from`, `to` checked in [`transfer`]
 			collection.check_allowlist(spender)?;
 		}
+
+		if collection.ignores_token_restrictions(spender) {
+			return Ok(Self::compute_allowance_decrease(
+				collection, from, spender, amount,
+			));
+		}
+
 		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
-			// TODO: should collection owner be allowed to perform this transfer?
 			ensure!(
 				<PalletStructure<T>>::check_indirectly_owned(
 					spender.clone(),
@@ -690,18 +696,25 @@
 				<CommonError<T>>::ApprovedValueTooLow,
 			);
 			return Ok(None);
-		}
-		let allowance = <Allowance<T>>::get((collection.id, from, spender)).checked_sub(amount);
-		if allowance.is_none() {
-			ensure!(
-				collection.ignores_allowance(spender),
-				<CommonError<T>>::ApprovedValueTooLow
-			);
 		}
 
+		let allowance = Self::compute_allowance_decrease(collection, from, spender, amount);
+		ensure!(allowance.is_some(), <CommonError<T>>::ApprovedValueTooLow);
+
 		Ok(allowance)
 	}
 
+	/// Returns `Some(amount)` if the `spender` have allowance to spend this amount.
+	/// Otherwise, it returns `None`.
+	fn compute_allowance_decrease(
+		collection: &FungibleHandle<T>,
+		from: &T::CrossAccountId,
+		spender: &T::CrossAccountId,
+		amount: u128,
+	) -> Option<u128> {
+		<Allowance<T>>::get((collection.id, from, spender)).checked_sub(amount)
+	}
+
 	/// Transfer fungible tokens from one account to another.
 	/// Same as the [`transfer`][`Pallet::transfer`] but spender doesn't needs to be an owner of the token pieces.
 	/// The owner should set allowance for the spender to transfer pieces.
modifiedpallets/nonfungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/nonfungible/src/lib.rs
+++ b/pallets/nonfungible/src/lib.rs
@@ -1246,7 +1246,7 @@
 			collection.check_allowlist(spender)?;
 		}
 
-		if collection.limits.owner_can_transfer() && collection.is_owner_or_admin(spender) {
+		if collection.ignores_token_restrictions(spender) {
 			return Ok(());
 		}
 
@@ -1269,11 +1269,8 @@
 		if <CollectionAllowance<T>>::get((collection.id, from, spender)) {
 			return Ok(());
 		}
-		ensure!(
-			collection.ignores_allowance(spender),
-			<CommonError<T>>::ApprovedValueTooLow
-		);
-		Ok(())
+
+		Err(<CommonError<T>>::ApprovedValueTooLow.into())
 	}
 
 	/// Transfer NFT token from one account to another.
modifiedpallets/refungible/src/lib.rsdiffbeforeafterboth
--- a/pallets/refungible/src/lib.rs
+++ b/pallets/refungible/src/lib.rs
@@ -1177,6 +1177,13 @@
 			// `from`, `to` checked in [`transfer`]
 			collection.check_allowlist(spender)?;
 		}
+
+		if collection.ignores_token_restrictions(spender) {
+			return Ok(Self::compute_allowance_decrease(
+				collection, token, from, &spender, amount,
+			));
+		}
+
 		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
 			// TODO: should collection owner be allowed to perform this transfer?
 			ensure!(
@@ -1191,21 +1198,30 @@
 			);
 			return Ok(None);
 		}
-		let allowance =
-			<Allowance<T>>::get((collection.id, token, from, &spender)).checked_sub(amount);
 
+		let allowance = Self::compute_allowance_decrease(collection, token, from, &spender, amount);
+		if allowance.is_some() {
+			return Ok(allowance);
+		}
+
 		// Allowance (if any) would be reduced if spender is also wallet operator
 		if <CollectionAllowance<T>>::get((collection.id, from, spender)) {
 			return Ok(allowance);
 		}
 
-		if allowance.is_none() {
-			ensure!(
-				collection.ignores_allowance(spender),
-				<CommonError<T>>::ApprovedValueTooLow
-			);
-		}
-		Ok(allowance)
+		Err(<CommonError<T>>::ApprovedValueTooLow.into())
+	}
+
+	/// Returns `Some(amount)` if the `spender` have allowance to spend this amount.
+	/// Otherwise, it returns `None`.
+	fn compute_allowance_decrease(
+		collection: &RefungibleHandle<T>,
+		token: TokenId,
+		from: &T::CrossAccountId,
+		spender: &T::CrossAccountId,
+		amount: u128,
+	) -> Option<u128> {
+		<Allowance<T>>::get((collection.id, token, from, spender)).checked_sub(amount)
 	}
 
 	/// Transfer RFT token pieces from one account to another.
modifiedtests/src/nesting/unnest.test.tsdiffbeforeafterboth
before · tests/src/nesting/unnest.test.ts
1// Copyright 2019-2022 Unique Network (Gibraltar) Ltd.2// This file is part of Unique Network.34// Unique Network is free software: you can redistribute it and/or modify5// it under the terms of the GNU General Public License as published by6// the Free Software Foundation, either version 3 of the License, or7// (at your option) any later version.89// Unique Network is distributed in the hope that it will be useful,10// but WITHOUT ANY WARRANTY; without even the implied warranty of11// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the12// GNU General Public License for more details.1314// You should have received a copy of the GNU General Public License15// along with Unique Network. If not, see <http://www.gnu.org/licenses/>.1617import {IKeyringPair} from '@polkadot/types/types';18import {expect, itSub, Pallets, usingPlaygrounds} from '../util';1920describe('Integration Test: Unnesting', () => {21  let alice: IKeyringPair;2223  before(async () => {24    await usingPlaygrounds(async (helper, privateKey) => {25      const donor = await privateKey({filename: __filename});26      [alice] = await helper.arrange.createAccounts([50n], donor);27    });28  });2930  itSub('NFT: allows the owner to successfully unnest a token', async ({helper}) => {31    const collection = await helper.nft.mintCollection(alice, {permissions: {nesting: {tokenOwner: true}}});32    const targetToken = await collection.mintToken(alice);3334    // Create a nested token35    const nestedToken = await collection.mintToken(alice, targetToken.nestingAccount());3637    // Unnest38    await expect(nestedToken.transferFrom(alice, targetToken.nestingAccount(), {Substrate: alice.address}), 'while unnesting').to.be.fulfilled;39    expect(await nestedToken.getOwner()).to.be.deep.equal({Substrate: alice.address});4041    // Nest and burn42    await nestedToken.nest(alice, targetToken);43    await expect(nestedToken.burnFrom(alice, targetToken.nestingAccount()), 'while burning').to.be.fulfilled;44    await expect(nestedToken.getOwner()).to.be.rejected;45  });4647  itSub('Fungible: allows the owner to successfully unnest a token', async ({helper}) => {48    const collection = await helper.nft.mintCollection(alice, {permissions: {nesting: {tokenOwner: true}}});49    const targetToken = await collection.mintToken(alice);5051    const collectionFT = await helper.ft.mintCollection(alice);5253    // Nest and unnest54    await collectionFT.mint(alice, 10n, targetToken.nestingAccount());55    await expect(collectionFT.transferFrom(alice, targetToken.nestingAccount(), {Substrate: alice.address}, 9n), 'while unnesting').to.be.fulfilled;56    expect(await collectionFT.getBalance({Substrate: alice.address})).to.be.equal(9n);57    expect(await collectionFT.getBalance(targetToken.nestingAccount())).to.be.equal(1n);5859    // Nest and burn60    await collectionFT.transfer(alice, targetToken.nestingAccount(), 5n);61    await expect(collectionFT.burnTokensFrom(alice, targetToken.nestingAccount(), 6n), 'while burning').to.be.fulfilled;62    expect(await collectionFT.getBalance({Substrate: alice.address})).to.be.equal(4n);63    expect(await collectionFT.getBalance(targetToken.nestingAccount())).to.be.equal(0n);64    expect(await targetToken.getChildren()).to.be.length(0);65  });6667  itSub.ifWithPallets('ReFungible: allows the owner to successfully unnest a token', [Pallets.ReFungible], async ({helper}) => {68    const collection = await helper.nft.mintCollection(alice, {permissions: {nesting: {tokenOwner: true}}});69    const targetToken = await collection.mintToken(alice);7071    const collectionRFT = await helper.rft.mintCollection(alice);7273    // Nest and unnest74    const token = await collectionRFT.mintToken(alice, 10n, targetToken.nestingAccount());75    await expect(token.transferFrom(alice, targetToken.nestingAccount(), {Substrate: alice.address}, 9n), 'while unnesting').to.be.fulfilled;76    expect(await token.getBalance({Substrate: alice.address})).to.be.equal(9n);77    expect(await token.getBalance(targetToken.nestingAccount())).to.be.equal(1n);7879    // Nest and burn80    await token.transfer(alice, targetToken.nestingAccount(), 5n);81    await expect(token.burnFrom(alice, targetToken.nestingAccount(), 6n), 'while burning').to.be.fulfilled;82    expect(await token.getBalance({Substrate: alice.address})).to.be.equal(4n);83    expect(await token.getBalance(targetToken.nestingAccount())).to.be.equal(0n);84    expect(await targetToken.getChildren()).to.be.length(0);85  });86});8788describe('Negative Test: Unnesting', () => {89  let alice: IKeyringPair;90  let bob: IKeyringPair;9192  before(async () => {93    await usingPlaygrounds(async (helper, privateKey) => {94      const donor = await privateKey({filename: __filename});95      [alice, bob] = await helper.arrange.createAccounts([50n, 10n], donor);96    });97  });9899  itSub('Disallows a non-owner to unnest/burn a token', async ({helper}) => {100    const collection = await helper.nft.mintCollection(alice, {permissions: {nesting: {tokenOwner: true}}});101    const targetToken = await collection.mintToken(alice);102103    // Create a nested token104    const nestedToken = await collection.mintToken(alice, targetToken.nestingAccount());105106    // Try to unnest107    await expect(nestedToken.unnest(bob, targetToken, {Substrate: alice.address})).to.be.rejectedWith(/common\.ApprovedValueTooLow/);108    expect(await nestedToken.getOwner()).to.be.deep.equal(targetToken.nestingAccount().toLowerCase());109110    // Try to burn111    await expect(nestedToken.burnFrom(bob, targetToken.nestingAccount())).to.be.rejectedWith(/common\.ApprovedValueTooLow/);112    expect(await nestedToken.getOwner()).to.be.deep.equal(targetToken.nestingAccount().toLowerCase());113  });114115  // todo another test for creating excessive depth matryoshka with Ethereum?116117  // Recursive nesting118  itSub('Prevents Ouroboros creation', async ({helper}) => {119    const collection = await helper.nft.mintCollection(alice, {permissions: {nesting: {tokenOwner: true}}});120    const targetToken = await collection.mintToken(alice);121122    // Fail to create a nested token ouroboros123    const nestedToken = await collection.mintToken(alice, targetToken.nestingAccount());124    await expect(targetToken.nest(alice, nestedToken)).to.be.rejectedWith(/^structure\.OuroborosDetected$/);125  });126});