From f5776f689768d37ab9d22305b6aafa2ef9811a6d Mon Sep 17 00:00:00 2001
From: Jegor Sidorenko <5252494+jsidorenko@users.noreply.github.com>
Date: Wed, 5 Jul 2023 11:17:08 +0200
Subject: [PATCH] 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 | 4 +--
.../frame/nft-fractionalization/src/tests.rs | 27 +++++++++++++++++++
.../nfts/src/features/create_delete_item.rs | 4 +++
substrate/frame/nfts/src/impl_nonfungibles.rs | 7 +++++
4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/substrate/frame/nft-fractionalization/src/lib.rs b/substrate/frame/nft-fractionalization/src/lib.rs
index a6417cc3857..b1663e95d85 100644
--- a/substrate/frame/nft-fractionalization/src/lib.rs
+++ b/substrate/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,
diff --git a/substrate/frame/nft-fractionalization/src/tests.rs b/substrate/frame/nft-fractionalization/src/tests.rs
index 8564b80533e..b82402bda1e 100644
--- a/substrate/frame/nft-fractionalization/src/tests.rs
+++ b/substrate/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(
diff --git a/substrate/frame/nfts/src/features/create_delete_item.rs b/substrate/frame/nfts/src/features/create_delete_item.rs
index a757273445f..e3d1334d48e 100644
--- a/substrate/frame/nfts/src/features/create_delete_item.rs
+++ b/substrate/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
diff --git a/substrate/frame/nfts/src/impl_nonfungibles.rs b/substrate/frame/nfts/src/impl_nonfungibles.rs
index b9e3951422f..090a6a372c1 100644
--- a/substrate/frame/nfts/src/impl_nonfungibles.rs
+++ b/substrate/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,
--
GitLab