Improve NFT locking (#14510)

* Update docs

* Prevent locking of the same NFT twice

* Validate item is not locked on burn

* Cover with tests

* chore

frame/nft-fractionalization/src/lib.rs

@@ -338,12 +338,12 @@ pub mod pallet {
 			T::PalletId::get().into_account_truncating()
 		}
 
-		/// Transfer the NFT from the account holding that NFT to the pallet's account.
+		/// Prevent further transferring of NFT.
 		fn do_lock_nft(nft_collection_id: T::NftCollectionId, nft_id: T::NftId) -> DispatchResult {
 			T::Nfts::disable_transfer(&nft_collection_id, &nft_id)
 		}
 
-		/// Transfer the NFT to the account returning the tokens.
+		/// Remove the transfer lock and transfer the NFT to the account returning the tokens.
 		fn do_unlock_nft(
 			nft_collection_id: T::NftCollectionId,
 			nft_id: T::NftId,

frame/nft-fractionalization/src/tests.rs

@@ -122,6 +122,33 @@ fn fractionalize_should_work() {
 			beneficiary: account(2),
 		}));
 
+		// owner can't burn an already fractionalized NFT
+		assert_noop!(
+			Nfts::burn(RuntimeOrigin::signed(account(1)), nft_collection_id, nft_id),
+			DispatchError::Module(ModuleError {
+				index: 4,
+				error: [12, 0, 0, 0],
+				message: Some("ItemLocked")
+			})
+		);
+
+		// can't fractionalize twice
+		assert_noop!(
+			NftFractionalization::fractionalize(
+				RuntimeOrigin::signed(account(1)),
+				nft_collection_id,
+				nft_id,
+				asset_id + 1,
+				account(2),
+				fractions,
+			),
+			DispatchError::Module(ModuleError {
+				index: 4,
+				error: [12, 0, 0, 0],
+				message: Some("ItemLocked")
+			})
+		);
+
 		let nft_id = nft_id + 1;
 		assert_noop!(
 			NftFractionalization::fractionalize(

frame/nfts/src/features/create_delete_item.rs

@@ -169,6 +169,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		with_details: impl FnOnce(&ItemDetailsFor<T, I>) -> DispatchResult,
 	) -> DispatchResult {
 		ensure!(!T::Locker::is_locked(collection, item), Error::<T, I>::ItemLocked);
+		ensure!(
+			!Self::has_system_attribute(&collection, &item, PalletAttributes::TransferDisabled)?,
+			Error::<T, I>::ItemLocked
+		);
 		let item_config = Self::get_item_config(&collection, &item)?;
 		// NOTE: if item's settings are not empty (e.g. item's metadata is locked)
 		// then we keep the config record and don't remove it

frame/nfts/src/impl_nonfungibles.rs

@@ -341,6 +341,13 @@ impl<T: Config<I>, I: 'static> Transfer<T::AccountId> for Pallet<T, I> {
 	}
 
 	fn disable_transfer(collection: &Self::CollectionId, item: &Self::ItemId) -> DispatchResult {
+		let transfer_disabled =
+			Self::has_system_attribute(&collection, &item, PalletAttributes::TransferDisabled)?;
+		// Can't lock the item twice
+		if transfer_disabled {
+			return Err(Error::<T, I>::ItemLocked.into())
+		}
+
 		<Self as Mutate<T::AccountId, ItemConfig>>::set_attribute(
 			collection,
 			item,