From a2c9ab8c043221f4902739d678739b1fa9319cef Mon Sep 17 00:00:00 2001 From: Matteo Muraca <56828990+muraca@users.noreply.github.com> Date: Mon, 1 Apr 2024 07:17:20 +0200 Subject: [PATCH] Removed `pallet::getter` usage from `pallet-alliance` (#3738) Part of #3326 cc @kianenigma @ggwpez @liamaharon polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp --------- Signed-off-by: Matteo Muraca Co-authored-by: Liam Aharon Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- prdoc/pr_3738.prdoc | 14 +++++ substrate/frame/alliance/src/benchmarking.rs | 10 ++-- substrate/frame/alliance/src/lib.rs | 7 --- substrate/frame/alliance/src/migration.rs | 12 ++--- substrate/frame/alliance/src/tests.rs | 55 +++++++++++--------- 5 files changed, 54 insertions(+), 44 deletions(-) create mode 100644 prdoc/pr_3738.prdoc diff --git a/prdoc/pr_3738.prdoc b/prdoc/pr_3738.prdoc new file mode 100644 index 000000000000..cbf19b95c36a --- /dev/null +++ b/prdoc/pr_3738.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Removed `pallet::getter` usage from `pallet-alliance` + +doc: + - audience: Runtime Dev + description: | + This PR removes `pallet::getter` usage from `pallet-alliance`, and updates dependant code accordingly. + The syntax `StorageItem::::get()` should be used instead. + +crates: + - name: pallet-alliance + bump: major diff --git a/substrate/frame/alliance/src/benchmarking.rs b/substrate/frame/alliance/src/benchmarking.rs index 4ccd0fc08f6a..710c32a848dd 100644 --- a/substrate/frame/alliance/src/benchmarking.rs +++ b/substrate/frame/alliance/src/benchmarking.rs @@ -505,8 +505,8 @@ mod benchmarks { assert_last_event::( Event::MembersInitialized { fellows: fellows.clone(), allies: allies.clone() }.into(), ); - assert_eq!(Alliance::::members(MemberRole::Fellow), fellows); - assert_eq!(Alliance::::members(MemberRole::Ally), allies); + assert_eq!(Members::::get(MemberRole::Fellow), fellows); + assert_eq!(Members::::get(MemberRole::Ally), allies); Ok(()) } @@ -563,7 +563,7 @@ mod benchmarks { { call.dispatch_bypass_filter(origin)?; } - assert_eq!(Alliance::::rule(), Some(rule.clone())); + assert_eq!(Rule::::get(), Some(rule.clone())); assert_last_event::(Event::NewRuleSet { rule }.into()); Ok(()) } @@ -583,7 +583,7 @@ mod benchmarks { call.dispatch_bypass_filter(origin)?; } - assert!(Alliance::::announcements().contains(&announcement)); + assert!(Announcements::::get().contains(&announcement)); assert_last_event::(Event::Announced { announcement }.into()); Ok(()) } @@ -606,7 +606,7 @@ mod benchmarks { call.dispatch_bypass_filter(origin)?; } - assert!(!Alliance::::announcements().contains(&announcement)); + assert!(!Announcements::::get().contains(&announcement)); assert_last_event::(Event::AnnouncementRemoved { announcement }.into()); Ok(()) } diff --git a/substrate/frame/alliance/src/lib.rs b/substrate/frame/alliance/src/lib.rs index 414d550c53a9..1f06241e9c83 100644 --- a/substrate/frame/alliance/src/lib.rs +++ b/substrate/frame/alliance/src/lib.rs @@ -442,24 +442,20 @@ pub mod pallet { /// The IPFS CID of the alliance rule. /// Fellows can propose a new rule with a super-majority. #[pallet::storage] - #[pallet::getter(fn rule)] pub type Rule, I: 'static = ()> = StorageValue<_, Cid, OptionQuery>; /// The current IPFS CIDs of any announcements. #[pallet::storage] - #[pallet::getter(fn announcements)] pub type Announcements, I: 'static = ()> = StorageValue<_, BoundedVec, ValueQuery>; /// Maps members to their candidacy deposit. #[pallet::storage] - #[pallet::getter(fn deposit_of)] pub type DepositOf, I: 'static = ()> = StorageMap<_, Blake2_128Concat, T::AccountId, BalanceOf, OptionQuery>; /// Maps member type to members of each type. #[pallet::storage] - #[pallet::getter(fn members)] pub type Members, I: 'static = ()> = StorageMap< _, Twox64Concat, @@ -471,20 +467,17 @@ pub mod pallet { /// A set of members who gave a retirement notice. They can retire after the end of retirement /// period stored as a future block number. #[pallet::storage] - #[pallet::getter(fn retiring_members)] pub type RetiringMembers, I: 'static = ()> = StorageMap<_, Blake2_128Concat, T::AccountId, BlockNumberFor, OptionQuery>; /// The current list of accounts deemed unscrupulous. These accounts non grata cannot submit /// candidacy. #[pallet::storage] - #[pallet::getter(fn unscrupulous_accounts)] pub type UnscrupulousAccounts, I: 'static = ()> = StorageValue<_, BoundedVec, ValueQuery>; /// The current list of websites deemed unscrupulous. #[pallet::storage] - #[pallet::getter(fn unscrupulous_websites)] pub type UnscrupulousWebsites, I: 'static = ()> = StorageValue<_, BoundedVec, T::MaxUnscrupulousItems>, ValueQuery>; diff --git a/substrate/frame/alliance/src/migration.rs b/substrate/frame/alliance/src/migration.rs index 432f09a16f47..b4ecc1819447 100644 --- a/substrate/frame/alliance/src/migration.rs +++ b/substrate/frame/alliance/src/migration.rs @@ -162,18 +162,18 @@ pub(crate) mod v1_to_v2 { #[cfg(test)] mod test { use super::*; - use crate::{mock::*, MemberRole}; + use crate::{mock::*, MemberRole, Members}; #[test] fn migration_v1_to_v2_works() { new_test_ext().execute_with(|| { assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4))); - assert_eq!(Alliance::members(MemberRole::Ally), vec![4]); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(Members::::get(MemberRole::Ally), vec![4]); + assert_eq!(Members::::get(MemberRole::Fellow), vec![1, 2, 3]); v1_to_v2::migrate::(); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3, 4]); - assert_eq!(Alliance::members(MemberRole::Ally), vec![]); - assert_eq!(Alliance::members(MemberRole::Retiring), vec![]); + assert_eq!(Members::::get(MemberRole::Fellow), vec![1, 2, 3, 4]); + assert_eq!(Members::::get(MemberRole::Ally), vec![]); + assert_eq!(Members::::get(MemberRole::Retiring), vec![]); }); } } diff --git a/substrate/frame/alliance/src/tests.rs b/substrate/frame/alliance/src/tests.rs index c65f10228e77..edb515b8115a 100644 --- a/substrate/frame/alliance/src/tests.rs +++ b/substrate/frame/alliance/src/tests.rs @@ -21,7 +21,7 @@ use frame_support::{assert_noop, assert_ok, error::BadOrigin}; use frame_system::{EventRecord, Phase}; use super::*; -use crate::mock::*; +use crate::{self as alliance, mock::*}; type AllianceMotionEvent = pallet_collective::Event; @@ -118,7 +118,7 @@ fn disband_works() { // join alliance and reserve funds assert_eq!(Balances::free_balance(9), 1000 - id_deposit); assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(9))); - assert_eq!(Alliance::deposit_of(9), Some(expected_join_deposit)); + assert_eq!(alliance::DepositOf::::get(9), Some(expected_join_deposit)); assert_eq!(Balances::free_balance(9), 1000 - id_deposit - expected_join_deposit); assert!(Alliance::is_member_of(&9, MemberRole::Ally)); @@ -314,7 +314,7 @@ fn set_rule_works() { new_test_ext().execute_with(|| { let cid = test_cid(); assert_ok!(Alliance::set_rule(RuntimeOrigin::signed(1), cid.clone())); - assert_eq!(Alliance::rule(), Some(cid.clone())); + assert_eq!(alliance::Rule::::get(), Some(cid.clone())); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::NewRuleSet { rule: cid, @@ -330,7 +330,7 @@ fn announce_works() { assert_noop!(Alliance::announce(RuntimeOrigin::signed(2), cid.clone()), BadOrigin); assert_ok!(Alliance::announce(RuntimeOrigin::signed(3), cid.clone())); - assert_eq!(Alliance::announcements(), vec![cid.clone()]); + assert_eq!(alliance::Announcements::::get(), vec![cid.clone()]); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::Announced { announcement: cid, @@ -343,7 +343,7 @@ fn remove_announcement_works() { new_test_ext().execute_with(|| { let cid = test_cid(); assert_ok!(Alliance::announce(RuntimeOrigin::signed(3), cid.clone())); - assert_eq!(Alliance::announcements(), vec![cid.clone()]); + assert_eq!(alliance::Announcements::::get(), vec![cid.clone()]); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::Announced { announcement: cid.clone(), })); @@ -351,7 +351,7 @@ fn remove_announcement_works() { System::set_block_number(2); assert_ok!(Alliance::remove_announcement(RuntimeOrigin::signed(3), cid.clone())); - assert_eq!(Alliance::announcements(), vec![]); + assert_eq!(alliance::Announcements::::get(), vec![]); System::assert_last_event(mock::RuntimeEvent::Alliance( crate::Event::AnnouncementRemoved { announcement: cid }, )); @@ -394,8 +394,8 @@ fn join_alliance_works() { // success to submit assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4))); assert_eq!(Balances::free_balance(4), 1000 - id_deposit - join_deposit); - assert_eq!(Alliance::deposit_of(4), Some(25)); - assert_eq!(Alliance::members(MemberRole::Ally), vec![4]); + assert_eq!(alliance::DepositOf::::get(4), Some(25)); + assert_eq!(alliance::Members::::get(MemberRole::Ally), vec![4]); // check already member assert_noop!( @@ -449,8 +449,8 @@ fn nominate_ally_works() { // success to nominate assert_ok!(Alliance::nominate_ally(RuntimeOrigin::signed(1), 4)); - assert_eq!(Alliance::deposit_of(4), None); - assert_eq!(Alliance::members(MemberRole::Ally), vec![4]); + assert_eq!(alliance::DepositOf::::get(4), None); + assert_eq!(alliance::Members::::get(MemberRole::Ally), vec![4]); // check already member assert_noop!( @@ -482,12 +482,12 @@ fn elevate_ally_works() { ); assert_ok!(Alliance::join_alliance(RuntimeOrigin::signed(4))); - assert_eq!(Alliance::members(MemberRole::Ally), vec![4]); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Ally), vec![4]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3]); assert_ok!(Alliance::elevate_ally(RuntimeOrigin::signed(2), 4)); - assert_eq!(Alliance::members(MemberRole::Ally), Vec::::new()); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3, 4]); + assert_eq!(alliance::Members::::get(MemberRole::Ally), Vec::::new()); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3, 4]); }); } @@ -499,10 +499,10 @@ fn give_retirement_notice_work() { Error::::NotMember ); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3]); assert_ok!(Alliance::give_retirement_notice(RuntimeOrigin::signed(3))); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2]); - assert_eq!(Alliance::members(MemberRole::Retiring), vec![3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2]); + assert_eq!(alliance::Members::::get(MemberRole::Retiring), vec![3]); System::assert_last_event(mock::RuntimeEvent::Alliance( crate::Event::MemberRetirementPeriodStarted { member: (3) }, )); @@ -527,7 +527,7 @@ fn retire_works() { Error::::RetirementNoticeNotGiven ); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3]); assert_ok!(Alliance::give_retirement_notice(RuntimeOrigin::signed(3))); assert_noop!( Alliance::retire(RuntimeOrigin::signed(3)), @@ -535,7 +535,7 @@ fn retire_works() { ); System::set_block_number(System::block_number() + RetirementPeriod::get()); assert_ok!(Alliance::retire(RuntimeOrigin::signed(3))); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2]); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::MemberRetired { member: (3), unreserved: None, @@ -551,7 +551,7 @@ fn retire_works() { #[test] fn abdicate_works() { new_test_ext().execute_with(|| { - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3]); assert_ok!(Alliance::abdicate_fellow_status(RuntimeOrigin::signed(3))); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::FellowAbdicated { @@ -573,9 +573,9 @@ fn kick_member_works() { ); >::insert(2, 25); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 2, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 2, 3]); assert_ok!(Alliance::kick_member(RuntimeOrigin::signed(2), 2)); - assert_eq!(Alliance::members(MemberRole::Fellow), vec![1, 3]); + assert_eq!(alliance::Members::::get(MemberRole::Fellow), vec![1, 3]); assert_eq!(>::get(2), None); System::assert_last_event(mock::RuntimeEvent::Alliance(crate::Event::MemberKicked { member: (2), @@ -596,8 +596,11 @@ fn add_unscrupulous_items_works() { UnscrupulousItem::Website("abc".as_bytes().to_vec().try_into().unwrap()) ] )); - assert_eq!(Alliance::unscrupulous_accounts().into_inner(), vec![3]); - assert_eq!(Alliance::unscrupulous_websites().into_inner(), vec!["abc".as_bytes().to_vec()]); + assert_eq!(alliance::UnscrupulousAccounts::::get().into_inner(), vec![3]); + assert_eq!( + alliance::UnscrupulousWebsites::::get().into_inner(), + vec!["abc".as_bytes().to_vec()] + ); assert_noop!( Alliance::add_unscrupulous_items( @@ -629,12 +632,12 @@ fn remove_unscrupulous_items_works() { RuntimeOrigin::signed(3), vec![UnscrupulousItem::AccountId(3)] )); - assert_eq!(Alliance::unscrupulous_accounts(), vec![3]); + assert_eq!(alliance::UnscrupulousAccounts::::get(), vec![3]); assert_ok!(Alliance::remove_unscrupulous_items( RuntimeOrigin::signed(3), vec![UnscrupulousItem::AccountId(3)] )); - assert_eq!(Alliance::unscrupulous_accounts(), Vec::::new()); + assert_eq!(alliance::UnscrupulousAccounts::::get(), Vec::::new()); }); }