Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,7 @@ impl pallet_uniques::Config for Runtime {

parameter_types! {
pub Features: PalletFeatures = PalletFeatures::all_enabled();
pub const MaxAttributesPerCall: u32 = 10;
}

impl pallet_nfts::Config for Runtime {
Expand All @@ -1582,7 +1583,10 @@ impl pallet_nfts::Config for Runtime {
type ItemAttributesApprovalsLimit = ItemAttributesApprovalsLimit;
type MaxTips = MaxTips;
type MaxDeadlineDuration = MaxDeadlineDuration;
type MaxAttributesPerCall = MaxAttributesPerCall;
type Features = Features;
type OffchainSignature = Signature;
type OffchainPublic = <Signature as traits::Verify>::Signer;
type WeightInfo = pallet_nfts::weights::SubstrateWeight<Runtime>;
#[cfg(feature = "runtime-benchmarks")]
type Helper = ();
Expand Down
6 changes: 3 additions & 3 deletions frame/nfts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional
frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" }
frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" }
sp-core = { version = "7.0.0", default-features = false, path = "../../primitives/core" }
sp-io = { version = "7.0.0", default-features = false, path = "../../primitives/io" }
sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" }
sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" }

[dev-dependencies]
pallet-balances = { version = "4.0.0-dev", path = "../balances" }
sp-core = { version = "7.0.0", path = "../../primitives/core" }
sp-io = { version = "7.0.0", path = "../../primitives/io" }
sp-std = { version = "5.0.0", path = "../../primitives/std" }
sp-keystore = { version = "0.13.0", path = "../../primitives/keystore" }

[features]
default = ["std"]
Expand All @@ -40,6 +39,7 @@ std = [
"log/std",
"scale-info/std",
"sp-core/std",
"sp-io/std",
"sp-runtime/std",
"sp-std/std",
]
Expand Down
68 changes: 62 additions & 6 deletions frame/nfts/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ use frame_support::{
BoundedVec,
};
use frame_system::RawOrigin as SystemOrigin;
use sp_runtime::traits::{Bounded, One};
use sp_io::crypto::{sr25519_generate, sr25519_sign};
use sp_runtime::{
traits::{Bounded, IdentifyAccount, One},
AccountId32, MultiSignature, MultiSigner,
};
use sp_std::prelude::*;

use crate::Pallet as Nfts;
Expand Down Expand Up @@ -148,7 +152,21 @@ fn default_item_config() -> ItemConfig {
ItemConfig { settings: ItemSettings::all_enabled() }
}

fn make_filled_vec(value: u16, length: usize) -> Vec<u8> {
let mut vec = vec![0u8; length];
let mut s = Vec::from(value.to_be_bytes());
vec.truncate(length - s.len());
vec.append(&mut s);
vec
}

benchmarks_instance_pallet! {
where_clause {
where
T::OffchainSignature: From<MultiSignature>,
T::AccountId: From<AccountId32>,
}

create {
let collection = T::Helper::collection(0);
let origin = T::CreateOrigin::try_successful_origin(&collection)
Expand Down Expand Up @@ -439,11 +457,7 @@ benchmarks_instance_pallet! {
T::Currency::make_free_balance_be(&target, DepositBalanceOf::<T, I>::max_value());
let value: BoundedVec<_, _> = vec![0u8; T::ValueLimit::get() as usize].try_into().unwrap();
for i in 0..n {
let mut key = vec![0u8; T::KeyLimit::get() as usize];
let mut s = Vec::from((i as u16).to_be_bytes());
key.truncate(s.len());
key.append(&mut s);

let key = make_filled_vec(i as u16, T::KeyLimit::get() as usize);
Nfts::<T, I>::set_attribute(
SystemOrigin::Signed(target.clone()).into(),
T::Helper::collection(0),
Expand Down Expand Up @@ -717,5 +731,47 @@ benchmarks_instance_pallet! {
}.into());
}

mint_pre_signed {
let n in 0 .. T::MaxAttributesPerCall::get() as u32;
let caller_public = sr25519_generate(0.into(), None);
let caller = MultiSigner::Sr25519(caller_public).into_account().into();
T::Currency::make_free_balance_be(&caller, DepositBalanceOf::<T, I>::max_value());
let caller_lookup = T::Lookup::unlookup(caller.clone());

let collection = T::Helper::collection(0);
let item = T::Helper::item(0);
assert_ok!(Nfts::<T, I>::force_create(
SystemOrigin::Root.into(),
caller_lookup.clone(),
default_collection_config::<T, I>()
));

let metadata = vec![0u8; T::StringLimit::get() as usize];
let mut attributes = vec![];
let attribute_value = vec![0u8; T::ValueLimit::get() as usize];
for i in 0..n {
let attribute_key = make_filled_vec(i as u16, T::KeyLimit::get() as usize);
attributes.push((attribute_key, attribute_value.clone()));
}
let mint_data = PreSignedMint {
collection,
item,
attributes,
metadata: metadata.clone(),
only_account: None,
deadline: One::one(),
};
let message = Encode::encode(&mint_data);
let signature = MultiSignature::Sr25519(sr25519_sign(0.into(), &caller_public, &message).unwrap());

let target: T::AccountId = account("target", 0, SEED);
T::Currency::make_free_balance_be(&target, DepositBalanceOf::<T, I>::max_value());
frame_system::Pallet::<T>::set_block_number(One::one());
}: _(SystemOrigin::Signed(target.clone()), mint_data, signature.into(), caller)
verify {
let metadata: BoundedVec<_, _> = metadata.try_into().unwrap();
assert_last_event::<T, I>(Event::ItemMetadataSet { collection, item, data: metadata }.into());
}

impl_benchmark_test_suite!(Nfts, crate::mock::new_test_ext(), crate::mock::Test);
}
2 changes: 1 addition & 1 deletion frame/nfts/src/common_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Various pieces of common functionality.

use super::*;
use crate::*;

impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Get the owner of the item, if the item exists.
Expand Down
81 changes: 56 additions & 25 deletions frame/nfts/src/features/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
namespace: AttributeNamespace<T::AccountId>,
key: BoundedVec<u8, T::KeyLimit>,
value: BoundedVec<u8, T::ValueLimit>,
depositor: T::AccountId,
) -> DispatchResult {
ensure!(
Self::is_pallet_feature_enabled(PalletFeature::Attributes),
Expand Down Expand Up @@ -66,14 +67,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

let attribute = Attribute::<T, I>::get((collection, maybe_item, &namespace, &key));
if attribute.is_none() {
let attribute_exists = attribute.is_some();
if !attribute_exists {
collection_details.attributes.saturating_inc();
}

let old_deposit =
attribute.map_or(AttributeDeposit { account: None, amount: Zero::zero() }, |m| m.1);

let mut deposit = Zero::zero();
// disabled DepositRequired setting only affects the CollectionOwner namespace
if collection_config.is_setting_enabled(CollectionSetting::DepositRequired) ||
namespace != AttributeNamespace::CollectionOwner
{
Expand All @@ -82,33 +85,50 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.saturating_add(T::AttributeDepositBase::get());
}

let is_collection_owner_namespace = namespace == AttributeNamespace::CollectionOwner;
let is_depositor_collection_owner =
is_collection_owner_namespace && collection_details.owner == depositor;

// NOTE: in the CollectionOwner namespace if the depositor is `None` that means the deposit
// was paid by the collection's owner.
let old_depositor =
if is_collection_owner_namespace && old_deposit.account.is_none() && attribute_exists {
Some(collection_details.owner.clone())
} else {
old_deposit.account
};
let depositor_has_changed = old_depositor != Some(depositor.clone());

// NOTE: when we transfer an item, we don't move attributes in the ItemOwner namespace.
// When the new owner updates the same attribute, we will update the depositor record
// and return the deposit to the previous owner.
if old_deposit.account.is_some() && old_deposit.account != Some(origin.clone()) {
T::Currency::unreserve(&old_deposit.account.unwrap(), old_deposit.amount);
T::Currency::reserve(&origin, deposit)?;
if depositor_has_changed {
if let Some(old_depositor) = old_depositor {
T::Currency::unreserve(&old_depositor, old_deposit.amount);
}
T::Currency::reserve(&depositor, deposit)?;
} else if deposit > old_deposit.amount {
T::Currency::reserve(&origin, deposit - old_deposit.amount)?;
T::Currency::reserve(&depositor, deposit - old_deposit.amount)?;
} else if deposit < old_deposit.amount {
T::Currency::unreserve(&origin, old_deposit.amount - deposit);
T::Currency::unreserve(&depositor, old_deposit.amount - deposit);
}

// NOTE: we don't track the depositor in the CollectionOwner namespace as it's always a
// collection's owner. This simplifies the collection's transfer to another owner.
let deposit_owner = match namespace {
AttributeNamespace::CollectionOwner => {
collection_details.owner_deposit.saturating_accrue(deposit);
if is_depositor_collection_owner {
if !depositor_has_changed {
collection_details.owner_deposit.saturating_reduce(old_deposit.amount);
None
},
_ => Some(origin),
};
}
collection_details.owner_deposit.saturating_accrue(deposit);
}

let new_deposit_owner = match is_depositor_collection_owner {
true => None,
false => Some(depositor),
};
Attribute::<T, I>::insert(
(&collection, maybe_item, &namespace, &key),
(&value, AttributeDeposit { account: deposit_owner, amount: deposit }),
(&value, AttributeDeposit { account: new_deposit_owner, amount: deposit }),
);

Collection::<T, I>::insert(collection, &collection_details);
Self::deposit_event(Event::AttributeSet { collection, maybe_item, key, value, namespace });
Ok(())
Expand Down Expand Up @@ -188,27 +208,38 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// NOTE: if the item was previously burned, the ItemConfigOf record
// might not exist. In that case, we allow to clear the attribute.
let maybe_is_locked = Self::get_item_config(&collection, &item)
.map_or(false, |c| {
c.has_disabled_setting(ItemSetting::UnlockedAttributes)
.map_or(None, |c| {
Some(c.has_disabled_setting(ItemSetting::UnlockedAttributes))
});
ensure!(!maybe_is_locked, Error::<T, I>::LockedItemAttributes);
match maybe_is_locked {
Some(is_locked) => {
// when item exists, then only the collection's owner can clear that
// attribute
ensure!(
check_owner == &collection_details.owner,
Error::<T, I>::NoPermission
);
ensure!(!is_locked, Error::<T, I>::LockedItemAttributes);
},
None => (),
}
},
},
_ => (),
};
}

collection_details.attributes.saturating_dec();
match namespace {
AttributeNamespace::CollectionOwner => {

match deposit.account {
Some(deposit_account) => {
T::Currency::unreserve(&deposit_account, deposit.amount);
},
None if namespace == AttributeNamespace::CollectionOwner => {
collection_details.owner_deposit.saturating_reduce(deposit.amount);
T::Currency::unreserve(&collection_details.owner, deposit.amount);
},
_ => (),
};

if let Some(deposit_account) = deposit.account {
T::Currency::unreserve(&deposit_account, deposit.amount);
}

Collection::<T, I>::insert(collection, &collection_details);
Expand Down
56 changes: 56 additions & 0 deletions frame/nfts/src/features/create_delete_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,62 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(())
}

pub(crate) fn do_mint_pre_signed(
mint_to: T::AccountId,
mint_data: PreSignedMintOf<T, I>,
signer: T::AccountId,
) -> DispatchResult {
let PreSignedMint { collection, item, attributes, metadata, deadline, only_account } =
mint_data;
let metadata = Self::construct_metadata(metadata)?;

ensure!(
attributes.len() <= T::MaxAttributesPerCall::get() as usize,
Error::<T, I>::MaxAttributesLimitReached
);
if let Some(account) = only_account {
ensure!(account == mint_to, Error::<T, I>::WrongOrigin);
}

let now = frame_system::Pallet::<T>::block_number();
ensure!(deadline >= now, Error::<T, I>::DeadlineExpired);

let collection_details =
Collection::<T, I>::get(&collection).ok_or(Error::<T, I>::UnknownCollection)?;
ensure!(collection_details.owner == signer, Error::<T, I>::NoPermission);

let item_config = ItemConfig { settings: Self::get_default_item_settings(&collection)? };
Self::do_mint(
collection,
item,
Some(mint_to.clone()),
mint_to.clone(),
item_config,
|_, _| Ok(()),
)?;
for (key, value) in attributes {
Self::do_set_attribute(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you take in consideration that this extrinsic can fail after persisting some state?
Every do_* has its own ensure checks.
I am not sure if its correct.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it could fail if some provided data is incorrect (currently the only possible cases are when the attributes feature is disabled by feature flags or when the NFT is locked). In that case, the strategy "it's all or nothing" applies here. The storage state will get reverted if the extrinsic fails.

collection_details.owner.clone(),
collection,
Some(item),
AttributeNamespace::CollectionOwner,
Self::construct_attribute_key(key)?,
Self::construct_attribute_value(value)?,
mint_to.clone(),
)?;
}
if !metadata.len().is_zero() {
Self::do_set_item_metadata(
Some(collection_details.owner.clone()),
collection,
item,
metadata,
Some(mint_to.clone()),
)?;
}
Ok(())
}

pub fn do_burn(
collection: T::CollectionId,
item: T::ItemId,
Expand Down
21 changes: 15 additions & 6 deletions frame/nfts/src/features/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
.saturating_add(T::MetadataDepositBase::get());
}

// the previous deposit was taken from the item's owner
if old_deposit.account.is_some() && maybe_depositor.is_none() {
T::Currency::unreserve(&old_deposit.account.unwrap(), old_deposit.amount);
T::Currency::reserve(&collection_details.owner, deposit)?;
let depositor = maybe_depositor.clone().unwrap_or(collection_details.owner.clone());
let old_depositor = old_deposit.account.unwrap_or(collection_details.owner.clone());

if depositor != old_depositor {
T::Currency::unreserve(&old_depositor, old_deposit.amount);
T::Currency::reserve(&depositor, deposit)?;
} else if deposit > old_deposit.amount {
T::Currency::reserve(&collection_details.owner, deposit - old_deposit.amount)?;
T::Currency::reserve(&depositor, deposit - old_deposit.amount)?;
} else if deposit < old_deposit.amount {
T::Currency::unreserve(&collection_details.owner, old_deposit.amount - deposit);
T::Currency::unreserve(&depositor, old_deposit.amount - deposit);
}

if maybe_depositor.is_none() {
Expand Down Expand Up @@ -191,4 +193,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Ok(())
})
}

/// A helper method to construct metadata.
pub fn construct_metadata(
metadata: Vec<u8>,
) -> Result<BoundedVec<u8, T::StringLimit>, DispatchError> {
Ok(BoundedVec::try_from(metadata).map_err(|_| Error::<T, I>::IncorrectMetadata)?)
}
}
Loading