git.delta.rocks / unique-network / refs/commits / 061409929b86

difftreelog

Merge pull request #70 from usetech-llc/fix/NFTPAR-290_291

str-mv2021-01-20parents: #f2bcdd2 #0997cff.patch.diff
in: master
Fix/nftpar 290 291

4 files changed

modifiedpallets/nft/src/lib.rsdiffbeforeafterboth
--- a/pallets/nft/src/lib.rs
+++ b/pallets/nft/src/lib.rs
@@ -1009,7 +1009,7 @@
             {
                 CollectionMode::NFT => Self::burn_nft_item(collection_id, item_id)?,
                 CollectionMode::Fungible(_)  => Self::burn_fungible_item(&sender, collection_id, value)?,
-                CollectionMode::ReFungible(_)  => Self::burn_refungible_item(collection_id, item_id, sender.clone())?,
+                CollectionMode::ReFungible(_)  => Self::burn_refungible_item(collection_id, item_id, &sender)?,
                 _ => ()
             };
 
@@ -1092,6 +1092,9 @@
 
             let sender = ensure_signed(origin)?;
 
+            Self::collection_exists(collection_id)?;
+            Self::token_exists(collection_id, item_id, &sender)?;
+
             // Transfer permissions check
             let target_collection = <Collection<T>>::get(collection_id);
             ensure!(Self::is_item_owner(sender.clone(), collection_id, item_id) ||
@@ -1216,7 +1219,8 @@
             let sender = ensure_signed(origin)?;
             
             Self::collection_exists(collection_id)?;
-            
+            Self::token_exists(collection_id, item_id, &sender)?;
+
             ensure!(ChainLimit::get().custom_data_limit >= data.len() as u32, Error::<T>::TokenVariableDataLimitExceeded);
 
             // Modify permissions check
@@ -1224,8 +1228,6 @@
             ensure!(Self::is_item_owner(sender.clone(), collection_id, item_id) ||
                 Self::is_owner_or_admin_permissions(collection_id, sender.clone()),
                 Error::<T>::NoPermission);
-
-            Self::item_exists(collection_id, item_id, &target_collection.mode)?;
 
             match target_collection.mode
             {
@@ -1320,7 +1322,7 @@
             Self::check_owner_or_admin_permissions(collection_id, sender.clone())?;
 
             // check schema limit
-            ensure!(schema.len() as u32 > ChainLimit::get().const_on_chain_schema_limit, "");
+            ensure!(schema.len() as u32 <= ChainLimit::get().const_on_chain_schema_limit, "");
 
             let mut target_collection = <Collection<T>>::get(collection_id);
             target_collection.const_on_chain_schema = schema;
@@ -1351,7 +1353,7 @@
             Self::check_owner_or_admin_permissions(collection_id, sender.clone())?;
 
             // check schema limit
-            ensure!(schema.len() as u32 > ChainLimit::get().variable_on_chain_schema_limit, "");
+            ensure!(schema.len() as u32 <= ChainLimit::get().variable_on_chain_schema_limit, "");
 
             let mut target_collection = <Collection<T>>::get(collection_id);
             target_collection.variable_on_chain_schema = schema;
@@ -1677,13 +1679,13 @@
         let value = item.owner.first().unwrap().fraction;
         let owner = item.owner.first().unwrap().owner.clone();
 
-        Self::add_token_index(collection_id, current_index, owner.clone())?;
+        Self::add_token_index(collection_id, current_index, &owner)?;
 
         <ItemListIndex>::insert(collection_id, current_index);
         <ReFungibleItemList<T>>::insert(collection_id, current_index, itemcopy);
 
         // Update balance
-        let new_balance = <Balance<T>>::get(collection_id, owner.clone())
+        let new_balance = <Balance<T>>::get(collection_id, &owner)
             .checked_add(value)
             .ok_or(Error::<T>::NumOverflow)?;
         <Balance<T>>::insert(collection_id, owner.clone(), new_balance);
@@ -1697,7 +1699,7 @@
             .ok_or(Error::<T>::NumOverflow)?;
 
         let item_owner = item.owner.clone();
-        Self::add_token_index(collection_id, current_index, item.owner.clone())?;
+        Self::add_token_index(collection_id, current_index, &item.owner)?;
 
         <ItemListIndex>::insert(collection_id, current_index);
         <NftItemList<T>>::insert(collection_id, current_index, item);
@@ -1714,29 +1716,44 @@
     fn burn_refungible_item(
         collection_id: CollectionId,
         item_id: TokenId,
-        owner: T::AccountId,
+        owner: &T::AccountId,
     ) -> DispatchResult {
         ensure!(
             <ReFungibleItemList<T>>::contains_key(collection_id, item_id),
             Error::<T>::TokenNotFound
         );
-        let collection = <ReFungibleItemList<T>>::get(collection_id, item_id);
-        let item = collection
+        let mut token = <ReFungibleItemList<T>>::get(collection_id, item_id);
+        let rft_balance = token
             .owner
             .iter()
-            .filter(|&i| i.owner == owner)
+            .filter(|&i| i.owner == *owner)
             .next()
             .unwrap();
-        Self::remove_token_index(collection_id, item_id, owner.clone())?;
+        Self::remove_token_index(collection_id, item_id, owner)?;
 
         // update balance
-        let new_balance = <Balance<T>>::get(collection_id, item.owner.clone())
-            .checked_sub(item.fraction)
+        let new_balance = <Balance<T>>::get(collection_id, rft_balance.owner.clone())
+            .checked_sub(rft_balance.fraction)
             .ok_or(Error::<T>::NumOverflow)?;
-        <Balance<T>>::insert(collection_id, item.owner.clone(), new_balance);
+        <Balance<T>>::insert(collection_id, rft_balance.owner.clone(), new_balance);
 
-        <ReFungibleItemList<T>>::remove(collection_id, item_id);
+        // Re-create owners list with sender removed
+        let index = token
+            .owner
+            .iter()
+            .position(|i| i.owner == *owner)
+            .unwrap();
+        token.owner.remove(index);
+        let owner_count = token.owner.len();
 
+        // Burn the token completely if this was the last (only) owner
+        if owner_count == 0 {
+            <ReFungibleItemList<T>>::remove(collection_id, item_id);
+        }
+        else {
+            <ReFungibleItemList<T>>::insert(collection_id, item_id, token);
+        }
+
         Ok(())
     }
 
@@ -1746,10 +1763,10 @@
             Error::<T>::TokenNotFound
         );
         let item = <NftItemList<T>>::get(collection_id, item_id);
-        Self::remove_token_index(collection_id, item_id, item.owner.clone())?;
+        Self::remove_token_index(collection_id, item_id, &item.owner)?;
 
         // update balance
-        let new_balance = <Balance<T>>::get(collection_id, item.owner.clone())
+        let new_balance = <Balance<T>>::get(collection_id, &item.owner)
             .checked_sub(1)
             .ok_or(Error::<T>::NumOverflow)?;
         <Balance<T>>::insert(collection_id, item.owner.clone(), new_balance);
@@ -1858,16 +1875,33 @@
         Ok(())
     }
 
