Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[NFTs] Offchain mint #13158

Merged
merged 27 commits into from
Feb 14, 2023
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
bdf9f7d
Allow to mint with the pre-signed signatures
jsidorenko Jan 12, 2023
320582d
Another try
jsidorenko Jan 12, 2023
a92dc15
WIP: test encoder
jsidorenko Jan 13, 2023
124d872
Fix the deposits
jsidorenko Jan 17, 2023
bb91166
Refactoring + tests + benchmarks
jsidorenko Jan 17, 2023
9f7e563
Add sp-core/runtime-benchmarks
jsidorenko Jan 17, 2023
a4c7e79
Remove sp-core from dev deps
jsidorenko Jan 17, 2023
09f86aa
Enable full_crypto for benchmarks
jsidorenko Jan 17, 2023
dc9ff18
Typo
jsidorenko Jan 20, 2023
55eeb12
Fix
jsidorenko Jan 21, 2023
e8ea9ec
Update frame/nfts/src/mock.rs
jsidorenko Jan 21, 2023
5f27ced
Merge branch 'master' of https://github.com/paritytech/substrate into…
Jan 21, 2023
62edf5f
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nfts
Jan 21, 2023
b119156
Add docs
jsidorenko Jan 22, 2023
781f834
Add attributes into the pre-signed object & track the deposit owner f…
jsidorenko Jan 31, 2023
839c37c
Update docs
jsidorenko Jan 31, 2023
4980270
Merge branch 'master' into js/offchain-mint
jsidorenko Jan 31, 2023
c2d7c99
Merge branch 'master' of https://github.com/paritytech/substrate into…
Jan 31, 2023
7120dd0
".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nfts
Jan 31, 2023
088dddd
Add the number of attributes provided to weights
jsidorenko Jan 31, 2023
ad7b0e1
Merge branch 'master' into js/offchain-mint
jsidorenko Feb 1, 2023
c53df9c
Apply suggestions
jsidorenko Feb 6, 2023
321daa5
Remove dead code
jsidorenko Feb 6, 2023
01c4d8d
Remove Copy
jsidorenko Feb 6, 2023
de9b190
Fix docs
jsidorenko Feb 6, 2023
c879e2f
Update frame/nfts/src/lib.rs
jsidorenko Feb 13, 2023
566d3c8
Update frame/nfts/src/lib.rs
jsidorenko Feb 13, 2023
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
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
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