diff --git a/pallets/nfts/src/benchmarking.rs b/pallets/nfts/src/benchmarking.rs index 07762273..047c08ca 100644 --- a/pallets/nfts/src/benchmarking.rs +++ b/pallets/nfts/src/benchmarking.rs @@ -657,7 +657,7 @@ benchmarks_instance_pallet! { } clear_collection_approvals { - let n in 1 .. T::ApprovalsLimit::get(); + let n in 1 .. 1_000; let (collection, caller, _) = create_collection::(); mint_item::(0); for i in 0 .. n { @@ -675,7 +675,7 @@ benchmarks_instance_pallet! { } force_clear_collection_approvals { - let n in 1 .. T::ApprovalsLimit::get(); + let n in 1 .. 1_000; let (collection, caller, _) = create_collection::(); let caller_lookup = T::Lookup::unlookup(caller.clone()); mint_item::(0); diff --git a/pallets/nfts/src/features/approvals.rs b/pallets/nfts/src/features/approvals.rs index 4c580421..8a740f39 100644 --- a/pallets/nfts/src/features/approvals.rs +++ b/pallets/nfts/src/features/approvals.rs @@ -304,8 +304,10 @@ impl, I: 'static> Pallet { } } - T::Currency::unreserve(&owner, deposits); - Self::deposit_event(Event::ApprovalsCancelled { collection, item: None, owner }); + if removed_approvals > 0 { + T::Currency::unreserve(&owner, deposits); + Self::deposit_event(Event::ApprovalsCancelled { collection, item: None, owner }); + } Ok(removed_approvals) } diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index a8fb136f..938f7981 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -2004,7 +2004,7 @@ fn approval_lifecycle_works() { } #[test] -fn check_approval_works_without_deadline_works() { +fn check_approval_without_deadline_works() { new_test_ext().execute_with(|| { let collection_id = 0; let collection_owner = account(1); @@ -2622,6 +2622,18 @@ fn approve_collection_transfer_works() { Error::::NoItemOwned ); + // Force-approve unknown collection, throws error `Error::NoItemOwned`. + assert_noop!( + Nfts::force_approve_collection_transfer( + RuntimeOrigin::root(), + item_owner.clone(), + collection_id, + delegate.clone(), + None + ), + Error::::NoItemOwned + ); + assert_ok!(Nfts::force_create( RuntimeOrigin::root(), collection_owner.clone(), @@ -2644,6 +2656,18 @@ fn approve_collection_transfer_works() { Error::::NoItemOwned ); + // Force-approve collection without items, throws error `Error::NoItemOwned`. + assert_noop!( + Nfts::force_approve_collection_transfer( + RuntimeOrigin::root(), + item_owner.clone(), + collection_id, + delegate.clone(), + None + ), + Error::::NoItemOwned + ); + assert_ok!(Nfts::force_mint( RuntimeOrigin::signed(collection_owner.clone()), collection_id, @@ -2682,18 +2706,6 @@ fn approve_collection_transfer_works() { Error::::ItemsNonTransferable ); - // Approve unknown collection, throws error `Error::NoItemOwned`. - assert_noop!( - approve_collection_transfer( - origin.clone(), - maybe_item_owner.clone(), - 2, - delegate.clone(), - None - ), - Error::::NoItemOwned - ); - assert_ok!(approve_collection_transfer( origin.clone(), maybe_item_owner.clone(), @@ -3183,6 +3195,17 @@ fn clear_collection_approvals_works() { .count() .is_zero()); + // Emitting no event if zero approvals removed. + assert_eq!( + clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 10), + Ok(Some(WeightOf::::clear_collection_approvals(0)).into()) + ); + assert!(!events().contains(&Event::::ApprovalsCancelled { + collection: collection_id, + item: None, + owner: owner.clone(), + })); + assert_noop!( Nfts::transfer( RuntimeOrigin::signed(delegate_1.clone()), diff --git a/pallets/nfts/src/weights.rs b/pallets/nfts/src/weights.rs index 91faa0ae..6e291bfc 100644 --- a/pallets/nfts/src/weights.rs +++ b/pallets/nfts/src/weights.rs @@ -2,7 +2,7 @@ //! Autogenerated weights for `pallet_nfts` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 40.0.0 -//! DATE: 2024-12-06, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-12-12, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `R0GUE`, CPU: `` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` @@ -633,33 +633,33 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: `Nfts::CollectionApprovals` (r:21 w:20) + /// Storage: `Nfts::CollectionApprovals` (r:1000 w:999) /// Proof: `Nfts::CollectionApprovals` (`max_values`: None, `max_size`: Some(137), added: 2612, mode: `MaxEncodedLen`) - /// The range of component `n` is `[1, 20]`. + /// The range of component `n` is `[1, 1000]`. fn clear_collection_approvals(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `348 + n * (75 ±0)` + // Measured: `327 + n * (75 ±0)` // Estimated: `3602 + n * (2612 ±0)` // Minimum execution time: 18_000_000 picoseconds. - Weight::from_parts(16_365_393, 3602) - // Standard Error: 5_564 - .saturating_add(Weight::from_parts(3_008_044, 0).saturating_mul(n.into())) + Weight::from_parts(18_000_000, 3602) + // Standard Error: 7_954 + .saturating_add(Weight::from_parts(3_625_558, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2612).saturating_mul(n.into())) } - /// Storage: `Nfts::CollectionApprovals` (r:21 w:20) + /// Storage: `Nfts::CollectionApprovals` (r:1000 w:999) /// Proof: `Nfts::CollectionApprovals` (`max_values`: None, `max_size`: Some(137), added: 2612, mode: `MaxEncodedLen`) - /// The range of component `n` is `[1, 20]`. + /// The range of component `n` is `[1, 1000]`. fn force_clear_collection_approvals(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `348 + n * (75 ±0)` + // Measured: `327 + n * (75 ±0)` // Estimated: `3602 + n * (2612 ±0)` // Minimum execution time: 18_000_000 picoseconds. - Weight::from_parts(16_193_192, 3602) - // Standard Error: 5_637 - .saturating_add(Weight::from_parts(3_029_821, 0).saturating_mul(n.into())) + Weight::from_parts(18_000_000, 3602) + // Standard Error: 5_861 + .saturating_add(Weight::from_parts(3_576_220, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) @@ -1424,33 +1424,33 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: `Nfts::CollectionApprovals` (r:21 w:20) + /// Storage: `Nfts::CollectionApprovals` (r:1000 w:999) /// Proof: `Nfts::CollectionApprovals` (`max_values`: None, `max_size`: Some(137), added: 2612, mode: `MaxEncodedLen`) - /// The range of component `n` is `[1, 20]`. + /// The range of component `n` is `[1, 1000]`. fn clear_collection_approvals(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `348 + n * (75 ±0)` + // Measured: `327 + n * (75 ±0)` // Estimated: `3602 + n * (2612 ±0)` // Minimum execution time: 18_000_000 picoseconds. - Weight::from_parts(16_365_393, 3602) - // Standard Error: 5_564 - .saturating_add(Weight::from_parts(3_008_044, 0).saturating_mul(n.into())) + Weight::from_parts(18_000_000, 3602) + // Standard Error: 7_954 + .saturating_add(Weight::from_parts(3_625_558, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(n.into()))) .saturating_add(Weight::from_parts(0, 2612).saturating_mul(n.into())) } - /// Storage: `Nfts::CollectionApprovals` (r:21 w:20) + /// Storage: `Nfts::CollectionApprovals` (r:1000 w:999) /// Proof: `Nfts::CollectionApprovals` (`max_values`: None, `max_size`: Some(137), added: 2612, mode: `MaxEncodedLen`) - /// The range of component `n` is `[1, 20]`. + /// The range of component `n` is `[1, 1000]`. fn force_clear_collection_approvals(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `348 + n * (75 ±0)` + // Measured: `327 + n * (75 ±0)` // Estimated: `3602 + n * (2612 ±0)` // Minimum execution time: 18_000_000 picoseconds. - Weight::from_parts(16_193_192, 3602) - // Standard Error: 5_637 - .saturating_add(Weight::from_parts(3_029_821, 0).saturating_mul(n.into())) + Weight::from_parts(18_000_000, 3602) + // Standard Error: 5_861 + .saturating_add(Weight::from_parts(3_576_220, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(n.into()))) .saturating_add(RocksDbWeight::get().writes((1_u64).saturating_mul(n.into())))