git.delta.rocks / unique-network / refs/commits / 956ec6eac126

difftreelog

Merge pull request #410 from UniqueNetwork/fix/admin-transfer

kozyrevdev2022-07-01parents: #cde697e #63f7000.patch.diff
in: master
Fix/admin transfer

7 files changed

modifiedpallets/nonfungible/src/lib.rsdiffbeforeafterboth
381			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;381			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;
382		ensure!(382		ensure!(&token_data.owner == sender, <CommonError<T>>::NoPermission);
383			&token_data.owner == sender
384				|| (collection.limits.owner_can_transfer() && collection.is_owner_or_admin(sender)),
385			<CommonError<T>>::NoPermission
386		);
387383
665661
666		let token_data =662		let token_data =
667			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;663			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;
668		// TODO: require sender to be token, owner, require admins to go through transfer_from
669		ensure!(664		ensure!(&token_data.owner == from, <CommonError<T>>::NoPermission);
670			&token_data.owner == from
671				|| (collection.limits.owner_can_transfer() && collection.is_owner_or_admin(from)),
672			<CommonError<T>>::NoPermission
673		);
674665
977			collection.check_allowlist(spender)?;968			collection.check_allowlist(spender)?;
978		}969		}
970
971		if collection.limits.owner_can_transfer() && collection.is_owner_or_admin(spender) {
972			return Ok(());
973		}
974
979		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {975		if let Some(source) = T::CrossTokenAddressMapping::address_to_token(from) {
980			// TODO: should collection owner be allowed to perform this transfer?
981			ensure!(976			ensure!(
982				<PalletStructure<T>>::check_indirectly_owned(977				<PalletStructure<T>>::check_indirectly_owned(
983					spender.clone(),978					spender.clone(),
modifiedtests/package.jsondiffbeforeafterboth
--- a/tests/package.json
+++ b/tests/package.json
@@ -60,6 +60,7 @@
     "testAddToContractAllowList": "mocha --timeout 9999999 -r ts-node/register ./**/addToContractAllowList.test.ts",
     "testTransfer": "mocha --timeout 9999999 -r ts-node/register ./**/transfer.test.ts",
     "testBurnItem": "mocha --timeout 9999999 -r ts-node/register ./**/burnItem.test.ts",
+    "testAdminTransferAndBurn": "mocha --timeout 9999999 -r ts-node/register ./**/adminTransferAndBurn.test.ts",
     "testSetMintPermission": "mocha --timeout 9999999 -r ts-node/register ./**/setMintPermission.test.ts",
     "testCreditFeesToTreasury": "mocha --timeout 9999999 -r ts-node/register ./**/creditFeesToTreasury.test.ts",
     "testContractSponsoring": "mocha --timeout 9999999 -r ts-node/register ./**/contractSponsoring.test.ts",
addedtests/src/adminTransferAndBurn.test.tsdiffbeforeafterboth
--- /dev/null
+++ b/tests/src/adminTransferAndBurn.test.ts
@@ -0,0 +1,72 @@
+// Copyright 2019-2022 Unique Network (Gibraltar) Ltd.
+// This file is part of Unique Network.
+
+// Unique Network is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Unique Network is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Unique Network. If not, see <http://www.gnu.org/licenses/>.
+
+import {IKeyringPair} from '@polkadot/types/types';
+import chai from 'chai';
+import chaiAsPromised from 'chai-as-promised';
+import {default as usingApi} from './substrate/substrate-api';
+import {
+  createCollectionExpectSuccess,
+  createItemExpectSuccess,
+  transferExpectFailure,
+  transferFromExpectSuccess,
+  burnItemExpectFailure,
+  burnFromExpectSuccess,
+  setCollectionLimitsExpectSuccess,
+} from './util/helpers';
+
+chai.use(chaiAsPromised);
+const expect = chai.expect;
+
+describe('Integration Test: ownerCanTransfer allows admins to use only transferFrom/burnFrom:', () => {
+  let alice: IKeyringPair;
+  let bob: IKeyringPair;
+  let charlie: IKeyringPair;
+
+  before(async () => {
+    await usingApi(async (api, privateKeyWrapper) => {
+      alice = privateKeyWrapper('//Alice');
+      bob = privateKeyWrapper('//Bob');
+      charlie = privateKeyWrapper('//Charlie');
+    });
+  });
+
+  it('admin transfers other user\'s token', async () => {
+    await usingApi(async () => {
+      const collectionId = await createCollectionExpectSuccess();
+      await setCollectionLimitsExpectSuccess(alice, collectionId, {ownerCanTransfer: true});
+
+      const tokenId = await createItemExpectSuccess(alice, collectionId, 'NFT', {Substrate: bob.address});
+
+      await transferExpectFailure(collectionId, tokenId, alice, charlie);
+
+      await transferFromExpectSuccess(collectionId, tokenId, alice, bob, charlie, 1);
+    });
+  });
+
+  it('admin burns other user\'s token', async () => {
+    await usingApi(async () => {
+      const collectionId = await createCollectionExpectSuccess();
+      await setCollectionLimitsExpectSuccess(alice, collectionId, {ownerCanTransfer: true});
+
+      const tokenId = await createItemExpectSuccess(alice, collectionId, 'NFT', {Substrate: bob.address});
+
+      await burnItemExpectFailure(alice, collectionId, tokenId);
+
+      await burnFromExpectSuccess(alice, bob, collectionId, tokenId);
+    });
+  });
+});
modifiedtests/src/burnItem.test.tsdiffbeforeafterboth
--- a/tests/src/burnItem.test.ts
+++ b/tests/src/burnItem.test.ts
@@ -155,7 +155,7 @@
     await addCollectionAdminExpectSuccess(alice, collectionId, bob.address);
 
     await usingApi(async (api) => {
-      const tx = api.tx.unique.burnItem(collectionId, tokenId, 1);
+      const tx = api.tx.unique.burnFrom(collectionId, {Substrate: alice.address}, tokenId, 1);
       const events = await submitTransactionAsync(bob, tx);
       const result = getGenericResult(events);
modifiedtests/src/nesting/graphs.test.tsdiffbeforeafterboth
--- a/tests/src/nesting/graphs.test.ts
+++ b/tests/src/nesting/graphs.test.ts
@@ -36,7 +36,7 @@
     await usingApi(async (api, privateKeyWrapper) => {
       const alice = privateKeyWrapper('//Alice');
       const collection = await buildComplexObjectGraph(api, alice);
-      await setCollectionLimitsExpectSuccess(alice, collection, {ownerCanTransfer: true});
+      const tokenTwoParent = tokenIdToCross(collection, 1);
 
       // to self
       await expect(
@@ -49,7 +49,7 @@
         'second transaction',
       ).to.be.rejectedWith(/structure\.OuroborosDetected/);
       await expect(
-        executeTransaction(api, alice, api.tx.unique.transfer(tokenIdToCross(collection, 8), collection, 2, 1)),
+        executeTransaction(api, alice, api.tx.unique.transferFrom(tokenTwoParent, tokenIdToCross(collection, 8), collection, 2, 1)),
         'third transaction',
       ).to.be.rejectedWith(/structure\.OuroborosDetected/);
     });
modifiedtests/src/nesting/nest.test.tsdiffbeforeafterboth
--- a/tests/src/nesting/nest.test.ts
+++ b/tests/src/nesting/nest.test.ts
@@ -123,7 +123,7 @@
       ], 'Children contents check at nesting #2');
 
       // Move token B to a different user outside the nesting tree
-      await transferExpectSuccess(collectionA, tokenB, alice, bob);
+      await transferFromExpectSuccess(collectionA, tokenB, alice, targetAddress, bob);
       children = await getTokenChildren(api, collectionA, targetToken);
       expect(children.length).to.be.equal(1, 'Children length check at unnesting');
       expect(children).to.be.have.deep.members([
@@ -470,7 +470,7 @@
       await expect(executeTransaction(
         api, 
         alice, 
-        api.tx.unique.transfer(targetAddress, collection, newToken, 1),
+        api.tx.unique.transferFrom(targetAddress, {Substrate: bob.address}, collection, newToken, 1),
       ), 'while nesting another\'s token token').to.be.rejectedWith(/common\.AddressNotInAllowlist/);
       expect(await getTokenOwner(api, collection, newToken)).to.be.deep.equal({Substrate: bob.address});
modifiedtests/src/util/helpers.tsdiffbeforeafterboth
--- a/tests/src/util/helpers.ts
+++ b/tests/src/util/helpers.ts
@@ -830,6 +830,25 @@
   });
 }
 
+export async function burnItemExpectFailure(sender: IKeyringPair, collectionId: number, tokenId: number, value: number | bigint = 1) {
+  await usingApi(async (api) => {
+    const tx = api.tx.unique.burnItem(collectionId, tokenId, value);
+
+    const events = await expect(submitTransactionExpectFailAsync(sender, tx)).to.be.rejected;
+    const result = getCreateCollectionResult(events);
+    // tslint:disable-next-line:no-unused-expression
+    expect(result.success).to.be.false;
+  });
+}
+
+export async function burnFromExpectSuccess(sender: IKeyringPair, from: IKeyringPair | CrossAccountId, collectionId: number, tokenId: number, value: number | bigint = 1) {
+  await usingApi(async (api) => {
+    const tx = api.tx.unique.burnFrom(collectionId, normalizeAccountId(from), tokenId, value);
+    const events = await submitTransactionAsync(sender, tx);
+    return getGenericResult(events).success;
+  });
+}
+
 export async function
 approve(
   api: ApiPromise,