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

--- a/pallets/nonfungible/src/lib.rs
+++ b/pallets/nonfungible/src/lib.rs
@@ -379,11 +379,7 @@
 	) -> DispatchResult {
 		let token_data =
 			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;
-		ensure!(
-			&token_data.owner == sender
-				|| (collection.limits.owner_can_transfer() && collection.is_owner_or_admin(sender)),
-			<CommonError<T>>::NoPermission
-		);
+		ensure!(&token_data.owner == sender, <CommonError<T>>::NoPermission);
 
 		if collection.permissions.access() == AccessMode::AllowList {
 			collection.check_allowlist(sender)?;
@@ -665,12 +661,7 @@
 
 		let token_data =
 			<TokenData<T>>::get((collection.id, token)).ok_or(<CommonError<T>>::TokenNotFound)?;
-		// TODO: require sender to be token, owner, require admins to go through transfer_from
-		ensure!(
-			&token_data.owner == from
-				|| (collection.limits.owner_can_transfer() && collection.is_owner_or_admin(from)),
-			<CommonError<T>>::NoPermission
-		);
+		ensure!(&token_data.owner == from, <CommonError<T>>::NoPermission);
 
 		if collection.permissions.access() == AccessMode::AllowList {
 			collection.check_allowlist(from)?;
@@ -976,8 +967,12 @@
 			// `from`, `to` checked in [`transfer`]
 			collection.check_allowlist(spender)?;
 		}
+
+		if collection.limits.owner_can_transfer() && collection.is_owner_or_admin(spender) {
+			return Ok(());
+		}
+
 		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(),

--- 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",

--- 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);
 

--- 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/);
     });

--- 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});
 

--- 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,