Skip to content

Commit

Permalink
removed pallet::getter usage from cumulus pallets (#3471)
Browse files Browse the repository at this point in the history 
Part of #3326 

@ggwpez @kianenigma @shawntabrizi

polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

---------

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
  • Loading branch information
@muraca
muraca authored Mar 18, 2024
1 parent 0c6c837 commit 610987a
Show file tree
Hide file tree
Showing 9 changed files with 292 additions and 266 deletions.
6 changes: 3 additions & 3 deletions cumulus/pallets/aura-ext/src/consensus_hook.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ where
let velocity = V.max(1);
let relay_chain_slot = state_proof.read_slot().expect("failed to read relay chain slot");

let (slot, authored) = pallet::Pallet::<T>::slot_info()
.expect("slot info is inserted on block initialization");
let (slot, authored) =
pallet::SlotInfo::<T>::get().expect("slot info is inserted on block initialization");

// Convert relay chain timestamp.
let relay_chain_timestamp =
Expand Down Expand Up @@ -100,7 +100,7 @@ impl<
/// is more recent than the included block itself.
pub fn can_build_upon(included_hash: T::Hash, new_slot: Slot) -> bool {
let velocity = V.max(1);
let (last_slot, authored_so_far) = match pallet::Pallet::<T>::slot_info() {
let (last_slot, authored_so_far) = match pallet::SlotInfo::<T>::get() {
None => return true,
Some(x) => x,
};
Expand Down
1 change: 0 additions & 1 deletion cumulus/pallets/aura-ext/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ pub mod pallet {
///
/// Updated on each block initialization.
#[pallet::storage]
#[pallet::getter(fn slot_info)]
pub(crate) type SlotInfo<T: Config> = StorageValue<_, (Slot, u32), OptionQuery>;

#[pallet::genesis_config]
Expand Down
76 changes: 38 additions & 38 deletions cumulus/pallets/collator-selection/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,23 @@ fn register_validators<T: Config + session::Config>(count: u32) -> Vec<T::Accoun

fn register_candidates<T: Config>(count: u32) {
let candidates = (0..count).map(|c| account("candidate", c, SEED)).collect::<Vec<_>>();
assert!(<CandidacyBond<T>>::get() > 0u32.into(), "Bond cannot be zero!");
assert!(CandidacyBond::<T>::get() > 0u32.into(), "Bond cannot be zero!");

for who in candidates {
T::Currency::make_free_balance_be(&who, <CandidacyBond<T>>::get() * 3u32.into());
T::Currency::make_free_balance_be(&who, CandidacyBond::<T>::get() * 3u32.into());
<CollatorSelection<T>>::register_as_candidate(RawOrigin::Signed(who).into()).unwrap();
}
}

fn min_candidates<T: Config>() -> u32 {
let min_collators = T::MinEligibleCollators::get();
let invulnerable_length = <Invulnerables<T>>::get().len();
let invulnerable_length = Invulnerables::<T>::get().len();
min_collators.saturating_sub(invulnerable_length.try_into().unwrap())
}

fn min_invulnerables<T: Config>() -> u32 {
let min_collators = T::MinEligibleCollators::get();
let candidates_length = <CandidateList<T>>::decode_len()
let candidates_length = CandidateList::<T>::decode_len()
.unwrap_or_default()
.try_into()
.unwrap_or_default();
Expand Down Expand Up @@ -143,8 +143,8 @@ mod benchmarks {
T::UpdateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;

// need to fill up candidates
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(c);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(c);
// get accounts and keys for the `c` candidates
let mut candidates = (0..c).map(|cc| validator::<T>(cc)).collect::<Vec<_>>();
// add one more to the list. should not be in `b` (invulnerables) because it's the account
Expand All @@ -159,15 +159,15 @@ mod benchmarks {
}
// ... and register them.
for (who, _) in candidates.iter() {
let deposit = <CandidacyBond<T>>::get();
let deposit = CandidacyBond::<T>::get();
T::Currency::make_free_balance_be(who, deposit * 1000_u32.into());
<CandidateList<T>>::try_mutate(|list| {
CandidateList::<T>::try_mutate(|list| {
list.try_push(CandidateInfo { who: who.clone(), deposit }).unwrap();
Ok::<(), BenchmarkError>(())
})
.unwrap();
T::Currency::reserve(who, deposit)?;
<LastAuthoredBlock<T>>::insert(
LastAuthoredBlock::<T>::insert(
who.clone(),
frame_system::Pallet::<T>::block_number() + T::KickThreshold::get(),
);
Expand All @@ -178,7 +178,7 @@ mod benchmarks {
invulnerables.sort();
let invulnerables: frame_support::BoundedVec<_, T::MaxInvulnerables> =
frame_support::BoundedVec::try_from(invulnerables).unwrap();
<Invulnerables<T>>::put(invulnerables);
Invulnerables::<T>::put(invulnerables);

#[extrinsic_call]
_(origin as T::RuntimeOrigin, new_invulnerable.clone());
Expand All @@ -197,8 +197,8 @@ mod benchmarks {
invulnerables.sort();
let invulnerables: frame_support::BoundedVec<_, T::MaxInvulnerables> =
frame_support::BoundedVec::try_from(invulnerables).unwrap();
<Invulnerables<T>>::put(invulnerables);
let to_remove = <Invulnerables<T>>::get().first().unwrap().clone();
Invulnerables::<T>::put(invulnerables);
let to_remove = Invulnerables::<T>::get().first().unwrap().clone();

#[extrinsic_call]
_(origin as T::RuntimeOrigin, to_remove.clone());
Expand Down Expand Up @@ -226,14 +226,14 @@ mod benchmarks {
k: Linear<0, { T::MaxCandidates::get() }>,
) -> Result<(), BenchmarkError> {
let initial_bond_amount: BalanceOf<T> = T::Currency::minimum_balance() * 2u32.into();
<CandidacyBond<T>>::put(initial_bond_amount);
CandidacyBond::<T>::put(initial_bond_amount);
register_validators::<T>(c);
register_candidates::<T>(c);
let kicked = cmp::min(k, c);
let origin =
T::UpdateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let bond_amount = if k > 0 {
<CandidateList<T>>::mutate(|candidates| {
CandidateList::<T>::mutate(|candidates| {
for info in candidates.iter_mut().skip(kicked as usize) {
info.deposit = T::Currency::minimum_balance() * 3u32.into();
}
Expand All @@ -254,13 +254,13 @@ mod benchmarks {
fn update_bond(
c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>,
) -> Result<(), BenchmarkError> {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(c);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(c);

register_validators::<T>(c);
register_candidates::<T>(c);

let caller = <CandidateList<T>>::get()[0].who.clone();
let caller = CandidateList::<T>::get()[0].who.clone();
v2::whitelist!(caller);

let bond_amount: BalanceOf<T> =
Expand All @@ -273,7 +273,7 @@ mod benchmarks {
Event::CandidateBondUpdated { account_id: caller, deposit: bond_amount }.into(),
);
assert!(
<CandidateList<T>>::get().iter().last().unwrap().deposit ==
CandidateList::<T>::get().iter().last().unwrap().deposit ==
T::Currency::minimum_balance() * 2u32.into()
);
Ok(())
Expand All @@ -283,8 +283,8 @@ mod benchmarks {
// one.
#[benchmark]
fn register_as_candidate(c: Linear<1, { T::MaxCandidates::get() - 1 }>) {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(c + 1);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(c + 1);

register_validators::<T>(c);
register_candidates::<T>(c);
Expand All @@ -310,8 +310,8 @@ mod benchmarks {

#[benchmark]
fn take_candidate_slot(c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>) {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(1);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(1);

register_validators::<T>(c);
register_candidates::<T>(c);
Expand All @@ -327,7 +327,7 @@ mod benchmarks {
)
.unwrap();

let target = <CandidateList<T>>::get().iter().last().unwrap().who.clone();
let target = CandidateList::<T>::get().iter().last().unwrap().who.clone();

#[extrinsic_call]
_(RawOrigin::Signed(caller.clone()), bond / 2u32.into(), target.clone());
Expand All @@ -341,13 +341,13 @@ mod benchmarks {
// worse case is the last candidate leaving.
#[benchmark]
fn leave_intent(c: Linear<{ min_candidates::<T>() + 1 }, { T::MaxCandidates::get() }>) {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(c);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(c);

register_validators::<T>(c);
register_candidates::<T>(c);

let leaving = <CandidateList<T>>::get().iter().last().unwrap().who.clone();
let leaving = CandidateList::<T>::get().iter().last().unwrap().who.clone();
v2::whitelist!(leaving);

#[extrinsic_call]
Expand All @@ -359,7 +359,7 @@ mod benchmarks {
// worse case is paying a non-existing candidate account.
#[benchmark]
fn note_author() {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
CandidacyBond::<T>::put(T::Currency::minimum_balance());
T::Currency::make_free_balance_be(
&<CollatorSelection<T>>::account_id(),
T::Currency::minimum_balance() * 4u32.into(),
Expand All @@ -385,42 +385,42 @@ mod benchmarks {
r: Linear<1, { T::MaxCandidates::get() }>,
c: Linear<1, { T::MaxCandidates::get() }>,
) {
<CandidacyBond<T>>::put(T::Currency::minimum_balance());
<DesiredCandidates<T>>::put(c);
CandidacyBond::<T>::put(T::Currency::minimum_balance());
DesiredCandidates::<T>::put(c);
frame_system::Pallet::<T>::set_block_number(0u32.into());

register_validators::<T>(c);
register_candidates::<T>(c);

let new_block: BlockNumberFor<T> = T::KickThreshold::get();
let zero_block: BlockNumberFor<T> = 0u32.into();
let candidates: Vec<T::AccountId> = <CandidateList<T>>::get()
let candidates: Vec<T::AccountId> = CandidateList::<T>::get()
.iter()
.map(|candidate_info| candidate_info.who.clone())
.collect();

let non_removals = c.saturating_sub(r);

for i in 0..c {
<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), zero_block);
LastAuthoredBlock::<T>::insert(candidates[i as usize].clone(), zero_block);
}

if non_removals > 0 {
for i in 0..non_removals {
<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);
LastAuthoredBlock::<T>::insert(candidates[i as usize].clone(), new_block);
}
} else {
for i in 0..c {
<LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);
LastAuthoredBlock::<T>::insert(candidates[i as usize].clone(), new_block);
}
}

let min_candidates = min_candidates::<T>();
let pre_length = <CandidateList<T>>::decode_len().unwrap_or_default();
let pre_length = CandidateList::<T>::decode_len().unwrap_or_default();

frame_system::Pallet::<T>::set_block_number(new_block);

let current_length: u32 = <CandidateList<T>>::decode_len()
let current_length: u32 = CandidateList::<T>::decode_len()
.unwrap_or_default()
.try_into()
.unwrap_or_default();
Expand All @@ -434,20 +434,20 @@ mod benchmarks {
// candidates > removals and remaining candidates > min candidates
// => remaining candidates should be shorter than before removal, i.e. some were
// actually removed.
assert!(<CandidateList<T>>::decode_len().unwrap_or_default() < pre_length);
assert!(CandidateList::<T>::decode_len().unwrap_or_default() < pre_length);
} else if c > r && non_removals < min_candidates {
// candidates > removals and remaining candidates would be less than min candidates
// => remaining candidates should equal min candidates, i.e. some were removed up to
// the minimum, but then any more were "forced" to stay in candidates.
let current_length: u32 = <CandidateList<T>>::decode_len()
let current_length: u32 = CandidateList::<T>::decode_len()
.unwrap_or_default()
.try_into()
.unwrap_or_default();
assert!(min_candidates == current_length);
} else {
// removals >= candidates, non removals must == 0
// can't remove more than exist
assert!(<CandidateList<T>>::decode_len().unwrap_or_default() == pre_length);
assert!(CandidateList::<T>::decode_len().unwrap_or_default() == pre_length);
}
}

Expand Down
Loading

0 comments on commit 610987a

Please sign in to comment.