git.delta.rocks / unique-network / refs/commits / 4bce10c77fc9

difftreelog

Fix collection limits

Greg Zaitsev2021-02-26parent: #4b68421.patch.diff
in: master

3 files changed

modifiedpallets/nft/src/lib.rsdiffbeforeafterboth
--- 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 = <Collection<T>>::get(collection_id);
+            let target_collection = <Collection<T>>::get(collection_id);
             if !target_collection.limits.owner_can_destroy {
                 fail!(Error::<T>::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 = <Collection<T>>::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::<T>::CollectionLimitBoundsExceeded);
 
             // token_limit   check  prev
-            ensure!(climits.token_limit > limits.token_limit && 
-                limits.token_limit <= chain_limits.account_token_ownership_limit, 
-                Error::<T>::AccountTokenLimitExceeded);
+            ensure!(old_limits.token_limit >= new_limits.token_limit, Error::<T>::CollectionTokenLimitExceeded);
+            ensure!(new_limits.token_limit > 0, Error::<T>::CollectionTokenLimitExceeded);
 
             ensure!(
-                (climits.owner_can_transfer || !limits.owner_can_transfer) &&
-                (climits.owner_can_destroy || !limits.owner_can_destroy),
-                Error::<T>::OwnerPermissionsCantBeReturned,
+                (old_limits.owner_can_transfer || !new_limits.owner_can_transfer) &&
+                (old_limits.owner_can_destroy || !new_limits.owner_can_destroy),
+                Error::<T>::OwnerPermissionsCantBeReverted,
             );
 
-            target_collection.limits = limits;
+            target_collection.limits = new_limits;
             <Collection<T>>::insert(collection_id, target_collection);
 
             Ok(())
modifiedtests/src/setCollectionLimits.test.tsdiffbeforeafterboth
--- 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
+    });
   });
 });
modifiedtests/src/types.tsdiffbeforeafterboth
17 SponsoredMintSize: BN;17 SponsoredMintSize: BN;
18 TokenLimit: BN;18 TokenLimit: BN;
19 SponsorTimeout: BN;19 SponsorTimeout: BN;
20 OwnerCanTransfer: boolean;
21 OwnerCanDestroy: boolean;
20 };22 };
21 MintMode: boolean;23 MintMode: boolean;
22 Mode: {24 Mode: {