From 4bce10c77fc9c3692543911f813ee5f75cca5bfe Mon Sep 17 00:00:00 2001 From: Greg Zaitsev Date: Fri, 26 Feb 2021 12:46:42 +0000 Subject: [PATCH] Fix collection limits --- --- a/pallets/nft/src/lib.rs +++ b/pallets/nft/src/lib.rs @@ -383,7 +383,7 @@ /// Collection limit bounds per collection exceeded CollectionLimitBoundsExceeded, /// Tried to enable permissions which are only permitted to be disabled - OwnerPermissionsCantBeReturned, + OwnerPermissionsCantBeReverted, /// Schema data size limit bound exceeded SchemaDataLimitExceeded, /// Maximum refungibility exceeded @@ -626,7 +626,7 @@ let sender = ensure_signed(origin)?; Self::check_owner_permissions(collection_id, sender)?; - let mut target_collection = >::get(collection_id); + let target_collection = >::get(collection_id); if !target_collection.limits.owner_can_destroy { fail!(Error::::NoPermission); } @@ -1532,31 +1532,32 @@ pub fn set_collection_limits( origin, collection_id: u32, - limits: CollectionLimits, + new_limits: CollectionLimits, ) -> DispatchResult { let sender = ensure_signed(origin)?; Self::check_owner_permissions(collection_id, sender.clone())?; let mut target_collection = >::get(collection_id); + let old_limits = target_collection.limits; let chain_limits = ChainLimit::get(); - let climits = target_collection.limits; // collection bounds - ensure!(limits.sponsor_transfer_timeout <= MAX_SPONSOR_TIMEOUT && - limits.account_token_ownership_limit <= MAX_TOKEN_OWNERSHIP, + ensure!(new_limits.sponsor_transfer_timeout <= MAX_SPONSOR_TIMEOUT && + new_limits.account_token_ownership_limit <= MAX_TOKEN_OWNERSHIP && + new_limits.sponsored_data_size <= chain_limits.custom_data_limit && + new_limits.sponsored_mint_size <= chain_limits.custom_data_limit, Error::::CollectionLimitBoundsExceeded); // token_limit check prev - ensure!(climits.token_limit > limits.token_limit && - limits.token_limit <= chain_limits.account_token_ownership_limit, - Error::::AccountTokenLimitExceeded); + ensure!(old_limits.token_limit >= new_limits.token_limit, Error::::CollectionTokenLimitExceeded); + ensure!(new_limits.token_limit > 0, Error::::CollectionTokenLimitExceeded); ensure!( - (climits.owner_can_transfer || !limits.owner_can_transfer) && - (climits.owner_can_destroy || !limits.owner_can_destroy), - Error::::OwnerPermissionsCantBeReturned, + (old_limits.owner_can_transfer || !new_limits.owner_can_transfer) && + (old_limits.owner_can_destroy || !new_limits.owner_can_destroy), + Error::::OwnerPermissionsCantBeReverted, ); - target_collection.limits = limits; + target_collection.limits = new_limits; >::insert(collection_id, target_collection); Ok(()) --- a/tests/src/setCollectionLimits.test.ts +++ b/tests/src/setCollectionLimits.test.ts @@ -28,21 +28,8 @@ const accountTokenOwnershipLimit = 0; const sponsoredDataSize = 0; const sponsoredMintSize = 0; -const tokenLimit = 0; - -describe('hooks', () => { - before(async () => { - await usingApi(async () => { - const keyring = new Keyring({ type: 'sr25519' }); - alice = keyring.addFromUri('//Alice'); - }); - }); - it('choose or create collection for testing', async () => { - await usingApi(async () => { - collectionIdForTesting = await createCollectionExpectSuccess({name: 'A', description: 'B', tokenPrefix: 'C', mode: {type: 'NFT'}}); - }); - }); -}); +const sponsorTimeout = 1; +const tokenLimit = 1; describe('setCollectionLimits positive', () => { let tx; @@ -50,6 +37,7 @@ await usingApi(async () => { const keyring = new Keyring({ type: 'sr25519' }); alice = keyring.addFromUri('//Alice'); + collectionIdForTesting = await createCollectionExpectSuccess({name: 'A', description: 'B', tokenPrefix: 'C', mode: {type: 'NFT'}}); }); }); it('execute setCollectionLimits with predefined params ', async () => { @@ -57,25 +45,28 @@ tx = api.tx.nft.setCollectionLimits( collectionIdForTesting, { - accountTokenOwnershipLimit, - sponsoredDataSize, - sponsoredMintSize, - tokenLimit, + AccountTokenOwnershipLimit: accountTokenOwnershipLimit, + SponsoredMintSize: sponsoredDataSize, + TokenLimit: tokenLimit, + SponsorTimeout: sponsorTimeout, + OwnerCanTransfer: true, + OwnerCanDestroy: true }, ); const events = await submitTransactionAsync(alice, tx); const result = getCreateItemResult(events); + + // get collection limits defined previously + const collectionInfo = await getDetailedCollectionInfo(api, collectionIdForTesting) as ICollectionInterface; + // tslint:disable-next-line:no-unused-expression expect(result.success).to.be.true; - }); - }); - it('get collection limits defined in previous test', async () => { - await usingApi(async (api: ApiPromise) => { - const collectionInfo = await getDetailedCollectionInfo(api, collectionIdForTesting) as ICollectionInterface; expect(collectionInfo.Limits.AccountTokenOwnershipLimit.toNumber()).to.be.equal(accountTokenOwnershipLimit); - expect(collectionInfo.Limits.SponsoredMintSize.toNumber()).to.be.equal(sponsoredMintSize); + expect(collectionInfo.Limits.SponsoredMintSize.toNumber()).to.be.equal(sponsoredDataSize); expect(collectionInfo.Limits.TokenLimit.toNumber()).to.be.equal(tokenLimit); - expect(collectionInfo.Limits.SponsorTimeout.toNumber()).to.be.equal(sponsoredDataSize); + expect(collectionInfo.Limits.SponsorTimeout.toNumber()).to.be.equal(sponsorTimeout); + expect(collectionInfo.Limits.OwnerCanTransfer.valueOf()).to.be.true; + expect(collectionInfo.Limits.OwnerCanDestroy.valueOf()).to.be.true; }); }); }); @@ -87,6 +78,7 @@ const keyring = new Keyring({ type: 'sr25519' }); alice = keyring.addFromUri('//Alice'); bob = keyring.addFromUri('//Bob'); + collectionIdForTesting = await createCollectionExpectSuccess({name: 'A', description: 'B', tokenPrefix: 'C', mode: {type: 'NFT'}}); }); }); it('execute setCollectionLimits for not exists collection', async () => { @@ -136,13 +128,41 @@ it('fails when trying to enable OwnerCanTransfer after it was disabled', async () => { const collectionId = await createCollectionExpectSuccess(); - await setCollectionLimitsExpectSuccess(alice, collectionId, { OwnerCanTransfer: false }); - await setCollectionLimitsExpectFailure(alice, collectionId, { OwnerCanTransfer: true }); + await setCollectionLimitsExpectSuccess(alice, collectionId, { + AccountTokenOwnershipLimit: accountTokenOwnershipLimit, + SponsoredMintSize: sponsoredDataSize, + TokenLimit: tokenLimit, + SponsorTimeout: sponsorTimeout, + OwnerCanTransfer: false, + OwnerCanDestroy: true + }); + await setCollectionLimitsExpectFailure(alice, collectionId, { + AccountTokenOwnershipLimit: accountTokenOwnershipLimit, + SponsoredMintSize: sponsoredDataSize, + TokenLimit: tokenLimit, + SponsorTimeout: sponsorTimeout, + OwnerCanTransfer: true, + OwnerCanDestroy: true + }); }); it('fails when trying to enable OwnerCanDestroy after it was disabled', async () => { const collectionId = await createCollectionExpectSuccess(); - await setCollectionLimitsExpectSuccess(alice, collectionId, { OwnerCanDestroy: false }); - await setCollectionLimitsExpectFailure(alice, collectionId, { OwnerCanDestroy: true }); + await setCollectionLimitsExpectSuccess(alice, collectionId, { + AccountTokenOwnershipLimit: accountTokenOwnershipLimit, + SponsoredMintSize: sponsoredDataSize, + TokenLimit: tokenLimit, + SponsorTimeout: sponsorTimeout, + OwnerCanTransfer: true, + OwnerCanDestroy: false + }); + await setCollectionLimitsExpectFailure(alice, collectionId, { + AccountTokenOwnershipLimit: accountTokenOwnershipLimit, + SponsoredMintSize: sponsoredDataSize, + TokenLimit: tokenLimit, + SponsorTimeout: sponsorTimeout, + OwnerCanTransfer: true, + OwnerCanDestroy: true + }); }); }); --- a/tests/src/types.ts +++ b/tests/src/types.ts @@ -17,6 +17,8 @@ SponsoredMintSize: BN; TokenLimit: BN; SponsorTimeout: BN; + OwnerCanTransfer: boolean; + OwnerCanDestroy: boolean; }; MintMode: boolean; Mode: { -- gitstuff