+    /// Check if token exists. In case of Fungible, check if there is an entry for 
+    /// the owner in fungible balances double map
+    fn token_exists(
+        collection_id: CollectionId,
+        item_id: TokenId,
+        owner: &T::AccountId
+    ) -> DispatchResult {
+        let target_collection = <Collection<T>>::get(collection_id);
+        let exists = match target_collection.mode
+        {
+            CollectionMode::NFT => <NftItemList<T>>::contains_key(collection_id, item_id),
+            CollectionMode::Fungible(_)  => <FungibleItemList<T>>::contains_key(collection_id, owner),
+            CollectionMode::ReFungible(_)  => <ReFungibleItemList<T>>::contains_key(collection_id, item_id),
+            _ => false
+        };
+
+        ensure!(exists == true, Error::<T>::TokenNotFound);
+        Ok(())
+    }
+
     fn transfer_fungible(
         collection_id: CollectionId,
         value: u128,
         owner: &T::AccountId,
         recipient: &T::AccountId,
     ) -> DispatchResult {
-        ensure!(
-            <FungibleItemList<T>>::contains_key(collection_id, owner),
-            Error::<T>::TokenNotFound
-        );
+        Self::token_exists(collection_id, 0, owner)?;
 
         let mut balance = <FungibleItemList<T>>::get(collection_id, owner);
         ensure!(balance.value >= value, Error::<T>::TokenValueTooLow);
@@ -1897,10 +1931,7 @@
         owner: T::AccountId,
         new_owner: T::AccountId,
     ) -> DispatchResult {
-        ensure!(
-            <ReFungibleItemList<T>>::contains_key(collection_id, item_id),
-            Error::<T>::TokenNotFound
-        );
+        Self::token_exists(collection_id, item_id, &owner)?;
 
         let full_item = <ReFungibleItemList<T>>::get(collection_id, item_id);
         let item = full_item
@@ -1941,7 +1972,7 @@
             <ReFungibleItemList<T>>::insert(collection_id, item_id, new_full_item);
 
             // update index collection
-            Self::move_token_index(collection_id, item_id, old_owner.clone(), new_owner.clone())?;
+            Self::move_token_index(collection_id, item_id, &old_owner, &new_owner)?;
         } else {
             let mut new_full_item = full_item.clone();
             new_full_item
@@ -1966,7 +1997,7 @@
                     owner: new_owner.clone(),
                     fraction: value,
                 });
