introduce remote-tests for pallet-bags-list (#10036)

* make a few primitives in bags-list public

* push new stuff

* update

* update log target

* bring remote tests here

* revert pub

* Update frame/bags-list/remote-tests/Cargo.toml

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* some rev commnets

* Fix

* cleanup

* Update Cargo.lock

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

substrate/Cargo.lock

@@ -5199,6 +5199,27 @@ dependencies = [
  "rand 0.8.4",
 ]
 
+[[package]]
+name = "pallet-bags-list-remote-tests"
+version = "4.0.0-dev"
+dependencies = [
+ "clap",
+ "frame-election-provider-support",
+ "frame-support",
+ "frame-system",
+ "log 0.4.14",
+ "pallet-bags-list",
+ "pallet-staking",
+ "remote-externalities",
+ "sp-core",
+ "sp-runtime",
+ "sp-std",
+ "sp-storage",
+ "sp-tracing",
+ "structopt",
+ "tokio",
+]
+
 [[package]]
 name = "pallet-balances"
 version = "4.0.0-dev"
@@ -9858,9 +9879,9 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623"
 
 [[package]]
 name = "structopt"
-version = "0.3.21"
+version = "0.3.23"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5277acd7ee46e63e5168a80734c9f6ee81b1367a7d8772a2d765df2a3705d28c"
+checksum = "bf9d950ef167e25e0bdb073cf1d68e9ad2795ac826f2f3f59647817cf23c0bfa"
 dependencies = [
  "clap",
  "lazy_static",
@@ -9869,9 +9890,9 @@ dependencies = [
 
 [[package]]
 name = "structopt-derive"
-version = "0.4.14"
+version = "0.4.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5ba9cdfda491b814720b6b06e0cac513d922fc407582032e8706e9f137976f90"
+checksum = "134d838a2c9943ac3125cf6df165eda53493451b719f3255b2a26b85f772d0ba"
 dependencies = [
  "heck",
  "proc-macro-error 1.0.4",

substrate/Cargo.toml

@@ -135,6 +135,7 @@ members = [
 	"frame/utility",
 	"frame/vesting",
 	"frame/bags-list",
+	"frame/bags-list/remote-tests",
 	"frame/bags-list/fuzzer",
 	"primitives/api",
 	"primitives/api/proc-macro",

substrate/frame/bags-list/remote-tests/Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+name = "pallet-bags-list-remote-tests"
+version = "4.0.0-dev"
+authors = ["Parity Technologies <admin@parity.io>"]
+edition = "2018"
+license = "Apache-2.0"
+homepage = "https://substrate.io"
+repository = "https://github.com/paritytech/substrate/"
+description = "FRAME pallet bags list remote test"
+readme = "README.md"
+
+[package.metadata.docs.rs]
+targets = ["x86_64-unknown-linux-gnu"]
+
+[dependencies]
+# frame
+pallet-staking = { path = "../../staking" }
+pallet-bags-list = { path = "../../bags-list" }
+frame-election-provider-support = { path = "../../election-provider-support" }
+frame-system = { path = "../../system" }
+frame-support = { path = "../../support" }
+
+# core
+sp-storage = { path = "../../../primitives/storage" }
+sp-core = { path = "../../../primitives/core" }
+sp-tracing = { path = "../../../primitives/tracing" }
+sp-runtime = { path = "../../../primitives/runtime" }
+sp-std = { path = "../../../primitives/std" }
+
+# utils
+remote-externalities = { path = "../../../utils/frame/remote-externalities" }
+
+# others
+tokio = { version = "1", features = ["macros"] }
+log = "0.4.14"
+structopt = "0.3.23"
+clap = "2.33.3"

substrate/frame/bags-list/remote-tests/src/lib.rs

@@ -0,0 +1,134 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2021 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Utilities for remote-testing pallet-bags-list.
+
+use sp_std::convert::TryInto;
+
+/// A common log target to use.
+pub const LOG_TARGET: &'static str = "runtime::bags-list::remote-tests";
+
+pub mod migration;
+pub mod sanity_check;
+pub mod snapshot;
+
+/// A wrapper for a runtime that the functions of this crate expect.
+///
+/// For example, this can be the `Runtime` type of the Polkadot runtime.
+pub trait RuntimeT:
+	pallet_staking::Config + pallet_bags_list::Config + frame_system::Config
+{
+}
+impl<T: pallet_staking::Config + pallet_bags_list::Config + frame_system::Config> RuntimeT for T {}
+
+fn percent(portion: u32, total: u32) -> f64 {
+	(portion as f64 / total as f64) * 100f64
+}
+
+/// Display the number of nodes in each bag, while identifying those that need a rebag.
+pub fn display_and_check_bags<Runtime: RuntimeT>(currency_unit: u64, currency_name: &'static str) {
+	use frame_election_provider_support::SortedListProvider;
+	use frame_support::traits::Get;
+
+	let min_nominator_bond = <pallet_staking::MinNominatorBond<Runtime>>::get();
+	log::info!(target: LOG_TARGET, "min nominator bond is {:?}", min_nominator_bond);
+
+	let voter_list_count = <Runtime as pallet_staking::Config>::SortedListProvider::count();
+
+	// go through every bag to track the total number of voters within bags and log some info about
+	// how voters are distributed within the bags.
+	let mut seen_in_bags = 0;
+	let mut rebaggable = 0;
+	let mut active_bags = 0;
+	for vote_weight_thresh in <Runtime as pallet_bags_list::Config>::BagThresholds::get() {
+		// 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 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) {
+			Some(bag) => bag,
+			None => {
+				log::info!(target: LOG_TARGET, "{} NO VOTERS.", pretty_thresh);
+				continue
+			},
+		};
+
+		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_as_balance: pallet_staking::BalanceOf<Runtime> =
+				vote_weight.try_into().map_err(|_| "can't convert").unwrap();
+
+			if vote_weight_as_balance < min_nominator_bond {
+				log::trace!(
+					target: LOG_TARGET,
+					"⚠️ {} Account found below min bond: {:?}.",
+					pretty_thresh,
+					id
+				);
+			}
+
+			let node =
+				pallet_bags_list::Node::<Runtime>::get(&id).expect("node in bag must exist.");
+			if node.is_misplaced(vote_weight) {
+				rebaggable += 1;
+				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
+				);
+			}
+		}
+
+		// update our overall counter
+		let voters_in_bag = bag.std_iter().count() as u32;
+		seen_in_bags += voters_in_bag;
+
+		// percentage of all nominators
+		let percent_of_voters = percent(voters_in_bag, voter_list_count);
+
+		log::info!(
+			target: LOG_TARGET,
+			"{} Nominators: {} [%{:.3}]",
+			pretty_thresh,
+			voters_in_bag,
+			percent_of_voters,
+		);
+	}
+
+	if seen_in_bags != voter_list_count {
+		log::error!(
+			target: LOG_TARGET,
+			"bags list population ({}) not on par whoever is voter_list ({})",
+			seen_in_bags,
+			voter_list_count,
+		)
+	}
+
+	log::info!(
+		target: LOG_TARGET,
+		"a total of {} nodes are in {} active bags [{} total bags], {} of which can be rebagged.",
+		voter_list_count,
+		active_bags,
+		<Runtime as pallet_bags_list::Config>::BagThresholds::get().len(),
+		rebaggable,
+	);
+}

substrate/frame/bags-list/remote-tests/src/migration.rs

@@ -0,0 +1,65 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Test to check the migration of the voter bag.
+
+use crate::{RuntimeT, LOG_TARGET};
+use frame_election_provider_support::SortedListProvider;
+use frame_support::traits::PalletInfoAccess;
+use pallet_staking::Nominators;
+use remote_externalities::{Builder, Mode, OnlineConfig};
+use sp_runtime::traits::Block as BlockT;
+
+/// Test voter bags migration. `currency_unit` is the number of planks per the the runtimes `UNITS`
+/// (i.e. number of decimal places per DOT, KSM etc)
+pub async fn execute<Runtime: RuntimeT, Block: BlockT>(
+	currency_unit: u64,
+	currency_name: &'static str,
+	ws_url: String,
+) {
+	let mut ext = Builder::<Block>::new()
+		.mode(Mode::Online(OnlineConfig {
+			transport: ws_url.to_string().into(),
+			pallets: vec![pallet_staking::Pallet::<Runtime>::name().to_string()],
+			at: None,
+			state_snapshot: None,
+		}))
+		.build()
+		.await
+		.unwrap();
+
+	ext.execute_with(|| {
+		// get the nominator & validator count prior to migrating; these should be invariant.
+		let pre_migrate_nominator_count = <Nominators<Runtime>>::iter().count() as u32;
+		log::info!(target: LOG_TARGET, "Nominator count: {}", pre_migrate_nominator_count);
+
+		// run the actual migration,
+		let moved = <Runtime as pallet_staking::Config>::SortedListProvider::regenerate(
+			pallet_staking::Nominators::<Runtime>::iter().map(|(n, _)| n),
+			pallet_staking::Pallet::<Runtime>::weight_of_fn(),
+		);
+		log::info!(target: LOG_TARGET, "Moved {} nominators", moved);
+
+		let voter_list_len =
+			<Runtime as pallet_staking::Config>::SortedListProvider::iter().count() as u32;
+		let voter_list_count = <Runtime as pallet_staking::Config>::SortedListProvider::count();
+		// and confirm it is equal to the length of the `VoterList`.
+		assert_eq!(pre_migrate_nominator_count, voter_list_len);
+		assert_eq!(pre_migrate_nominator_count, voter_list_count);
+
+		crate::display_and_check_bags::<Runtime>(currency_unit, currency_name);
+	});
+}

substrate/frame/bags-list/remote-tests/src/sanity_check.rs

@@ -0,0 +1,54 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Test to execute the sanity-check of the voter bag.
+
+use frame_election_provider_support::SortedListProvider;
+use frame_support::{
+	storage::generator::StorageMap,
+	traits::{Get, PalletInfoAccess},
+};
+use remote_externalities::{Builder, Mode, OnlineConfig};
+use sp_runtime::traits::Block as BlockT;
+use sp_std::convert::TryInto;
+
+/// Execute the sanity check of the bags-list.
+pub async fn execute<Runtime: crate::RuntimeT, Block: BlockT>(
+	currency_unit: u64,
+	currency_name: &'static str,
+	ws_url: String,
+) {
+	let mut ext = Builder::<Block>::new()
+		.mode(Mode::Online(OnlineConfig {
+			transport: ws_url.to_string().into(),
+			pallets: vec![pallet_bags_list::Pallet::<Runtime>::name().to_string()],
+			at: None,
+			state_snapshot: None,
+		}))
+		.inject_hashed_prefix(&<pallet_staking::Bonded<Runtime>>::prefix_hash())
+		.inject_hashed_prefix(&<pallet_staking::Ledger<Runtime>>::prefix_hash())
+		.build()
+		.await
+		.unwrap();
+
+	ext.execute_with(|| {
+		sp_core::crypto::set_default_ss58_version(Runtime::SS58Prefix::get().try_into().unwrap());
+		pallet_bags_list::Pallet::<Runtime>::sanity_check().unwrap();
+		log::info!(target: crate::LOG_TARGET, "executed bags-list sanity check with no errors.");
+
+		crate::display_and_check_bags::<Runtime>(currency_unit, currency_name);
+	});
+}

substrate/frame/bags-list/remote-tests/src/snapshot.rs

@@ -0,0 +1,86 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Test to execute the snapshot using the voter bag.
+
+use frame_support::traits::PalletInfoAccess;
+use remote_externalities::{Builder, Mode, OnlineConfig};
+use sp_runtime::traits::Block as BlockT;
+
+/// Execute create a snapshot from pallet-staking.
+pub async fn execute<Runtime: crate::RuntimeT, Block: BlockT>(
+	voter_limit: Option<usize>,
+	currency_unit: u64,
+	ws_url: String,
+) {
+	use frame_support::storage::generator::StorageMap;
+	let mut ext = Builder::<Block>::new()
+		.mode(Mode::Online(OnlineConfig {
+			transport: ws_url.to_string().into(),
+			// NOTE: we don't scrape pallet-staking, this kinda ensures that the source of the data
+			// is bags-list.
+			pallets: vec![pallet_bags_list::Pallet::<Runtime>::name().to_string()],
+			at: None,
+			state_snapshot: None,
+		}))
+		.inject_hashed_prefix(&<pallet_staking::Bonded<Runtime>>::prefix_hash())
+		.inject_hashed_prefix(&<pallet_staking::Ledger<Runtime>>::prefix_hash())
+		.inject_hashed_prefix(&<pallet_staking::Validators<Runtime>>::prefix_hash())
+		.inject_hashed_prefix(&<pallet_staking::Nominators<Runtime>>::prefix_hash())
+		.inject_hashed_key(&<pallet_staking::CounterForNominators<Runtime>>::hashed_key())
+		.inject_hashed_key(&<pallet_staking::CounterForValidators<Runtime>>::hashed_key())
+		.build()
+		.await
+		.unwrap();
+
+	ext.execute_with(|| {
+		use frame_election_provider_support::{ElectionDataProvider, SortedListProvider};
+		log::info!(
+			target: crate::LOG_TARGET,
+			"{} nodes in bags list.",
+			<Runtime as pallet_staking::Config>::SortedListProvider::count(),
+		);
+
+		let voters = <pallet_staking::Pallet<Runtime> as ElectionDataProvider<
+			Runtime::AccountId,
+			Runtime::BlockNumber,
+		>>::voters(voter_limit)
+		.unwrap();
+
+		let mut voters_nominator_only = voters
+			.iter()
+			.filter(|(v, _, _)| pallet_staking::Nominators::<Runtime>::contains_key(v))
+			.cloned()
+			.collect::<Vec<_>>();
+		voters_nominator_only.sort_by_key(|(_, w, _)| *w);
+
+		let currency_unit = currency_unit as f64;
+		let min_voter = voters_nominator_only
+			.first()
+			.map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit));
+		let max_voter = voters_nominator_only
+			.last()
+			.map(|(x, y, _)| (x.clone(), *y as f64 / currency_unit));
+		log::info!(
+			target: crate::LOG_TARGET,
+			"a snapshot with limit {:?} has been created, {} voters are taken. min nominator: {:?}, max: {:?}",
+			voter_limit,
+			voters.len(),
+			min_voter,
+			max_voter
+		);
+	});
+}

