diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 4774bfda1f..9686bf426f 100644 --- a/substrate/Cargo.lock +++ b/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", diff --git a/substrate/Cargo.toml b/substrate/Cargo.toml index 743e0f7066..197b156dea 100644 --- a/substrate/Cargo.toml +++ b/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", diff --git a/substrate/frame/bags-list/remote-tests/Cargo.toml b/substrate/frame/bags-list/remote-tests/Cargo.toml new file mode 100644 index 0000000000..c670178c61 --- /dev/null +++ b/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 "] +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" diff --git a/substrate/frame/bags-list/remote-tests/src/lib.rs b/substrate/frame/bags-list/remote-tests/src/lib.rs new file mode 100644 index 0000000000..e471c4c95b --- /dev/null +++ b/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 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(currency_unit: u64, currency_name: &'static str) { + use frame_election_provider_support::SortedListProvider; + use frame_support::traits::Get; + + let min_nominator_bond = >::get(); + log::info!(target: LOG_TARGET, "min nominator bond is {:?}", min_nominator_bond); + + let voter_list_count = ::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 ::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::::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::::weight_of(&id); + let vote_weight_as_balance: pallet_staking::BalanceOf = + 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::::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::(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, + ::BagThresholds::get().len(), + rebaggable, + ); +} diff --git a/substrate/frame/bags-list/remote-tests/src/migration.rs b/substrate/frame/bags-list/remote-tests/src/migration.rs new file mode 100644 index 0000000000..1e977011f1 --- /dev/null +++ b/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 . + +//! 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( + currency_unit: u64, + currency_name: &'static str, + ws_url: String, +) { + let mut ext = Builder::::new() + .mode(Mode::Online(OnlineConfig { + transport: ws_url.to_string().into(), + pallets: vec![pallet_staking::Pallet::::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 = >::iter().count() as u32; + log::info!(target: LOG_TARGET, "Nominator count: {}", pre_migrate_nominator_count); + + // run the actual migration, + let moved = ::SortedListProvider::regenerate( + pallet_staking::Nominators::::iter().map(|(n, _)| n), + pallet_staking::Pallet::::weight_of_fn(), + ); + log::info!(target: LOG_TARGET, "Moved {} nominators", moved); + + let voter_list_len = + ::SortedListProvider::iter().count() as u32; + let voter_list_count = ::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::(currency_unit, currency_name); + }); +} diff --git a/substrate/frame/bags-list/remote-tests/src/sanity_check.rs b/substrate/frame/bags-list/remote-tests/src/sanity_check.rs new file mode 100644 index 0000000000..e5e9f45bac --- /dev/null +++ b/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 . + +//! 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( + currency_unit: u64, + currency_name: &'static str, + ws_url: String, +) { + let mut ext = Builder::::new() + .mode(Mode::Online(OnlineConfig { + transport: ws_url.to_string().into(), + pallets: vec![pallet_bags_list::Pallet::::name().to_string()], + at: None, + state_snapshot: None, + })) + .inject_hashed_prefix(&>::prefix_hash()) + .inject_hashed_prefix(&>::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::::sanity_check().unwrap(); + log::info!(target: crate::LOG_TARGET, "executed bags-list sanity check with no errors."); + + crate::display_and_check_bags::(currency_unit, currency_name); + }); +} diff --git a/substrate/frame/bags-list/remote-tests/src/snapshot.rs b/substrate/frame/bags-list/remote-tests/src/snapshot.rs new file mode 100644 index 0000000000..6e186a65cb --- /dev/null +++ b/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 . + +//! 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( + voter_limit: Option, + currency_unit: u64, + ws_url: String, +) { + use frame_support::storage::generator::StorageMap; + let mut ext = Builder::::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::::name().to_string()], + at: None, + state_snapshot: None, + })) + .inject_hashed_prefix(&>::prefix_hash()) + .inject_hashed_prefix(&>::prefix_hash()) + .inject_hashed_prefix(&>::prefix_hash()) + .inject_hashed_prefix(&>::prefix_hash()) + .inject_hashed_key(&>::hashed_key()) + .inject_hashed_key(&>::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.", + ::SortedListProvider::count(), + ); + + let voters = as ElectionDataProvider< + Runtime::AccountId, + Runtime::BlockNumber, + >>::voters(voter_limit) + .unwrap(); + + let mut voters_nominator_only = voters + .iter() + .filter(|(v, _, _)| pallet_staking::Nominators::::contains_key(v)) + .cloned() + .collect::>(); + 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 + ); + }); +} diff --git a/substrate/frame/bags-list/src/lib.rs b/substrate/frame/bags-list/src/lib.rs index 10a692e8b3..b7f96799e4 100644 --- a/substrate/frame/bags-list/src/lib.rs +++ b/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 = StorageValue<_, u32, ValueQuery>; diff --git a/substrate/frame/bags-list/src/list/mod.rs b/substrate/frame/bags-list/src/list/mod.rs index 057565e645..4efc316381 100644 --- a/substrate/frame/bags-list/src/list/mod.rs +++ b/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(weight: VoteWeight) -> VoteWeight { +pub fn notional_bag_for(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 { impl Node { /// Get a node by id. - pub(crate) fn get(id: &T::AccountId) -> Option> { + pub fn get(id: &T::AccountId) -> Option> { crate::ListNodes::::try_get(id).ok() } @@ -734,7 +734,7 @@ impl Node { } /// `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::(current_weight) != self.bag_upper } diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 02099d8543..ec34efe397 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -692,6 +692,9 @@ impl Pallet { // 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 Pallet { if let Some(Nominations { submitted_in, mut targets, suppressed: _ }) = >::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) } } diff --git a/substrate/utils/frame/remote-externalities/src/lib.rs b/substrate/utils/frame/remote-externalities/src/lib.rs index 733ec7c320..3b9e08f75d 100644 --- a/substrate/utils/frame/remote-externalities/src/lib.rs +++ b/substrate/utils/frame/remote-externalities/src/lib.rs @@ -385,7 +385,7 @@ impl Builder { }; for prefix in &self.hashed_prefixes { - debug!( + info!( target: LOG_TARGET, "adding data for hashed prefix: {:?}", HexDisplay::from(prefix) @@ -397,7 +397,7 @@ impl Builder { 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)); }