-                Self::add_token_index(collection_id, item_id, new_owner.clone())?;
+                Self::add_token_index(collection_id, item_id, &new_owner)?;
             }
 
             <ReFungibleItemList<T>>::insert(collection_id, item_id, new_full_item);
@@ -1981,10 +2012,7 @@
         sender: T::AccountId,
         new_owner: T::AccountId,
     ) -> DispatchResult {
-        ensure!(
-            <NftItemList<T>>::contains_key(collection_id, item_id),
-            Error::<T>::TokenNotFound
-        );
+        Self::token_exists(collection_id, item_id, &sender)?;
 
         let mut item = <NftItemList<T>>::get(collection_id, item_id);
 
@@ -2010,25 +2038,11 @@
         <NftItemList<T>>::insert(collection_id, item_id, item);
 
         // update index collection
-        Self::move_token_index(collection_id, item_id, old_owner.clone(), new_owner.clone())?;
+        Self::move_token_index(collection_id, item_id, &old_owner, &new_owner)?;
 
         Ok(())
     }
     
-    fn item_exists(
-        collection_id: CollectionId,
-        item_id: TokenId,
-        mode: &CollectionMode
-    ) -> DispatchResult {
-        match mode {
-            CollectionMode::NFT => ensure!(<NftItemList<T>>::contains_key(collection_id, item_id), Error::<T>::TokenNotFound),
-            CollectionMode::ReFungible(_) => ensure!(<ReFungibleItemList<T>>::contains_key(collection_id, item_id), Error::<T>::TokenNotFound),
-            _ => ()
-        };
-        
-        Ok(())
-    }
-
     fn set_re_fungible_variable_data(
         collection_id: CollectionId,
         item_id: TokenId,
@@ -2090,12 +2104,12 @@
             .unwrap();
 
         let item_owner = item.owner.clone();
-        Self::add_token_index(collection_id, current_index, item.owner.clone()).unwrap();
+        Self::add_token_index(collection_id, current_index, &item.owner).unwrap();
 
         <ItemListIndex>::insert(collection_id, current_index);
 
         // Update balance
-        let new_balance = <Balance<T>>::get(collection_id, item_owner.clone())
+        let new_balance = <Balance<T>>::get(collection_id, &item_owner)
             .checked_add(1)
             .unwrap();
         <Balance<T>>::insert(collection_id, item_owner.clone(), new_balance);
@@ -2106,7 +2120,7 @@
             .checked_add(1)
             .unwrap();
 
-        Self::add_token_index(collection_id, current_index, (*owner).clone()).unwrap();
+        Self::add_token_index(collection_id, current_index, owner).unwrap();
 
         <ItemListIndex>::insert(collection_id, current_index);
 
@@ -2125,24 +2139,24 @@
         let value = item.owner.first().unwrap().fraction;
         let owner = item.owner.first().unwrap().owner.clone();
 
-        Self::add_token_index(collection_id, current_index, owner.clone()).unwrap();
+        Self::add_token_index(collection_id, current_index, &owner).unwrap();
 
         <ItemListIndex>::insert(collection_id, current_index);
 
         // Update balance
-        let new_balance = <Balance<T>>::get(collection_id, owner.clone())
+        let new_balance = <Balance<T>>::get(collection_id, &owner)
             .checked_add(value)
             .unwrap();
         <Balance<T>>::insert(collection_id, owner.clone(), new_balance);
     }
 
