--- 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 = >::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::::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::::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 = >::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 = >::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)?; ::insert(collection_id, current_index); >::insert(collection_id, current_index, itemcopy); // Update balance - let new_balance = >::get(collection_id, owner.clone()) + let new_balance = >::get(collection_id, &owner) .checked_add(value) .ok_or(Error::::NumOverflow)?; >::insert(collection_id, owner.clone(), new_balance); @@ -1697,7 +1699,7 @@ .ok_or(Error::::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)?; ::insert(collection_id, current_index); >::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!( >::contains_key(collection_id, item_id), Error::::TokenNotFound ); - let collection = >::get(collection_id, item_id); - let item = collection + let mut token = >::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 = >::get(collection_id, item.owner.clone()) - .checked_sub(item.fraction) + let new_balance = >::get(collection_id, rft_balance.owner.clone()) + .checked_sub(rft_balance.fraction) .ok_or(Error::::NumOverflow)?; - >::insert(collection_id, item.owner.clone(), new_balance); + >::insert(collection_id, rft_balance.owner.clone(), new_balance); - >::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 { + >::remove(collection_id, item_id); + } + else { + >::insert(collection_id, item_id, token); + } + Ok(()) } @@ -1746,10 +1763,10 @@ Error::::TokenNotFound ); let item = >::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 = >::get(collection_id, item.owner.clone()) + let new_balance = >::get(collection_id, &item.owner) .checked_sub(1) .ok_or(Error::::NumOverflow)?; >::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 = >::get(collection_id); + let exists = match target_collection.mode + { + CollectionMode::NFT => >::contains_key(collection_id, item_id), + CollectionMode::Fungible(_) => >::contains_key(collection_id, owner), + CollectionMode::ReFungible(_) => >::contains_key(collection_id, item_id), + _ => false + }; + + ensure!(exists == true, Error::::TokenNotFound); + Ok(()) + } + fn transfer_fungible( collection_id: CollectionId, value: u128, owner: &T::AccountId, recipient: &T::AccountId, ) -> DispatchResult { - ensure!( - >::contains_key(collection_id, owner), - Error::::TokenNotFound - ); + Self::token_exists(collection_id, 0, owner)?; let mut balance = >::get(collection_id, owner); ensure!(balance.value >= value, Error::::TokenValueTooLow); @@ -1897,10 +1931,7 @@ owner: T::AccountId, new_owner: T::AccountId, ) -> DispatchResult { - ensure!( - >::contains_key(collection_id, item_id), - Error::::TokenNotFound - ); + Self::token_exists(collection_id, item_id, &owner)?; let full_item = >::get(collection_id, item_id); let item = full_item @@ -1941,7 +1972,7 @@ >::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)?; } >::insert(collection_id, item_id, new_full_item); @@ -1981,10 +2012,7 @@ sender: T::AccountId, new_owner: T::AccountId, ) -> DispatchResult { - ensure!( - >::contains_key(collection_id, item_id), - Error::::TokenNotFound - ); + Self::token_exists(collection_id, item_id, &sender)?; let mut item = >::get(collection_id, item_id); @@ -2010,25 +2038,11 @@ >::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!(>::contains_key(collection_id, item_id), Error::::TokenNotFound), - CollectionMode::ReFungible(_) => ensure!(>::contains_key(collection_id, item_id), Error::::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(); ::insert(collection_id, current_index); // Update balance - let new_balance = >::get(collection_id, item_owner.clone()) + let new_balance = >::get(collection_id, &item_owner) .checked_add(1) .unwrap(); >::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(); ::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(); ::insert(collection_id, current_index); // Update balance - let new_balance = >::get(collection_id, owner.clone()) + let new_balance = >::get(collection_id, &owner) .checked_add(value) .unwrap(); >::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 >::contains_key(owner.clone()) { + if >::contains_key(owner) { // bound Owned tokens by a single address - let count = >::get(owner.clone()); + let count = >::get(owner); ensure!(count < ChainLimit::get().account_token_ownership_limit, Error::::AddressOwnershipLimitExceeded); >::insert(owner.clone(), count @@ -2153,9 +2167,9 @@ >::insert(owner.clone(), 1); } - let list_exists = >::contains_key(collection_id, owner.clone()); + let list_exists = >::contains_key(collection_id, owner); if list_exists { - let mut list = >::get(collection_id, owner.clone()); + let mut list = >::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()); - >::insert(collection_id, owner, itm); - + >::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 >::insert(owner.clone(), - >::get(owner.clone()) + >::get(owner) .checked_sub(1) .ok_or(Error::::NumOverflow)?); - let list_exists = >::contains_key(collection_id, owner.clone()); + let list_exists = >::contains_key(collection_id, owner); if list_exists { - let mut list = >::get(collection_id, owner.clone()); + let mut list = >::get(collection_id, owner); let item_contains = list.contains(&item_index.clone()); if item_contains { list.retain(|&item| item != item_index); - >::insert(collection_id, owner, list); + >::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)?; --- a/pallets/nft/src/tests.rs +++ b/pallets/nft/src/tests.rs @@ -12,14 +12,17 @@ } fn default_limits() { - assert_ok!(TemplateModule::set_chain_limits(RawOrigin::Root.into(), ChainLimits { + assert_ok!(TemplateModule::set_chain_limits(RawOrigin::Root.into(), ChainLimits { collection_numbers_limit: default_collection_numbers_limit(), account_token_ownership_limit: 10, collections_admins_limit: 5, custom_data_limit: 2048, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); } @@ -1255,7 +1258,7 @@ }); } -// If Public Access mode is set to WhiteList, oken transfers can’t be Approved by a non-whitelisted address (see Approve method). +// If Public Access mode is set to WhiteList, token transfers can’t be Approved by a non-whitelisted address (see Approve method). #[test] fn white_list_test_6() { new_test_ext().execute_with(|| { @@ -1264,6 +1267,10 @@ let collection_id = create_test_collection(&CollectionMode::NFT, 1); let origin1 = Origin::signed(1); + + let data = default_nft_data(); + create_test_item(collection_id, &data.into()); + assert_ok!(TemplateModule::set_public_access_mode( origin1.clone(), collection_id, @@ -1638,7 +1645,10 @@ custom_data_limit: 2048, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1667,7 +1677,10 @@ custom_data_limit: 2048, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1690,7 +1703,10 @@ custom_data_limit: 2048, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1713,7 +1729,10 @@ custom_data_limit: 2, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1744,7 +1763,10 @@ custom_data_limit: 2, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1775,7 +1797,10 @@ custom_data_limit: 2, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1806,7 +1831,10 @@ custom_data_limit: 2, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1923,7 +1951,10 @@ custom_data_limit: 10, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); let collection_id = create_test_collection(&CollectionMode::NFT, 1); @@ -1948,7 +1979,10 @@ custom_data_limit: 10, nft_sponsor_transfer_timeout: 15, fungible_sponsor_transfer_timeout: 15, - refungible_sponsor_transfer_timeout: 15, + refungible_sponsor_transfer_timeout: 15, + const_on_chain_schema_limit: 1024, + offchain_schema_limit: 1024, + variable_on_chain_schema_limit: 1024, })); --- 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", --- 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) => {