substrate/frame/bags-list/src/lib.rs

@@ -65,12 +65,10 @@ pub mod mock;
 mod tests;
 pub mod weights;
 
+pub use list::{notional_bag_for, Bag, Error, List, Node};
 pub use pallet::*;
 pub use weights::WeightInfo;
 
-pub use list::Error;
-use list::List;
-
 pub(crate) const LOG_TARGET: &'static str = "runtime::bags_list";
 
 // syntactic sugar for logging.
@@ -155,7 +153,7 @@ pub mod pallet {
 
 	/// How many ids are registered.
 	// NOTE: This is merely a counter for `ListNodes`. It should someday be replaced by the
-	// `CountedMaop` storage.
+	// `CountedMap` storage.
 	#[pallet::storage]
 	pub(crate) type CounterForListNodes<T> = StorageValue<_, u32, ValueQuery>;
 

substrate/frame/bags-list/src/list/mod.rs

@@ -53,7 +53,7 @@ mod tests;
 ///
 /// Note that even if the thresholds list does not have `VoteWeight::MAX` as its final member, this
 /// function behaves as if it does.
-pub(crate) fn notional_bag_for<T: Config>(weight: VoteWeight) -> VoteWeight {
+pub fn notional_bag_for<T: Config>(weight: VoteWeight) -> VoteWeight {
 	let thresholds = T::BagThresholds::get();
 	let idx = thresholds.partition_point(|&threshold| weight > threshold);
 	thresholds.get(idx).copied().unwrap_or(VoteWeight::MAX)
@@ -690,7 +690,7 @@ pub struct Node<T: Config> {
 
 impl<T: Config> Node<T> {
 	/// Get a node by id.
-	pub(crate) fn get(id: &T::AccountId) -> Option<Node<T>> {
+	pub fn get(id: &T::AccountId) -> Option<Node<T>> {
 		crate::ListNodes::<T>::try_get(id).ok()
 	}
 
@@ -734,7 +734,7 @@ impl<T: Config> Node<T> {
 	}
 
 	/// `true` when this voter is in the wrong bag.
-	pub(crate) fn is_misplaced(&self, current_weight: VoteWeight) -> bool {
+	pub fn is_misplaced(&self, current_weight: VoteWeight) -> bool {
 		notional_bag_for::<T>(current_weight) != self.bag_upper
 	}
 

substrate/frame/staking/src/pallet/impls.rs

@@ -692,6 +692,9 @@ impl<T: Config> Pallet<T> {
 		// track every nominator iterated over, but not necessarily added to `all_voters`
 		let mut nominators_seen = 0u32;
 
+		// cache the total-issuance once in this function
+		let weight_of = Self::weight_of_fn();
+
 		let mut nominators_iter = T::SortedListProvider::iter();
 		while nominators_taken < nominators_quota && nominators_seen < nominators_quota * 2 {
 			let nominator = match nominators_iter.next() {
@@ -705,17 +708,23 @@ impl<T: Config> Pallet<T> {
 			if let Some(Nominations { submitted_in, mut targets, suppressed: _ }) =
 				<Nominators<T>>::get(&nominator)
 			{
+				log!(
+					trace,
+					"fetched nominator {:?} with weight {:?}",
+					nominator,
+					weight_of(&nominator)
+				);
 				targets.retain(|stash| {
 					slashing_spans
 						.get(stash)
 						.map_or(true, |spans| submitted_in >= spans.last_nonzero_slash())
 				});
 				if !targets.len().is_zero() {
-					all_voters.push((nominator.clone(), Self::weight_of(&nominator), targets));
+					all_voters.push((nominator.clone(), weight_of(&nominator), targets));
 					nominators_taken.saturating_inc();
 				}
 			} else {
-				log!(error, "invalid item in `SortedListProvider`: {:?}", nominator)
+				log!(error, "DEFENSIVE: invalid item in `SortedListProvider`: {:?}", nominator)
 			}
 		}
 

substrate/utils/frame/remote-externalities/src/lib.rs

@@ -385,7 +385,7 @@ impl<B: BlockT> Builder<B> {
 		};
 
 		for prefix in &self.hashed_prefixes {
-			debug!(
+			info!(
 				target: LOG_TARGET,
 				"adding data for hashed prefix: {:?}",
 				HexDisplay::from(prefix)
@@ -397,7 +397,7 @@ impl<B: BlockT> Builder<B> {
 
 		for key in &self.hashed_keys {
 			let key = StorageKey(key.to_vec());
-			debug!(target: LOG_TARGET, "adding data for hashed key: {:?}", HexDisplay::from(&key));
+			info!(target: LOG_TARGET, "adding data for hashed key: {:?}", HexDisplay::from(&key));
 			let value = self.rpc_get_storage(key.clone(), Some(at)).await?;
 			keys_and_values.push((key, value));
 		}