-    fn add_token_index(collection_id: CollectionId, item_index: TokenId, owner: T::AccountId) -> DispatchResult {
+    fn add_token_index(collection_id: CollectionId, item_index: TokenId, owner: &T::AccountId) -> DispatchResult {
 
         // add to account limit
-        if <AccountItemCount<T>>::contains_key(owner.clone()) {
+        if <AccountItemCount<T>>::contains_key(owner) {
 
             // bound Owned tokens by a single address
-            let count = <AccountItemCount<T>>::get(owner.clone());
+            let count = <AccountItemCount<T>>::get(owner);
             ensure!(count < ChainLimit::get().account_token_ownership_limit, Error::<T>::AddressOwnershipLimitExceeded);
 
             <AccountItemCount<T>>::insert(owner.clone(), count
@@ -2153,9 +2167,9 @@
             <AccountItemCount<T>>::insert(owner.clone(), 1);
         }
 
-        let list_exists = <AddressTokens<T>>::contains_key(collection_id, owner.clone());
+        let list_exists = <AddressTokens<T>>::contains_key(collection_id, owner);
         if list_exists {
-            let mut list = <AddressTokens<T>>::get(collection_id, owner.clone());
+            let mut list = <AddressTokens<T>>::get(collection_id, owner);
             let item_contains = list.contains(&item_index.clone());
 
             if !item_contains {
@@ -2166,8 +2180,7 @@
         } else {
             let mut itm = Vec::new();
             itm.push(item_index.clone());
-            <AddressTokens<T>>::insert(collection_id, owner, itm);
-            
+            <AddressTokens<T>>::insert(collection_id, owner.clone(), itm);
         }
 
         Ok(())
@@ -2176,24 +2189,24 @@
     fn remove_token_index(
         collection_id: CollectionId,
         item_index: TokenId,
-        owner: T::AccountId,
+        owner: &T::AccountId,
     ) -> DispatchResult {
 
         // update counter
         <AccountItemCount<T>>::insert(owner.clone(), 
-            <AccountItemCount<T>>::get(owner.clone())
+            <AccountItemCount<T>>::get(owner)
             .checked_sub(1)
             .ok_or(Error::<T>::NumOverflow)?);
 
 
-        let list_exists = <AddressTokens<T>>::contains_key(collection_id, owner.clone());
+        let list_exists = <AddressTokens<T>>::contains_key(collection_id, owner);
         if list_exists {
-            let mut list = <AddressTokens<T>>::get(collection_id, owner.clone());
+            let mut list = <AddressTokens<T>>::get(collection_id, owner);
             let item_contains = list.contains(&item_index.clone());
 
             if item_contains {
                 list.retain(|&item| item != item_index);
-                <AddressTokens<T>>::insert(collection_id, owner, list);
+                <AddressTokens<T>>::insert(collection_id, owner.clone(), list);
             }
         }
 
@@ -2203,8 +2216,8 @@
     fn move_token_index(
         collection_id: CollectionId,
         item_index: TokenId,
-        old_owner: T::AccountId,
-        new_owner: T::AccountId,
+        old_owner: &T::AccountId,
+        new_owner: &T::AccountId,
     ) -> DispatchResult {
         Self::remove_token_index(collection_id, item_index, old_owner)?;
         Self::add_token_index(collection_id, item_index, new_owner)?;
modifiedpallets/nft/src/tests.rsdiffbeforeafterboth
20 nft_sponsor_transfer_timeout: 15,20 nft_sponsor_transfer_timeout: 15,
21 fungible_sponsor_transfer_timeout: 15,21 fungible_sponsor_transfer_timeout: 15,
22 refungible_sponsor_transfer_timeout: 15, 22 refungible_sponsor_transfer_timeout: 15,
23 const_on_chain_schema_limit: 1024,
24 offchain_schema_limit: 1024,
25 variable_on_chain_schema_limit: 1024,
23 }));26 }));
24}27}
2528
1255 });1258 });
1256}1259}
12571260
1258// If Public Access mode is set to WhiteList, oken transfers can’t be Approved by a non-whitelisted address (see Approve method).1261// If Public Access mode is set to WhiteList, token transfers can’t be Approved by a non-whitelisted address (see Approve method).
1259#[test]1262#[test]
1260fn white_list_test_6() {1263fn white_list_test_6() {
1261 new_test_ext().execute_with(|| {1264 new_test_ext().execute_with(|| {
12651268
1266 let origin1 = Origin::signed(1);1269 let origin1 = Origin::signed(1);
1270
1271 let data = default_nft_data();
1272 create_test_item(collection_id, &data.into());
1273
1267 assert_ok!(TemplateModule::set_public_access_mode(1274 assert_ok!(TemplateModule::set_public_access_mode(
1268 origin1.clone(),1275 origin1.clone(),
1639 nft_sponsor_transfer_timeout: 15,1646 nft_sponsor_transfer_timeout: 15,
1640 fungible_sponsor_transfer_timeout: 15,1647 fungible_sponsor_transfer_timeout: 15,
1641 refungible_sponsor_transfer_timeout: 15, 1648 refungible_sponsor_transfer_timeout: 15,
1649 const_on_chain_schema_limit: 1024,
1650 offchain_schema_limit: 1024,
1651 variable_on_chain_schema_limit: 1024,
1642 }));1652 }));
1643 1653
1644 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1654 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1668 nft_sponsor_transfer_timeout: 15,1678 nft_sponsor_transfer_timeout: 15,
1669 fungible_sponsor_transfer_timeout: 15,1679 fungible_sponsor_transfer_timeout: 15,
1670 refungible_sponsor_transfer_timeout: 15, 1680 refungible_sponsor_transfer_timeout: 15,
1681 const_on_chain_schema_limit: 1024,
1682 offchain_schema_limit: 1024,
1683 variable_on_chain_schema_limit: 1024,
1671 }));1684 }));
1672 1685
1673 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1686 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1691 nft_sponsor_transfer_timeout: 15,1704 nft_sponsor_transfer_timeout: 15,
1692 fungible_sponsor_transfer_timeout: 15,1705 fungible_sponsor_transfer_timeout: 15,
1693 refungible_sponsor_transfer_timeout: 15, 1706 refungible_sponsor_transfer_timeout: 15,
1707 const_on_chain_schema_limit: 1024,
1708 offchain_schema_limit: 1024,
1709 variable_on_chain_schema_limit: 1024,
1694 }));1710 }));
1695 1711
1696 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1712 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1714 nft_sponsor_transfer_timeout: 15,1730 nft_sponsor_transfer_timeout: 15,
1715 fungible_sponsor_transfer_timeout: 15,1731 fungible_sponsor_transfer_timeout: 15,
1716 refungible_sponsor_transfer_timeout: 15, 1732 refungible_sponsor_transfer_timeout: 15,
1733 const_on_chain_schema_limit: 1024,
1734 offchain_schema_limit: 1024,
1735 variable_on_chain_schema_limit: 1024,
1717 }));1736 }));
1718 1737
1719 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1738 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1745 nft_sponsor_transfer_timeout: 15,1764 nft_sponsor_transfer_timeout: 15,
1746 fungible_sponsor_transfer_timeout: 15,1765 fungible_sponsor_transfer_timeout: 15,
1747 refungible_sponsor_transfer_timeout: 15, 1766 refungible_sponsor_transfer_timeout: 15,
1767 const_on_chain_schema_limit: 1024,
1768 offchain_schema_limit: 1024,
1769 variable_on_chain_schema_limit: 1024,
1748 }));1770 }));
17491771
1750 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1772 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1776 nft_sponsor_transfer_timeout: 15,1798 nft_sponsor_transfer_timeout: 15,
1777 fungible_sponsor_transfer_timeout: 15,1799 fungible_sponsor_transfer_timeout: 15,
1778 refungible_sponsor_transfer_timeout: 15, 1800 refungible_sponsor_transfer_timeout: 15,
1801 const_on_chain_schema_limit: 1024,
1802 offchain_schema_limit: 1024,
1803 variable_on_chain_schema_limit: 1024,
1779 }));1804 }));
17801805
1781 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1806 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1807 nft_sponsor_transfer_timeout: 15,1832 nft_sponsor_transfer_timeout: 15,
1808 fungible_sponsor_transfer_timeout: 15,1833 fungible_sponsor_transfer_timeout: 15,
1809 refungible_sponsor_transfer_timeout: 15, 1834 refungible_sponsor_transfer_timeout: 15,
1835 const_on_chain_schema_limit: 1024,
1836 offchain_schema_limit: 1024,
1837 variable_on_chain_schema_limit: 1024,
1810 }));1838 }));
18111839
1812 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1840 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1924 nft_sponsor_transfer_timeout: 15,1952 nft_sponsor_transfer_timeout: 15,
1925 fungible_sponsor_transfer_timeout: 15,1953 fungible_sponsor_transfer_timeout: 15,
1926 refungible_sponsor_transfer_timeout: 15, 1954 refungible_sponsor_transfer_timeout: 15,
1955 const_on_chain_schema_limit: 1024,
1956 offchain_schema_limit: 1024,
1957 variable_on_chain_schema_limit: 1024,
1927 }));1958 }));
19281959
1929 let collection_id = create_test_collection(&CollectionMode::NFT, 1);1960 let collection_id = create_test_collection(&CollectionMode::NFT, 1);
1949 nft_sponsor_transfer_timeout: 15,1980 nft_sponsor_transfer_timeout: 15,
1950 fungible_sponsor_transfer_timeout: 15,1981 fungible_sponsor_transfer_timeout: 15,
1951 refungible_sponsor_transfer_timeout: 15, 1982 refungible_sponsor_transfer_timeout: 15,
1983 const_on_chain_schema_limit: 1024,
1984 offchain_schema_limit: 1024,
1985 variable_on_chain_schema_limit: 1024,
1952 }));1986 }));
19531987
19541988
modifiedtests/package.jsondiffbeforeafterboth
--- a/tests/package.json
+++ b/tests/package.json
@@ -29,7 +29,8 @@
     "testCreateCollection": "mocha --timeout 9999999 -r ts-node/register ./**/createCollection.test.ts",
     "testToggleContractWhiteList": "mocha --timeout 9999999 -r ts-node/register ./**/toggleContractWhiteList.test.ts",
     "testAddToContractWhiteList": "mocha --timeout 9999999 -r ts-node/register ./**/addToContractWhiteList.test.ts",
-    "testTransfer": "mocha --timeout 9999999 -r ts-node/register ./**/transfer.test.ts"
+    "testTransfer": "mocha --timeout 9999999 -r ts-node/register ./**/transfer.test.ts",
+    "testBurnItem": "mocha --timeout 9999999 -r ts-node/register ./**/burnItem.test.ts"
   },
   "author": "",
   "license": "Apache 2.0",
modifiedtests/src/addToWhiteList.test.tsdiffbeforeafterboth
--- a/tests/src/addToWhiteList.test.ts
+++ b/tests/src/addToWhiteList.test.ts
@@ -18,7 +18,7 @@
 let Alice: IKeyringPair;
 let Bob: IKeyringPair;
 
-describe.only('Integration Test ext. addToWhiteList()', () => {
+describe('Integration Test ext. addToWhiteList()', () => {
 
   before(async () => {
     await usingApi(async (api) => {
@@ -41,7 +41,7 @@
   });
 });
 
-describe.only('Negative Integration Test ext. addToWhiteList()', () => {
+describe('Negative Integration Test ext. addToWhiteList()', () => {
 
   it('White list an address in the collection that does not exist', async () => {
     await usingApi(async (api) => {