From cf8729e82eb2adc7a5332493cd45fc882bc1cb2a Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Dec 2024 17:08:39 +0100 Subject: [PATCH 1/5] refactor: tests --- pallets/nfts/src/tests.rs | 365 ++++++++++++++++---------------------- 1 file changed, 148 insertions(+), 217 deletions(-) diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index 3c224eea..e0dedf40 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -39,9 +39,11 @@ use sp_runtime::{ use crate::{mock::*, Event, SystemConfig, *}; -type CollectionId = ::CollectionId; +type CollectionId = ::CollectionId; +type CollectionApprovalDeposit = ::CollectionApprovalDeposit; +type CollectionApprovals = crate::CollectionApprovals; type AccountIdOf = ::AccountId; -type WeightOf = ::WeightInfo; +type WeightOf = ::WeightInfo; fn account(id: u8) -> AccountIdOf { [id; 32].into() @@ -164,44 +166,44 @@ fn item_config_from_disabled_settings(settings: BitFlags) -> ItemCo fn clear_collection_approvals( origin: RuntimeOrigin, - maybe_owner: Option, + maybe_owner: Option<&AccountId>, collection: CollectionId, limit: u32, ) -> DispatchResultWithPostInfo { match maybe_owner { - Some(owner) => Nfts::force_clear_collection_approvals(origin, owner, collection, limit), + Some(owner) => Nfts::force_clear_collection_approvals(origin, owner.clone(), collection, limit), None => Nfts::clear_collection_approvals(origin, collection, limit), } } fn approve_collection_transfer( origin: RuntimeOrigin, - maybe_owner: Option, + maybe_owner: Option<&AccountId>, collection: CollectionId, - delegate: AccountIdOf, + delegate: &AccountId, maybe_deadline: Option>, ) -> DispatchResult { match maybe_owner { Some(owner) => Nfts::force_approve_collection_transfer( origin, - owner, + owner.clone(), collection, - delegate, + delegate.clone(), maybe_deadline, ), - None => Nfts::approve_collection_transfer(origin, collection, delegate, maybe_deadline), + None => Nfts::approve_collection_transfer(origin, collection, delegate.clone(), maybe_deadline), } } fn cancel_collection_approval( origin: RuntimeOrigin, - maybe_owner: Option, + maybe_owner: Option<&AccountId>, collection: CollectionId, - delegate: AccountIdOf, + delegate: &AccountId, ) -> DispatchResult { match maybe_owner { - Some(owner) => Nfts::force_cancel_collection_approval(origin, owner, collection, delegate), - None => Nfts::cancel_collection_approval(origin, collection, delegate), + Some(owner) => Nfts::force_cancel_collection_approval(origin, owner.clone(), collection, delegate.clone()), + None => Nfts::cancel_collection_approval(origin, collection, delegate.clone()), } } @@ -2415,7 +2417,7 @@ fn cancel_collection_approval_works() { // Parameters for `cancel_collection_approval`. (RuntimeOrigin::signed(item_owner.clone()), None), // Parameters for `force_cancel_collection_approval`. - (root(), Some(item_owner.clone())), + (root(), Some(&item_owner)), ] { assert_ok!(Nfts::approve_collection_transfer( RuntimeOrigin::signed(item_owner.clone()), @@ -2423,15 +2425,23 @@ fn cancel_collection_approval_works() { delegate.clone(), None )); - assert_eq!(Balances::reserved_balance(&item_owner), 1); - - // Cancel an unapproved delegate. + // Cancel an approval for a non existing collection. assert_noop!( cancel_collection_approval( origin.clone(), - maybe_owner.clone(), + maybe_owner, 1, - delegate.clone() + &delegate + ), + Error::::Unapproved + ); + // Cancel an unapproved delegate. + assert_noop!( + cancel_collection_approval( + origin.clone(), + maybe_owner, + collection_id, + &account(69), ), Error::::Unapproved ); @@ -2439,32 +2449,18 @@ fn cancel_collection_approval_works() { // Successfully cancel a collection approval. assert_ok!(cancel_collection_approval( origin.clone(), - maybe_owner.clone(), + maybe_owner, collection_id, - delegate.clone() + &delegate )); assert_eq!(Balances::reserved_balance(&item_owner), 0); - assert!(events().contains(&Event::::ApprovalCancelled { + assert!(!CollectionApprovals::contains_key((collection_id, &item_owner, &delegate))); + System::assert_last_event(Event::::ApprovalCancelled { collection: collection_id, item: None, owner: item_owner.clone(), delegate: delegate.clone() - })); - assert!(!CollectionApprovals::::contains_key(( - collection_id, - item_owner.clone(), - delegate.clone() - ))); - - assert_noop!( - Nfts::transfer( - RuntimeOrigin::signed(delegate.clone()), - collection_id, - item_id, - account(4) - ), - Error::::NoPermission - ); + }.into()); } }); } @@ -2577,8 +2573,9 @@ fn approvals_limit_works() { #[test] fn approve_collection_transfer_works() { new_test_ext().execute_with(|| { - let (collection_id, locked_collection_id) = (0, 1); + let mut collection_id = 0; let collection_owner = account(1); + let deadline: BlockNumberFor = 20; let delegate = account(3); let item_id = 42; let item_owner = account(2); @@ -2604,136 +2601,97 @@ fn approve_collection_transfer_works() { ); } + // Provide balance to accounts. Balances::make_free_balance_be(&item_owner, 100); Balances::make_free_balance_be(&delegate, 100); - // Approve unknown collection, throws error `Error::NoItemOwned`. - assert_noop!( - Nfts::approve_collection_transfer( - RuntimeOrigin::signed(item_owner.clone()), - collection_id, - delegate.clone(), - None - ), - 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(), - default_collection_config() - )); - assert_ok!(Nfts::force_create( - RuntimeOrigin::root(), - collection_owner.clone(), - default_collection_config() - )); - - // Approve collection without items, throws error `Error::NoItemOwned`. - assert_noop!( - Nfts::approve_collection_transfer( - RuntimeOrigin::signed(item_owner.clone()), - collection_id, - delegate.clone(), - None - ), - 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, - item_id, - item_owner.clone(), - default_item_config() - )); - assert_ok!(Nfts::force_mint( - RuntimeOrigin::signed(collection_owner.clone()), - locked_collection_id, - item_id, - item_owner.clone(), - default_item_config() - )); - for (origin, maybe_item_owner) in [ // Parameters for `approve_collection_transfer`. (RuntimeOrigin::signed(item_owner.clone()), None), // Parameters for `force_approve_collection_transfer`. - (root(), Some(item_owner.clone())), + (root(), Some(&item_owner)), ] { - // Throws error `Error::ItemsNonTransferable`. - assert_ok!(Nfts::lock_collection( - RuntimeOrigin::signed(collection_owner.clone()), - locked_collection_id, - CollectionSettings::from_disabled(CollectionSetting::TransferableItems.into()) - )); + // Approve unknown collection, throws error `Error::NoItemOwned`. assert_noop!( approve_collection_transfer( origin.clone(), - maybe_item_owner.clone(), - locked_collection_id, - delegate.clone(), + maybe_item_owner, + collection_id, + &delegate, None ), - Error::::ItemsNonTransferable + Error::::NoItemOwned ); + assert_ok!(Nfts::force_create( + RuntimeOrigin::root(), + collection_owner.clone(), + default_collection_config() + )); - assert_ok!(approve_collection_transfer( - origin.clone(), - maybe_item_owner.clone(), + // Approve collection without items, throws error `Error::NoItemOwned`. + assert_noop!( + Nfts::approve_collection_transfer( + RuntimeOrigin::signed(item_owner.clone()), + collection_id, + delegate.clone(), + None + ), + Error::::NoItemOwned + ); + assert_ok!(Nfts::force_mint( + RuntimeOrigin::signed(collection_owner.clone()), collection_id, - delegate.clone(), - None + item_id, + item_owner.clone(), + default_item_config() )); - assert!(events().contains(&Event::::TransferApproved { - collection: collection_id, - item: None, - owner: item_owner.clone(), - delegate: delegate.clone(), - deadline: None, - })); - assert_eq!(Balances::reserved_balance(&item_owner), 1); - // Approve same delegate again not updating the total reserved funds. - assert_ok!(approve_collection_transfer( - origin.clone(), - maybe_item_owner.clone(), + for deadline in [None, None, Some(deadline), Some(deadline * 2), Some(deadline), None] { + // Approve same delegate with no deadline. + assert_ok!(approve_collection_transfer( + origin.clone(), + maybe_item_owner, + collection_id, + &delegate, + deadline, + )); + let deadline = deadline.map(|d| d + 1); + System::assert_last_event(Event::::TransferApproved { + collection: collection_id, + item: None, + owner: item_owner.clone(), + delegate: delegate.clone(), + deadline, + }.into()); + assert_eq!(Balances::reserved_balance(&item_owner), 1); + assert_eq!(CollectionApprovals::get((collection_id, &item_owner, &delegate)), Some((deadline, CollectionApprovalDeposit::get()))); + + } + + // Set collection settings to non transferable, throws error `Error::ItemsNonTransferable`. + assert_ok!(Nfts::lock_collection( + RuntimeOrigin::signed(collection_owner.clone()), collection_id, - delegate.clone(), - None + CollectionSettings::from_disabled(CollectionSetting::TransferableItems.into()) )); - assert_eq!(Balances::reserved_balance(&item_owner), 1); + assert_noop!( + approve_collection_transfer( + origin.clone(), + maybe_item_owner, + collection_id, + &delegate, + None + ), + Error::::ItemsNonTransferable + ); - // Clean up. + // To reset reserved balance. assert_ok!(Nfts::cancel_collection_approval( RuntimeOrigin::signed(item_owner.clone()), collection_id, delegate.clone() )); + collection_id.saturating_inc(); } }); } @@ -2806,7 +2764,7 @@ fn cancel_approval_works_with_admin() { default_collection_config() )); assert_ok!(Nfts::force_mint( - RuntimeOrigin::signed(account(1)), + RuntimeOrigin::signed(collection_owner.clone()), collection_id, item_id, item_owner.clone(), @@ -2825,7 +2783,7 @@ fn cancel_approval_works_with_admin() { RuntimeOrigin::signed(item_owner.clone()), 1, item_id, - account(1) + collection_owner.clone() ), Error::::UnknownItem ); @@ -2834,7 +2792,7 @@ fn cancel_approval_works_with_admin() { RuntimeOrigin::signed(item_owner.clone()), collection_id, 43, - account(1) + collection_owner.clone() ), Error::::UnknownItem ); @@ -3075,7 +3033,7 @@ fn clear_all_transfer_approvals_works() { })); assert_eq!(approvals(collection_id, item_id), vec![]); assert_eq!( - CollectionApprovals::::iter_prefix((collection_id, item_owner.clone())).count(), + CollectionApprovals::iter_prefix((collection_id, item_owner.clone())).count(), 0 ); @@ -3095,131 +3053,104 @@ fn clear_collection_approvals_works() { new_test_ext().execute_with(|| { let balance = 100; let collection_id = 0; - let delegate_1 = account(3); - let delegate_2 = account(4); let item_id = 42; - let owner = account(1); + let item_owner = account(1); // Origin checks for the `clear_collection_approvals`. for origin in [root(), none()] { assert_noop!( Nfts::clear_collection_approvals(origin, collection_id, 0), - BadOrigin.with_weight(WeightOf::::clear_collection_approvals(0)) + BadOrigin.with_weight(WeightOf::clear_collection_approvals(0)) ); } // Origin checks for the `force_clear_collection_approvals`. - for origin in [RuntimeOrigin::signed(owner.clone()), none()] { + for origin in [RuntimeOrigin::signed(item_owner.clone()), none()] { assert_noop!( - Nfts::force_clear_collection_approvals(origin, owner.clone(), collection_id, 0), - BadOrigin.with_weight(WeightOf::::clear_collection_approvals(0)) + Nfts::force_clear_collection_approvals(origin, item_owner.clone(), collection_id, 0), + BadOrigin.with_weight(WeightOf::clear_collection_approvals(0)) ); } assert_ok!(Nfts::force_create( RuntimeOrigin::root(), - owner.clone(), + item_owner.clone(), default_collection_config() )); assert_ok!(Nfts::force_mint( - RuntimeOrigin::signed(account(1)), + RuntimeOrigin::signed(item_owner.clone()), collection_id, item_id, - owner.clone(), + item_owner.clone(), default_item_config() )); + Balances::make_free_balance_be(&item_owner, balance); for (origin, maybe_owner) in [ // Parameters for `clear_collection_approvals`. - (root(), Some(owner.clone())), + (root(), Some(&item_owner)), // Parameters for `force_clear_collection_approvals`. - (RuntimeOrigin::signed(owner.clone()), None), + (RuntimeOrigin::signed(item_owner.clone()), None), ] { - Balances::make_free_balance_be(&owner, balance); - assert_ok!(Nfts::approve_collection_transfer( - RuntimeOrigin::signed(owner.clone()), - collection_id, - delegate_1.clone(), - None - )); - assert_ok!(Nfts::approve_collection_transfer( - RuntimeOrigin::signed(owner.clone()), - collection_id, - delegate_2.clone(), - None - )); + // Approve 10 accounts. + let mut approvals = 0u64; + for i in 10..20 { + assert_ok!(Nfts::approve_collection_transfer( + RuntimeOrigin::signed(item_owner.clone()), + collection_id, + account(i), + None + )); + approvals.saturating_inc(); + } + // Removes zero collection approval. assert_eq!( - clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 0), - Ok(Some(WeightOf::::clear_collection_approvals(0)).into()) + clear_collection_approvals(origin.clone(), maybe_owner, collection_id, 0), + Ok(Some(WeightOf::clear_collection_approvals(0)).into()) ); - assert_eq!(Balances::free_balance(&owner), balance - 2); + assert_eq!(Balances::free_balance(&item_owner), balance - approvals); assert_eq!( - CollectionApprovals::::iter_prefix((collection_id, owner.clone())).count(), - 2 + CollectionApprovals::iter_prefix((collection_id, &item_owner)).count(), + approvals as usize ); assert!(!events().contains(&Event::::ApprovalsCancelled { collection: collection_id, item: None, - owner: owner.clone(), + owner: item_owner.clone(), })); // Partially removes collection approvals. assert_eq!( clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 1), - Ok(Some(WeightOf::::clear_collection_approvals(1)).into()) - ); - assert_eq!(Balances::free_balance(&owner), balance - 1); - assert_eq!( - CollectionApprovals::::iter_prefix((collection_id, owner.clone())).count(), - 1 + Ok(Some(WeightOf::clear_collection_approvals(1)).into()) ); - - // Successfully removes all collection approvals. Only charges post-dispatch weight for - // the removed approvals. + assert_eq!(Balances::free_balance(&item_owner), balance - approvals + 1); assert_eq!( - clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 10), - Ok(Some(WeightOf::::clear_collection_approvals(1)).into()) + CollectionApprovals::iter_prefix((collection_id, item_owner.clone())).count(), + approvals as usize - 1 ); assert!(events().contains(&Event::::ApprovalsCancelled { collection: collection_id, item: None, - owner: owner.clone(), + owner: item_owner.clone(), })); - assert_eq!(Balances::free_balance(&owner), balance); - assert!(CollectionApprovals::::iter_prefix((collection_id, owner.clone())) - .count() - .is_zero()); - // Emitting no event if zero approvals removed. + // Successfully remove all collection approvals. Only charges post-dispatch weight for + // the removed approvals. assert_eq!( clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 10), - Ok(Some(WeightOf::::clear_collection_approvals(0)).into()) + Ok(Some(WeightOf::clear_collection_approvals(9)).into()) ); - assert!(!events().contains(&Event::::ApprovalsCancelled { + assert_eq!(Balances::free_balance(&item_owner), balance); + assert!(CollectionApprovals::iter_prefix((collection_id, item_owner.clone())) + .count() + .is_zero()); + assert!(events().contains(&Event::::ApprovalsCancelled { collection: collection_id, item: None, - owner: owner.clone(), + owner: item_owner.clone(), })); - - assert_noop!( - Nfts::transfer( - RuntimeOrigin::signed(delegate_1.clone()), - collection_id, - item_id, - account(5) - ), - Error::::NoPermission - ); - assert_noop!( - Nfts::transfer( - RuntimeOrigin::signed(delegate_2.clone()), - collection_id, - item_id, - account(5) - ), - Error::::NoPermission - ); } }); } From e1a9e9d72301df1527c495f92381bad3d3a126a7 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Dec 2024 17:09:08 +0100 Subject: [PATCH 2/5] fmt --- pallets/nfts/src/tests.rs | 69 ++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index e0dedf40..c849b708 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -171,7 +171,8 @@ fn clear_collection_approvals( limit: u32, ) -> DispatchResultWithPostInfo { match maybe_owner { - Some(owner) => Nfts::force_clear_collection_approvals(origin, owner.clone(), collection, limit), + Some(owner) => + Nfts::force_clear_collection_approvals(origin, owner.clone(), collection, limit), None => Nfts::clear_collection_approvals(origin, collection, limit), } } @@ -191,7 +192,8 @@ fn approve_collection_transfer( delegate.clone(), maybe_deadline, ), - None => Nfts::approve_collection_transfer(origin, collection, delegate.clone(), maybe_deadline), + None => + Nfts::approve_collection_transfer(origin, collection, delegate.clone(), maybe_deadline), } } @@ -202,7 +204,12 @@ fn cancel_collection_approval( delegate: &AccountId, ) -> DispatchResult { match maybe_owner { - Some(owner) => Nfts::force_cancel_collection_approval(origin, owner.clone(), collection, delegate.clone()), + Some(owner) => Nfts::force_cancel_collection_approval( + origin, + owner.clone(), + collection, + delegate.clone(), + ), None => Nfts::cancel_collection_approval(origin, collection, delegate.clone()), } } @@ -2427,12 +2434,7 @@ fn cancel_collection_approval_works() { )); // Cancel an approval for a non existing collection. assert_noop!( - cancel_collection_approval( - origin.clone(), - maybe_owner, - 1, - &delegate - ), + cancel_collection_approval(origin.clone(), maybe_owner, 1, &delegate), Error::::Unapproved ); // Cancel an unapproved delegate. @@ -2455,12 +2457,15 @@ fn cancel_collection_approval_works() { )); assert_eq!(Balances::reserved_balance(&item_owner), 0); assert!(!CollectionApprovals::contains_key((collection_id, &item_owner, &delegate))); - System::assert_last_event(Event::::ApprovalCancelled { - collection: collection_id, - item: None, - owner: item_owner.clone(), - delegate: delegate.clone() - }.into()); + System::assert_last_event( + Event::::ApprovalCancelled { + collection: collection_id, + item: None, + owner: item_owner.clone(), + delegate: delegate.clone(), + } + .into(), + ); } }); } @@ -2656,19 +2661,25 @@ fn approve_collection_transfer_works() { deadline, )); let deadline = deadline.map(|d| d + 1); - System::assert_last_event(Event::::TransferApproved { - collection: collection_id, - item: None, - owner: item_owner.clone(), - delegate: delegate.clone(), - deadline, - }.into()); + System::assert_last_event( + Event::::TransferApproved { + collection: collection_id, + item: None, + owner: item_owner.clone(), + delegate: delegate.clone(), + deadline, + } + .into(), + ); assert_eq!(Balances::reserved_balance(&item_owner), 1); - assert_eq!(CollectionApprovals::get((collection_id, &item_owner, &delegate)), Some((deadline, CollectionApprovalDeposit::get()))); - + assert_eq!( + CollectionApprovals::get((collection_id, &item_owner, &delegate)), + Some((deadline, CollectionApprovalDeposit::get())) + ); } - // Set collection settings to non transferable, throws error `Error::ItemsNonTransferable`. + // Set collection settings to non transferable, throws error + // `Error::ItemsNonTransferable`. assert_ok!(Nfts::lock_collection( RuntimeOrigin::signed(collection_owner.clone()), collection_id, @@ -3066,7 +3077,12 @@ fn clear_collection_approvals_works() { // Origin checks for the `force_clear_collection_approvals`. for origin in [RuntimeOrigin::signed(item_owner.clone()), none()] { assert_noop!( - Nfts::force_clear_collection_approvals(origin, item_owner.clone(), collection_id, 0), + Nfts::force_clear_collection_approvals( + origin, + item_owner.clone(), + collection_id, + 0 + ), BadOrigin.with_weight(WeightOf::clear_collection_approvals(0)) ); } @@ -3091,7 +3107,6 @@ fn clear_collection_approvals_works() { // Parameters for `force_clear_collection_approvals`. (RuntimeOrigin::signed(item_owner.clone()), None), ] { - // Approve 10 accounts. let mut approvals = 0u64; for i in 10..20 { From e607ae5b6104c06681769955e9c71043fc445950 Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Dec 2024 17:12:14 +0100 Subject: [PATCH 3/5] refactor: type order --- pallets/nfts/src/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index c849b708..6185c77c 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -39,10 +39,10 @@ use sp_runtime::{ use crate::{mock::*, Event, SystemConfig, *}; -type CollectionId = ::CollectionId; -type CollectionApprovalDeposit = ::CollectionApprovalDeposit; -type CollectionApprovals = crate::CollectionApprovals; type AccountIdOf = ::AccountId; +type CollectionApprovals = crate::CollectionApprovals; +type CollectionApprovalDeposit = ::CollectionApprovalDeposit; +type CollectionId = ::CollectionId; type WeightOf = ::WeightInfo; fn account(id: u8) -> AccountIdOf { From 8629075a581619b1418dc92a1b64ddefc5c5c3db Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Thu, 12 Dec 2024 17:43:57 +0100 Subject: [PATCH 4/5] docs: deadlines --- pallets/nfts/src/tests.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index 6185c77c..fce4dabf 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -2651,6 +2651,9 @@ fn approve_collection_transfer_works() { default_item_config() )); + // Approving a collection with no deadline (`None`) and a deadline. The repetition is to + // demonstrate all cases possible (e.g. approving a delegate with no deadline to with a + // deadline). for deadline in [None, None, Some(deadline), Some(deadline * 2), Some(deadline), None] { // Approve same delegate with no deadline. assert_ok!(approve_collection_transfer( From bde7aa6ebce340bae272b606a7d303137c6163ab Mon Sep 17 00:00:00 2001 From: Daanvdplas Date: Fri, 13 Dec 2024 09:46:29 +0100 Subject: [PATCH 5/5] docs: improve --- pallets/nfts/src/tests.rs | 45 +++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/pallets/nfts/src/tests.rs b/pallets/nfts/src/tests.rs index fce4dabf..ac16c456 100644 --- a/pallets/nfts/src/tests.rs +++ b/pallets/nfts/src/tests.rs @@ -2651,11 +2651,25 @@ fn approve_collection_transfer_works() { default_item_config() )); - // Approving a collection with no deadline (`None`) and a deadline. The repetition is to - // demonstrate all cases possible (e.g. approving a delegate with no deadline to with a - // deadline). - for deadline in [None, None, Some(deadline), Some(deadline * 2), Some(deadline), None] { - // Approve same delegate with no deadline. + // Approving a collection to a delegate with: + // 1. no deadline. + // 2. no deadline, again. + // 3. deadline. + // 3. equal deadline. + // 4. larger deadline. + // 5. smaller deadline. + // 6. no deadline, again. + // + // This tests all cases of approving the same delegate. + for deadline in [ + None, + None, + Some(deadline), + Some(deadline), + Some(deadline * 2), + Some(deadline), + None, + ] { assert_ok!(approve_collection_transfer( origin.clone(), maybe_item_owner, @@ -3069,6 +3083,7 @@ fn clear_collection_approvals_works() { let collection_id = 0; let item_id = 42; let item_owner = account(1); + let delegates = 10..20; // Origin checks for the `clear_collection_approvals`. for origin in [root(), none()] { @@ -3110,9 +3125,9 @@ fn clear_collection_approvals_works() { // Parameters for `force_clear_collection_approvals`. (RuntimeOrigin::signed(item_owner.clone()), None), ] { - // Approve 10 accounts. + // Approve delegates. let mut approvals = 0u64; - for i in 10..20 { + for i in delegates.clone() { assert_ok!(Nfts::approve_collection_transfer( RuntimeOrigin::signed(item_owner.clone()), collection_id, @@ -3122,7 +3137,7 @@ fn clear_collection_approvals_works() { approvals.saturating_inc(); } - // Removes zero collection approval. + // Remove zero collection approvals. assert_eq!( clear_collection_approvals(origin.clone(), maybe_owner, collection_id, 0), Ok(Some(WeightOf::clear_collection_approvals(0)).into()) @@ -3138,7 +3153,7 @@ fn clear_collection_approvals_works() { owner: item_owner.clone(), })); - // Partially removes collection approvals. + // Partially remove collection approvals. assert_eq!( clear_collection_approvals(origin.clone(), maybe_owner.clone(), collection_id, 1), Ok(Some(WeightOf::clear_collection_approvals(1)).into()) @@ -3169,6 +3184,18 @@ fn clear_collection_approvals_works() { item: None, owner: item_owner.clone(), })); + // Ensure delegates are not able to transfer. + for i in delegates.clone() { + assert_noop!( + Nfts::transfer( + RuntimeOrigin::signed(account(i)), + collection_id, + item_id, + account(5) + ), + Error::::NoPermission + ); + } } }); }