difftreelog
Merge pull request #410 from UniqueNetwork/fix/admin-transfer
in: master
Fix/admin transfer
7 files changed
pallets/nonfungible/src/lib.rsdiffbeforeafterboth--- 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(),
tests/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",
tests/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);
+ });
+ });
+});
tests/src/burnItem.test.tsdiffbeforeafterboth155 await addCollectionAdminExpectSuccess(alice, collectionId, bob.address);155 await addCollectionAdminExpectSuccess(alice, collectionId, bob.address);156156157 await usingApi(async (api) => {157 await usingApi(async (api) => {158 const tx = api.tx.unique.burnItem(collectionId, tokenId, 1);158 const tx = api.tx.unique.burnFrom(collectionId, {Substrate: alice.address}, tokenId, 1);159 const events = await submitTransactionAsync(bob, tx);159 const events = await submitTransactionAsync(bob, tx);160 const result = getGenericResult(events);160 const result = getGenericResult(events);161161tests/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/);
});
tests/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});
tests/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,