Skip to content

Commit

Permalink
Make bags-list generic over node value and instantiable (paritytech#1…
Browse files Browse the repository at this point in the history 
…0997)

* make instantiable

* update

* cargo fmt

* Clean up

* bags-list: Make it generic over node value

* Respond to some feedback

* Apply suggestions from code review

Co-authored-by: Kian Paimani <[email protected]>

* Add back default impl for weight update worst case

* Update to Score in more places'

* Use VoteWeight, not u64 to reduce test diff

* FMT

* FullCodec implies Codec

* formatting

* Fixup bags list remote test

Co-authored-by: doordashcon <[email protected]>
Co-authored-by: Doordashcon <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
  • Loading branch information
@emostov @Doordashcon
@kianenigma @grishasobol
4 people authored and grishasobol committed Mar 28, 2022
1 parent 5d5203e commit 2fc095b
Show file tree
Hide file tree
Showing 14 changed files with 428 additions and 283 deletions.
3 changes: 2 additions & 1 deletion bin/node/runtime/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,9 +690,10 @@ parameter_types! {

impl pallet_bags_list::Config for Runtime {
type Event = Event;
type VoteWeightProvider = Staking;
type ScoreProvider = Staking;
type WeightInfo = pallet_bags_list::weights::SubstrateWeight<Runtime>;
type BagThresholds = BagThresholds;
type Score = sp_npos_elections::VoteWeight;
}

parameter_types! {
Expand Down
23 changes: 18 additions & 5 deletions frame/bags-list/remote-tests/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

//! Utilities for remote-testing pallet-bags-list.
use frame_election_provider_support::ScoreProvider;
use sp_std::prelude::*;

/// A common log target to use.
Expand Down Expand Up @@ -55,8 +56,12 @@ pub fn display_and_check_bags<Runtime: RuntimeT>(currency_unit: u64, currency_na
let mut rebaggable = 0;
let mut active_bags = 0;
for vote_weight_thresh in <Runtime as pallet_bags_list::Config>::BagThresholds::get() {
let vote_weight_thresh_u64: u64 = (*vote_weight_thresh)
.try_into()
.map_err(|_| "runtime must configure score to at most u64 to use this test")
.unwrap();
// threshold in terms of UNITS (e.g. KSM, DOT etc)
let vote_weight_thresh_as_unit = *vote_weight_thresh as f64 / currency_unit as f64;
let vote_weight_thresh_as_unit = vote_weight_thresh_u64 as f64 / currency_unit as f64;
let pretty_thresh = format!("Threshold: {}. {}", vote_weight_thresh_as_unit, currency_name);

let bag = match pallet_bags_list::Pallet::<Runtime>::list_bags_get(*vote_weight_thresh) {
Expand All @@ -70,9 +75,13 @@ pub fn display_and_check_bags<Runtime: RuntimeT>(currency_unit: u64, currency_na
active_bags += 1;

for id in bag.std_iter().map(|node| node.std_id().clone()) {
let vote_weight = pallet_staking::Pallet::<Runtime>::weight_of(&id);
let vote_weight = <Runtime as pallet_bags_list::Config>::ScoreProvider::score(&id);
let vote_weight_thresh_u64: u64 = (*vote_weight_thresh)
.try_into()
.map_err(|_| "runtime must configure score to at most u64 to use this test")
.unwrap();
let vote_weight_as_balance: pallet_staking::BalanceOf<Runtime> =
vote_weight.try_into().map_err(|_| "can't convert").unwrap();
vote_weight_thresh_u64.try_into().map_err(|_| "can't convert").unwrap();

if vote_weight_as_balance < min_nominator_bond {
log::trace!(
Expand All @@ -87,13 +96,17 @@ pub fn display_and_check_bags<Runtime: RuntimeT>(currency_unit: u64, currency_na
pallet_bags_list::Node::<Runtime>::get(&id).expect("node in bag must exist.");
if node.is_misplaced(vote_weight) {
rebaggable += 1;
let notional_bag = pallet_bags_list::notional_bag_for::<Runtime, _>(vote_weight);
let notional_bag_as_u64: u64 = notional_bag
.try_into()
.map_err(|_| "runtime must configure score to at most u64 to use this test")
.unwrap();
log::trace!(
target: LOG_TARGET,
"Account {:?} can be rebagged from {:?} to {:?}",
id,
vote_weight_thresh_as_unit,
pallet_bags_list::notional_bag_for::<Runtime>(vote_weight) as f64 /
currency_unit as f64
notional_bag_as_u64 as f64 / currency_unit as f64
);
}
}
Expand Down
47 changes: 24 additions & 23 deletions frame/bags-list/src/benchmarks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
use super::*;
use crate::list::List;
use frame_benchmarking::{account, whitelist_account, whitelisted_caller};
use frame_election_provider_support::VoteWeightProvider;
use frame_election_provider_support::ScoreProvider;
use frame_support::{assert_ok, traits::Get};
use frame_system::RawOrigin as SystemOrigin;
use sp_runtime::traits::One;

frame_benchmarking::benchmarks! {
rebag_non_terminal {
Expand All @@ -36,29 +37,29 @@ frame_benchmarking::benchmarks! {

// clear any pre-existing storage.
// NOTE: safe to call outside block production
List::<T>::unsafe_clear();
List::<T, _>::unsafe_clear();

// define our origin and destination thresholds.
let origin_bag_thresh = T::BagThresholds::get()[0];
let dest_bag_thresh = T::BagThresholds::get()[1];

// seed items in the origin bag.
let origin_head: T::AccountId = account("origin_head", 0, 0);
assert_ok!(List::<T>::insert(origin_head.clone(), origin_bag_thresh));
assert_ok!(List::<T, _>::insert(origin_head.clone(), origin_bag_thresh));

let origin_middle: T::AccountId = account("origin_middle", 0, 0); // the node we rebag (_R_)
assert_ok!(List::<T>::insert(origin_middle.clone(), origin_bag_thresh));
assert_ok!(List::<T, _>::insert(origin_middle.clone(), origin_bag_thresh));

let origin_tail: T::AccountId = account("origin_tail", 0, 0);
assert_ok!(List::<T>::insert(origin_tail.clone(), origin_bag_thresh));
assert_ok!(List::<T, _>::insert(origin_tail.clone(), origin_bag_thresh));

// seed items in the destination bag.
let dest_head: T::AccountId = account("dest_head", 0, 0);
assert_ok!(List::<T>::insert(dest_head.clone(), dest_bag_thresh));
assert_ok!(List::<T, _>::insert(dest_head.clone(), dest_bag_thresh));

// the bags are in the expected state after initial setup.
assert_eq!(
List::<T>::get_bags(),
List::<T, _>::get_bags(),
vec![
(origin_bag_thresh, vec![origin_head.clone(), origin_middle.clone(), origin_tail.clone()]),
(dest_bag_thresh, vec![dest_head.clone()])
Expand All @@ -67,12 +68,12 @@ frame_benchmarking::benchmarks! {

let caller = whitelisted_caller();
// update the weight of `origin_middle` to guarantee it will be rebagged into the destination.
T::VoteWeightProvider::set_vote_weight_of(&origin_middle, dest_bag_thresh);
T::ScoreProvider::set_score_of(&origin_middle, dest_bag_thresh);
}: rebag(SystemOrigin::Signed(caller), origin_middle.clone())
verify {
// check the bags have updated as expected.
assert_eq!(
List::<T>::get_bags(),
List::<T, _>::get_bags(),
vec![
(
origin_bag_thresh,
Expand Down Expand Up @@ -104,18 +105,18 @@ frame_benchmarking::benchmarks! {

// seed items in the origin bag.
let origin_head: T::AccountId = account("origin_head", 0, 0);
assert_ok!(List::<T>::insert(origin_head.clone(), origin_bag_thresh));
assert_ok!(List::<T, _>::insert(origin_head.clone(), origin_bag_thresh));

let origin_tail: T::AccountId = account("origin_tail", 0, 0); // the node we rebag (_R_)
assert_ok!(List::<T>::insert(origin_tail.clone(), origin_bag_thresh));
assert_ok!(List::<T, _>::insert(origin_tail.clone(), origin_bag_thresh));

// seed items in the destination bag.
let dest_head: T::AccountId = account("dest_head", 0, 0);
assert_ok!(List::<T>::insert(dest_head.clone(), dest_bag_thresh));
assert_ok!(List::<T, _>::insert(dest_head.clone(), dest_bag_thresh));

// the bags are in the expected state after initial setup.
assert_eq!(
List::<T>::get_bags(),
List::<T, _>::get_bags(),
vec![
(origin_bag_thresh, vec![origin_head.clone(), origin_tail.clone()]),
(dest_bag_thresh, vec![dest_head.clone()])
Expand All @@ -124,12 +125,12 @@ frame_benchmarking::benchmarks! {

let caller = whitelisted_caller();
// update the weight of `origin_tail` to guarantee it will be rebagged into the destination.
T::VoteWeightProvider::set_vote_weight_of(&origin_tail, dest_bag_thresh);
T::ScoreProvider::set_score_of(&origin_tail, dest_bag_thresh);
}: rebag(SystemOrigin::Signed(caller), origin_tail.clone())
verify {
// check the bags have updated as expected.
assert_eq!(
List::<T>::get_bags(),
List::<T, _>::get_bags(),
vec![
(origin_bag_thresh, vec![origin_head.clone()]),
(dest_bag_thresh, vec![dest_head.clone(), origin_tail.clone()])
Expand All @@ -147,30 +148,30 @@ frame_benchmarking::benchmarks! {

// insert the nodes in order
let lighter: T::AccountId = account("lighter", 0, 0);
assert_ok!(List::<T>::insert(lighter.clone(), bag_thresh));
assert_ok!(List::<T, _>::insert(lighter.clone(), bag_thresh));

let heavier_prev: T::AccountId = account("heavier_prev", 0, 0);
assert_ok!(List::<T>::insert(heavier_prev.clone(), bag_thresh));
assert_ok!(List::<T, _>::insert(heavier_prev.clone(), bag_thresh));

let heavier: T::AccountId = account("heavier", 0, 0);
assert_ok!(List::<T>::insert(heavier.clone(), bag_thresh));
assert_ok!(List::<T, _>::insert(heavier.clone(), bag_thresh));

let heavier_next: T::AccountId = account("heavier_next", 0, 0);
assert_ok!(List::<T>::insert(heavier_next.clone(), bag_thresh));
assert_ok!(List::<T, _>::insert(heavier_next.clone(), bag_thresh));

T::VoteWeightProvider::set_vote_weight_of(&lighter, bag_thresh - 1);
T::VoteWeightProvider::set_vote_weight_of(&heavier, bag_thresh);
T::ScoreProvider::set_score_of(&lighter, bag_thresh - One::one());
T::ScoreProvider::set_score_of(&heavier, bag_thresh);

assert_eq!(
List::<T>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
List::<T, _>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
vec![lighter.clone(), heavier_prev.clone(), heavier.clone(), heavier_next.clone()]
);

whitelist_account!(heavier);
}: _(SystemOrigin::Signed(heavier.clone()), lighter.clone())
verify {
assert_eq!(
List::<T>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
List::<T, _>::iter().map(|n| n.id().clone()).collect::<Vec<_>>(),
vec![heavier, lighter, heavier_prev, heavier_next]
)
}
Expand Down
Loading

0 comments on commit 2fc095b

Please sign in to comment.