diff --git a/frame/nfts/src/benchmarking.rs b/frame/nfts/src/benchmarking.rs index 7368446e593df..eca404df2f142 100644 --- a/frame/nfts/src/benchmarking.rs +++ b/frame/nfts/src/benchmarking.rs @@ -42,13 +42,8 @@ fn create_collection, I: 'static>( let caller_lookup = T::Lookup::unlookup(caller.clone()); let collection = T::Helper::collection(0); T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); - assert!(Nfts::::force_create( - SystemOrigin::Root.into(), - collection, - caller_lookup.clone(), - false, - ) - .is_ok()); + assert!(Nfts::::force_create(SystemOrigin::Root.into(), caller_lookup.clone(), false,) + .is_ok()); (collection, caller, caller_lookup) } @@ -142,7 +137,7 @@ benchmarks_instance_pallet! { whitelist_account!(caller); let admin = T::Lookup::unlookup(caller.clone()); T::Currency::make_free_balance_be(&caller, DepositBalanceOf::::max_value()); - let call = Call::::create { collection, admin }; + let call = Call::::create { admin }; }: { call.dispatch_bypass_filter(origin)? } verify { assert_last_event::(Event::Created { collection: T::Helper::collection(0), creator: caller.clone(), owner: caller }.into()); @@ -151,7 +146,7 @@ benchmarks_instance_pallet! { force_create { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); - }: _(SystemOrigin::Root, T::Helper::collection(0), caller_lookup, true) + }: _(SystemOrigin::Root, caller_lookup, true) verify { assert_last_event::(Event::ForceCreated { collection: T::Helper::collection(0), owner: caller }.into()); } diff --git a/frame/nfts/src/functions.rs b/frame/nfts/src/functions.rs index 27ab752dbabf6..f935a5b2eba90 100644 --- a/frame/nfts/src/functions.rs +++ b/frame/nfts/src/functions.rs @@ -94,7 +94,12 @@ impl, I: 'static> Pallet { }, ); + let next_id = collection.increment(); + CollectionAccount::::insert(&owner, &collection, ()); + NextCollectionId::::set(Some(next_id)); + + Self::deposit_event(Event::NextCollectionIdIncremented { next_id }); Self::deposit_event(event); Ok(()) } @@ -284,4 +289,14 @@ impl, I: 'static> Pallet { Ok(()) } + + #[cfg(any(test, feature = "runtime-benchmarks"))] + pub fn set_next_id(id: T::CollectionId) { + NextCollectionId::::set(Some(id)); + } + + #[cfg(test)] + pub fn get_next_id() -> T::CollectionId { + NextCollectionId::::get().unwrap_or(T::CollectionId::initial_value()) + } } diff --git a/frame/nfts/src/lib.rs b/frame/nfts/src/lib.rs index cdb098d2eceed..2cde6e5e1ab5d 100644 --- a/frame/nfts/src/lib.rs +++ b/frame/nfts/src/lib.rs @@ -61,6 +61,29 @@ pub use weights::WeightInfo; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; +pub trait Incrementable { + fn increment(&self) -> Self; + fn initial_value() -> Self; +} + +macro_rules! impl_incrementable { + ($($type:ty),+) => { + $( + impl Incrementable for $type { + fn increment(&self) -> Self { + self.saturating_add(1) + } + + fn initial_value() -> Self { + 0 + } + } + )+ + }; +} + +impl_incrementable!(u8, u16, u32, u64, u128, i8, i16, i32, i64, i128); + #[frame_support::pallet] pub mod pallet { use super::*; @@ -94,7 +117,7 @@ pub mod pallet { + IsType<::RuntimeEvent>; /// Identifier for the collection of item. - type CollectionId: Member + Parameter + MaxEncodedLen + Copy; + type CollectionId: Member + Parameter + MaxEncodedLen + Copy + Incrementable; /// The type used to identify a unique item within a collection. type ItemId: Member + Parameter + MaxEncodedLen + Copy; @@ -278,6 +301,12 @@ pub mod pallet { pub(super) type CollectionMaxSupply, I: 'static = ()> = StorageMap<_, Blake2_128Concat, T::CollectionId, u32, OptionQuery>; + #[pallet::storage] + /// Stores the `CollectionId` that is going to be used for the next collection. + /// This gets incremented by 1 whenever a new collection is created. + pub(super) type NextCollectionId, I: 'static = ()> = + StorageValue<_, T::CollectionId, OptionQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -372,6 +401,8 @@ pub mod pallet { OwnershipAcceptanceChanged { who: T::AccountId, maybe_collection: Option }, /// Max supply has been set for a collection. CollectionMaxSupplySet { collection: T::CollectionId, max_supply: u32 }, + /// Event gets emmited when the `NextCollectionId` gets incremented. + NextCollectionIdIncremented { next_id: T::CollectionId }, /// The price was set for the instance. ItemPriceSet { collection: T::CollectionId, @@ -458,7 +489,6 @@ pub mod pallet { /// `ItemDeposit` funds of sender are reserved. /// /// Parameters: - /// - `collection`: The identifier of the new collection. This must not be currently in use. /// - `admin`: The admin of this collection. The admin is the initial address of each /// member of the collection's admin team. /// @@ -466,11 +496,10 @@ pub mod pallet { /// /// Weight: `O(1)` #[pallet::weight(T::WeightInfo::create())] - pub fn create( - origin: OriginFor, - collection: T::CollectionId, - admin: AccountIdLookupOf, - ) -> DispatchResult { + pub fn create(origin: OriginFor, admin: AccountIdLookupOf) -> DispatchResult { + let collection = + NextCollectionId::::get().unwrap_or(T::CollectionId::initial_value()); + let owner = T::CreateOrigin::ensure_origin(origin, &collection)?; let admin = T::Lookup::lookup(admin)?; @@ -492,7 +521,6 @@ pub mod pallet { /// /// Unlike `create`, no funds are reserved. /// - /// - `collection`: The identifier of the new item. This must not be currently in use. /// - `owner`: The owner of this collection of items. The owner has full superuser /// permissions /// over this item, but may later change and configure the permissions using @@ -504,13 +532,15 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::force_create())] pub fn force_create( origin: OriginFor, - collection: T::CollectionId, owner: AccountIdLookupOf, free_holding: bool, ) -> DispatchResult { T::ForceOrigin::ensure_origin(origin)?; let owner = T::Lookup::lookup(owner)?; + let collection = + NextCollectionId::::get().unwrap_or(T::CollectionId::initial_value()); + Self::do_create_collection( collection, owner.clone(), diff --git a/frame/nfts/src/tests.rs b/frame/nfts/src/tests.rs index 19d24f4924d46..f5d44bef7f886 100644 --- a/frame/nfts/src/tests.rs +++ b/frame/nfts/src/tests.rs @@ -98,12 +98,12 @@ fn basic_setup_works() { #[test] fn basic_minting_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_eq!(collections(), vec![(1, 0)]); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); assert_eq!(items(), vec![(1, 0, 42)]); - assert_ok!(Nfts::force_create(Origin::root(), 1, 2, true)); + assert_ok!(Nfts::force_create(Origin::root(), 2, true)); assert_eq!(collections(), vec![(1, 0), (2, 1)]); assert_ok!(Nfts::mint(Origin::signed(2), 1, 69, 1)); assert_eq!(items(), vec![(1, 0, 42), (1, 1, 69)]); @@ -114,7 +114,7 @@ fn basic_minting_should_work() { fn lifecycle_should_work() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::create(Origin::signed(1), 0, 1)); + assert_ok!(Nfts::create(Origin::signed(1), 1)); assert_eq!(Balances::reserved_balance(&1), 2); assert_eq!(collections(), vec![(1, 0)]); assert_ok!(Nfts::set_collection_metadata(Origin::signed(1), 0, bvec![0, 0], false)); @@ -157,7 +157,7 @@ fn lifecycle_should_work() { fn destroy_with_bad_witness_should_not_work() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::create(Origin::signed(1), 0, 1)); + assert_ok!(Nfts::create(Origin::signed(1), 1)); let w = Collection::::get(0).unwrap().destroy_witness(); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); @@ -168,7 +168,7 @@ fn destroy_with_bad_witness_should_not_work() { #[test] fn mint_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); assert_eq!(Nfts::owner(0, 42).unwrap(), 1); assert_eq!(collections(), vec![(1, 0)]); @@ -179,7 +179,7 @@ fn mint_should_work() { #[test] fn transfer_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::transfer(Origin::signed(2), 0, 42, 3)); @@ -194,7 +194,7 @@ fn transfer_should_work() { #[test] fn freezing_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); assert_ok!(Nfts::freeze(Origin::signed(1), 0, 42)); assert_noop!(Nfts::transfer(Origin::signed(1), 0, 42, 2), Error::::Frozen); @@ -211,7 +211,7 @@ fn freezing_should_work() { #[test] fn origin_guards_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); Balances::make_free_balance_be(&2, 100); @@ -236,7 +236,7 @@ fn transfer_owner_should_work() { Balances::make_free_balance_be(&1, 100); Balances::make_free_balance_be(&2, 100); Balances::make_free_balance_be(&3, 100); - assert_ok!(Nfts::create(Origin::signed(1), 0, 1)); + assert_ok!(Nfts::create(Origin::signed(1), 1)); assert_eq!(collections(), vec![(1, 0)]); assert_noop!(Nfts::transfer_ownership(Origin::signed(1), 0, 2), Error::::Unaccepted); assert_ok!(Nfts::set_accept_ownership(Origin::signed(2), Some(0))); @@ -275,7 +275,7 @@ fn transfer_owner_should_work() { #[test] fn set_team_should_work() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::set_team(Origin::signed(1), 0, 2, 3, 4)); assert_ok!(Nfts::mint(Origin::signed(2), 0, 42, 2)); @@ -294,7 +294,7 @@ fn set_collection_metadata_should_work() { Nfts::set_collection_metadata(Origin::signed(1), 0, bvec![0u8; 20], false), Error::::UnknownCollection, ); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); // Cannot add metadata to unowned item assert_noop!( Nfts::set_collection_metadata(Origin::signed(2), 0, bvec![0u8; 20], false), @@ -351,7 +351,7 @@ fn set_item_metadata_should_work() { Balances::make_free_balance_be(&1, 30); // Cannot add metadata to unknown item - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); // Cannot add metadata to unowned item assert_noop!( @@ -404,7 +404,7 @@ fn set_attribute_should_work() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); assert_ok!(Nfts::set_attribute(Origin::signed(1), 0, None, bvec![0], bvec![0])); assert_ok!(Nfts::set_attribute(Origin::signed(1), 0, Some(0), bvec![0], bvec![0])); @@ -449,7 +449,7 @@ fn set_attribute_should_respect_freeze() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); assert_ok!(Nfts::set_attribute(Origin::signed(1), 0, None, bvec![0], bvec![0])); assert_ok!(Nfts::set_attribute(Origin::signed(1), 0, Some(0), bvec![0], bvec![0])); @@ -481,7 +481,7 @@ fn force_item_status_should_work() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 1)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 69, 2)); assert_ok!(Nfts::set_collection_metadata(Origin::signed(1), 0, bvec![0; 20], false)); @@ -515,7 +515,7 @@ fn force_item_status_should_work() { fn burn_works() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&1, 100); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, false)); + assert_ok!(Nfts::force_create(Origin::root(), 1, false)); assert_ok!(Nfts::set_team(Origin::signed(1), 0, 2, 3, 4)); assert_noop!( @@ -539,7 +539,7 @@ fn burn_works() { #[test] fn approval_lifecycle_works() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::approve_transfer(Origin::signed(2), 0, 42, 3, None)); assert_ok!(Nfts::transfer(Origin::signed(3), 0, 42, 4)); @@ -554,7 +554,7 @@ fn approval_lifecycle_works() { #[test] fn cancel_approval_works() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::approve_transfer(Origin::signed(2), 0, 42, 3, None)); @@ -601,7 +601,7 @@ fn cancel_approval_works() { #[test] fn approving_multiple_accounts_works() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); let current_block = 1; @@ -620,7 +620,7 @@ fn approving_multiple_accounts_works() { #[test] fn approvals_limit_works() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); for i in 3..13 { @@ -640,7 +640,7 @@ fn approval_deadline_works() { System::set_block_number(0); assert!(System::block_number().is_zero()); - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); // the approval expires after the 2nd block. @@ -664,7 +664,7 @@ fn approval_deadline_works() { #[test] fn cancel_approval_works_with_admin() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::approve_transfer(Origin::signed(2), 0, 42, 3, None)); @@ -692,7 +692,7 @@ fn cancel_approval_works_with_admin() { #[test] fn cancel_approval_works_with_force() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::approve_transfer(Origin::signed(2), 0, 42, 3, None)); @@ -714,7 +714,7 @@ fn cancel_approval_works_with_force() { #[test] fn clear_all_transfer_approvals_works() { new_test_ext().execute_with(|| { - assert_ok!(Nfts::force_create(Origin::root(), 0, 1, true)); + assert_ok!(Nfts::force_create(Origin::root(), 1, true)); assert_ok!(Nfts::mint(Origin::signed(1), 0, 42, 2)); assert_ok!(Nfts::approve_transfer(Origin::signed(2), 0, 42, 3, None)); @@ -747,7 +747,7 @@ fn max_supply_should_work() { let max_supply = 2; // validate set_collection_max_supply - assert_ok!(Nfts::force_create(Origin::root(), collection_id, user_id, true)); + assert_ok!(Nfts::force_create(Origin::root(), user_id, true)); assert!(!CollectionMaxSupply::::contains_key(collection_id)); assert_ok!(Nfts::set_collection_max_supply( @@ -793,7 +793,7 @@ fn set_price_should_work() { let item_1 = 1; let item_2 = 2; - assert_ok!(Nfts::force_create(Origin::root(), collection_id, user_id, true)); + assert_ok!(Nfts::force_create(Origin::root(), user_id, true)); assert_ok!(Nfts::mint(Origin::signed(user_id), collection_id, item_1, user_id)); assert_ok!(Nfts::mint(Origin::signed(user_id), collection_id, item_2, user_id)); @@ -851,7 +851,7 @@ fn buy_item_should_work() { Balances::make_free_balance_be(&user_2, initial_balance); Balances::make_free_balance_be(&user_3, initial_balance); - assert_ok!(Nfts::force_create(Origin::root(), collection_id, user_1, true)); + assert_ok!(Nfts::force_create(Origin::root(), user_1, true)); assert_ok!(Nfts::mint(Origin::signed(user_1), collection_id, item_1, user_1)); assert_ok!(Nfts::mint(Origin::signed(user_1), collection_id, item_2, user_1));