Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed pallet::getter usage from pallet-collective #3456

Merged
merged 10 commits into from
Mar 5, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ where
}

fn proposal_of(proposal_hash: HashOf<T>) -> Option<ProposalOf<T, I>> {
pallet_collective::Pallet::<T, I>::proposal_of(proposal_hash)
pallet_collective::ProposalOf::<T, I>::get(proposal_hash)
}
}

Expand Down
15 changes: 15 additions & 0 deletions prdoc/pr_3456.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# 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` from `pallet-collective`
muraca marked this conversation as resolved.
Show resolved Hide resolved

doc:
- audience: Runtime Dev
description: |
This PR removes all the `pallet::getter` usages from `pallet-collective`, and updates depdendant runtimes accordingly.
muraca marked this conversation as resolved.
Show resolved Hide resolved
The syntax `StorageItem::<T, I>::get()` should be used instead.

crates:
- name: pallet-collective
- name: kitchensink-runtime
- name: collectives-westend-runtime
8 changes: 4 additions & 4 deletions substrate/bin/node/runtime/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use pallet_identity::legacy::IdentityField;
use sp_std::prelude::*;

use crate::{
AccountId, AllianceMotion, Assets, Authorship, Balances, Hash, NegativeImbalance, Runtime,
RuntimeCall,
AccountId, AllianceCollective, AllianceMotion, Assets, Authorship, Balances, Hash,
NegativeImbalance, Runtime, RuntimeCall,
};

pub struct Author;
Expand Down Expand Up @@ -107,7 +107,7 @@ impl ProposalProvider<AccountId, Hash, RuntimeCall> for AllianceProposalProvider
}

fn proposal_of(proposal_hash: Hash) -> Option<RuntimeCall> {
AllianceMotion::proposal_of(proposal_hash)
pallet_collective::ProposalOf::<Runtime, AllianceCollective>::get(proposal_hash)
}
}

Expand Down Expand Up @@ -276,7 +276,7 @@ mod multiplier_tests {
let next = runtime_multiplier_update(fm);
fm = next;
if fm == min_multiplier() {
break
break;
}
iterations += 1;
}
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/alliance/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl ProposalProvider<AccountId, H256, RuntimeCall> for AllianceProposalProvider
}

fn proposal_of(proposal_hash: H256) -> Option<RuntimeCall> {
AllianceMotion::proposal_of(proposal_hash)
pallet_collective::ProposalOf::<Test, Instance1>::get(proposal_hash)
}
}

Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/alliance/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ fn propose_works() {
Box::new(proposal.clone()),
proposal_len
));
assert_eq!(*AllianceMotion::proposals(), vec![hash]);
assert_eq!(AllianceMotion::proposal_of(&hash), Some(proposal));
assert_eq!(*pallet_collective::Proposals::<Test, Instance1>::get(), vec![hash]);
assert_eq!(pallet_collective::ProposalOf::<Test, Instance1>::get(&hash), Some(proposal));
assert_eq!(
System::events(),
vec![EventRecord {
Expand Down
32 changes: 16 additions & 16 deletions substrate/frame/collective/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ benchmarks_instance_pallet! {
}: _(SystemOrigin::Root, new_members.clone(), new_members.last().cloned(), T::MaxMembers::get())
verify {
new_members.sort();
assert_eq!(Collective::<T, I>::members(), new_members);
assert_eq!(Members::<T, I>::get(), new_members);
}

execute {
Expand Down Expand Up @@ -199,14 +199,14 @@ benchmarks_instance_pallet! {
)?;
}

assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);

let proposal: T::Proposal = SystemCall::<T>::remark { remark: id_to_remark_data(p, b as usize) }.into();

}: propose(SystemOrigin::Signed(caller.clone()), threshold, Box::new(proposal.clone()), bytes_in_storage)
verify {
// New proposal is recorded
assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);
let proposal_hash = T::Hashing::hash_of(&proposal);
assert_last_event::<T, I>(Event::Proposed { account: caller, proposal_index: p - 1, proposal_hash, threshold }.into());
}
Expand Down Expand Up @@ -269,7 +269,7 @@ benchmarks_instance_pallet! {
approve,
)?;

assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

// Voter switches vote to nay, but does not kill the vote, just updates + inserts
let approve = false;
Expand All @@ -280,8 +280,8 @@ benchmarks_instance_pallet! {
}: _(SystemOrigin::Signed(voter), last_hash, index, approve)
verify {
// All proposals exist and the last proposal has just been updated.
assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
let voting = Collective::<T, I>::voting(&last_hash).ok_or("Proposal Missing")?;
assert_eq!(Proposals::<T, I>::get().len(), p as usize);
let voting = Voting::<T, I>::get(&last_hash).ok_or("Proposal Missing")?;
assert_eq!(voting.ayes.len(), (m - 3) as usize);
assert_eq!(voting.nays.len(), 1);
}
Expand Down Expand Up @@ -344,7 +344,7 @@ benchmarks_instance_pallet! {
approve,
)?;

assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

// Voter switches vote to nay, which kills the vote
let approve = false;
Expand All @@ -361,7 +361,7 @@ benchmarks_instance_pallet! {
}: close(SystemOrigin::Signed(voter), last_hash, index, Weight::MAX, bytes_in_storage)
verify {
// The last proposal is removed.
assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
}

Expand Down Expand Up @@ -428,7 +428,7 @@ benchmarks_instance_pallet! {
true,
)?;

assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

// Caller switches vote to aye, which passes the vote
let index = p - 1;
Expand All @@ -442,7 +442,7 @@ benchmarks_instance_pallet! {
}: close(SystemOrigin::Signed(caller), last_hash, index, Weight::MAX, bytes_in_storage)
verify {
// The last proposal is removed.
assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
assert_last_event::<T, I>(Event::Executed { proposal_hash: last_hash, result: Ok(()) }.into());
}

Expand Down Expand Up @@ -519,12 +519,12 @@ benchmarks_instance_pallet! {
)?;

System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

// Prime nay will close it as disapproved
}: close(SystemOrigin::Signed(caller), last_hash, index, Weight::MAX, bytes_in_storage)
verify {
assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
}

Expand Down Expand Up @@ -591,12 +591,12 @@ benchmarks_instance_pallet! {

// caller is prime, prime already votes aye by creating the proposal
System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

// Prime aye will close it as approved
}: close(SystemOrigin::Signed(caller), last_hash, p - 1, Weight::MAX, bytes_in_storage)
verify {
assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
assert_last_event::<T, I>(Event::Executed { proposal_hash: last_hash, result: Ok(()) }.into());
}

Expand Down Expand Up @@ -640,11 +640,11 @@ benchmarks_instance_pallet! {
}

System::<T>::set_block_number(BlockNumberFor::<T>::max_value());
assert_eq!(Collective::<T, I>::proposals().len(), p as usize);
assert_eq!(Proposals::<T, I>::get().len(), p as usize);

}: _(SystemOrigin::Root, last_hash)
verify {
assert_eq!(Collective::<T, I>::proposals().len(), (p - 1) as usize);
assert_eq!(Proposals::<T, I>::get().len(), (p - 1) as usize);
assert_last_event::<T, I>(Event::Disapproved { proposal_hash: last_hash }.into());
}

Expand Down
Loading
Loading