Abstracts elections-phragmen pallet to use NposSolver (#12588)

* Abstracts elections-phragmen pallet to use NposSolver

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections-phragmen/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* changes the name of the pallet; adds changelog

* update changelog

* Adds weight testing

* Adds log macro_rules

* renames elections-phragment dir to elections

* weights rename

* fixes typo in cargo toml

* pre/post solve weight scafolding

* refactor do_post_election

* refactors into pre and post election solve for independent benchmarking

* deconstructs PreElectionResults struct

* updates benchmarking pre and post election solve; mock weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* addresses PR comments

* adds pre_solve and post_sove weights

* Adds comments on election pallet id param name change

* ".git/.scripts/bench-bot.sh" pallet dev pallet_elections

* Finishes pre-post solve weights

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/elections/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Addresses PR comments: no panic in on_init path; nits

* Fixes node build

* Implements approval voting to use as a `NposSolver` (#13367)

* Implements the approval voting methods in sp_npos_elections

* fmt

* remove unecessary file

* comment clarification

* re-run weights

* fix typo

* updates MaxVoters in tests for integrity_tests to pass

* Refactors election provider support benchmarks outside its own crate (#13431)

* Refactors election provider support benchmarks outside its own crate
---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Ross Bulat <ross@parity.io>
This commit is contained in:
Gonçalo Pestana
2023-02-23 11:21:00 +00:00
committed by GitHub
parent 17e055e594
commit b793666ca5
28 changed files with 1197 additions and 1018 deletions
@@ -1,25 +0,0 @@
// This file is part of Substrate.
// Copyright (C) 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.
//! All migrations of this pallet.
/// Version 3.
pub mod v3;
/// Version 4.
pub mod v4;
/// Version 5.
pub mod v5;
@@ -1,165 +0,0 @@
// This file is part of Substrate.
// Copyright (C) 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.
//! Migrations to version [`3.0.0`], as denoted by the changelog.
use super::super::LOG_TARGET;
use crate::{Config, Pallet};
use codec::{Decode, Encode, FullCodec};
use frame_support::{
pallet_prelude::ValueQuery, traits::StorageVersion, weights::Weight, RuntimeDebug, Twox64Concat,
};
use sp_std::prelude::*;
#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)]
struct SeatHolder<AccountId, Balance> {
who: AccountId,
stake: Balance,
deposit: Balance,
}
#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)]
struct Voter<AccountId, Balance> {
votes: Vec<AccountId>,
stake: Balance,
deposit: Balance,
}
/// Trait to implement to give information about types used for migration
pub trait V2ToV3 {
/// System config account id
type AccountId: 'static + FullCodec;
/// Elections-phragmen currency balance.
type Balance: 'static + FullCodec + Copy;
}
#[frame_support::storage_alias]
type Candidates<V, T: Config> =
StorageValue<Pallet<T>, Vec<(<V as V2ToV3>::AccountId, <V as V2ToV3>::Balance)>, ValueQuery>;
#[frame_support::storage_alias]
type Members<V, T: Config> = StorageValue<
Pallet<T>,
Vec<SeatHolder<<V as V2ToV3>::AccountId, <V as V2ToV3>::Balance>>,
ValueQuery,
>;
#[frame_support::storage_alias]
type RunnersUp<V, T: Config> = StorageValue<
Pallet<T>,
Vec<SeatHolder<<V as V2ToV3>::AccountId, <V as V2ToV3>::Balance>>,
ValueQuery,
>;
#[frame_support::storage_alias]
type Voting<V, T: Config> = StorageMap<
Pallet<T>,
Twox64Concat,
<V as V2ToV3>::AccountId,
Voter<<V as V2ToV3>::AccountId, <V as V2ToV3>::Balance>,
>;
/// Apply all of the migrations from 2 to 3.
///
/// ### Warning
///
/// This code will **ONLY** check that the storage version is less than or equal to 2_0_0.
/// Further check might be needed at the user runtime.
///
/// Be aware that this migration is intended to be used only for the mentioned versions. Use
/// with care and run at your own risk.
pub fn apply<V: V2ToV3, T: Config>(
old_voter_bond: V::Balance,
old_candidacy_bond: V::Balance,
) -> Weight {
let storage_version = StorageVersion::get::<Pallet<T>>();
log::info!(
target: LOG_TARGET,
"Running migration for elections-phragmen with storage version {:?}",
storage_version,
);
if storage_version <= 2 {
migrate_voters_to_recorded_deposit::<V, T>(old_voter_bond);
migrate_candidates_to_recorded_deposit::<V, T>(old_candidacy_bond);
migrate_runners_up_to_recorded_deposit::<V, T>(old_candidacy_bond);
migrate_members_to_recorded_deposit::<V, T>(old_candidacy_bond);
StorageVersion::new(3).put::<Pallet<T>>();
Weight::MAX
} else {
log::warn!(
target: LOG_TARGET,
"Attempted to apply migration to V3 but failed because storage version is {:?}",
storage_version,
);
Weight::zero()
}
}
/// Migrate from the old legacy voting bond (fixed) to the new one (per-vote dynamic).
pub fn migrate_voters_to_recorded_deposit<V: V2ToV3, T: Config>(old_deposit: V::Balance) {
<Voting<V, T>>::translate::<(V::Balance, Vec<V::AccountId>), _>(|_who, (stake, votes)| {
Some(Voter { votes, stake, deposit: old_deposit })
});
log::info!(target: LOG_TARGET, "migrated {} voter accounts.", <Voting<V, T>>::iter().count());
}
/// Migrate all candidates to recorded deposit.
pub fn migrate_candidates_to_recorded_deposit<V: V2ToV3, T: Config>(old_deposit: V::Balance) {
let _ = <Candidates<V, T>>::translate::<Vec<V::AccountId>, _>(|maybe_old_candidates| {
maybe_old_candidates.map(|old_candidates| {
log::info!(target: LOG_TARGET, "migrated {} candidate accounts.", old_candidates.len());
old_candidates.into_iter().map(|c| (c, old_deposit)).collect::<Vec<_>>()
})
});
}
/// Migrate all members to recorded deposit.
pub fn migrate_members_to_recorded_deposit<V: V2ToV3, T: Config>(old_deposit: V::Balance) {
let _ = <Members<V, T>>::translate::<Vec<(V::AccountId, V::Balance)>, _>(|maybe_old_members| {
maybe_old_members.map(|old_members| {
log::info!(target: LOG_TARGET, "migrated {} member accounts.", old_members.len());
old_members
.into_iter()
.map(|(who, stake)| SeatHolder { who, stake, deposit: old_deposit })
.collect::<Vec<_>>()
})
});
}
/// Migrate all runners-up to recorded deposit.
pub fn migrate_runners_up_to_recorded_deposit<V: V2ToV3, T: Config>(old_deposit: V::Balance) {
let _ = <RunnersUp<V, T>>::translate::<Vec<(V::AccountId, V::Balance)>, _>(
|maybe_old_runners_up| {
maybe_old_runners_up.map(|old_runners_up| {
log::info!(
target: LOG_TARGET,
"migrated {} runner-up accounts.",
old_runners_up.len(),
);
old_runners_up
.into_iter()
.map(|(who, stake)| SeatHolder { who, stake, deposit: old_deposit })
.collect::<Vec<_>>()
})
},
);
}
@@ -1,107 +0,0 @@
// This file is part of Substrate.
// Copyright (C) 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.
//! Migrations to version [`4.0.0`], as denoted by the changelog.
use super::super::LOG_TARGET;
use frame_support::{
traits::{Get, StorageVersion},
weights::Weight,
};
/// The old prefix.
pub const OLD_PREFIX: &[u8] = b"PhragmenElection";
/// Migrate the entire storage of this pallet to a new prefix.
///
/// This new prefix must be the same as the one set in construct_runtime. For safety, use
/// `PalletInfo` to get it, as:
/// `<Runtime as frame_system::Config>::PalletInfo::name::<ElectionsPhragmenPallet>`.
///
/// The old storage prefix, `PhragmenElection` is hardcoded in the migration code.
pub fn migrate<T: crate::Config, N: AsRef<str>>(new_pallet_name: N) -> Weight {
if new_pallet_name.as_ref().as_bytes() == OLD_PREFIX {
log::info!(
target: LOG_TARGET,
"New pallet name is equal to the old prefix. No migration needs to be done.",
);
return Weight::zero()
}
let storage_version = StorageVersion::get::<crate::Pallet<T>>();
log::info!(
target: LOG_TARGET,
"Running migration to v4 for elections-phragmen with storage version {:?}",
storage_version,
);
if storage_version <= 3 {
log::info!("new prefix: {}", new_pallet_name.as_ref());
frame_support::storage::migration::move_pallet(
OLD_PREFIX,
new_pallet_name.as_ref().as_bytes(),
);
StorageVersion::new(4).put::<crate::Pallet<T>>();
<T as frame_system::Config>::BlockWeights::get().max_block
} else {
log::warn!(
target: LOG_TARGET,
"Attempted to apply migration to v4 but failed because storage version is {:?}",
storage_version,
);
Weight::zero()
}
}
/// Some checks prior to migration. This can be linked to
/// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing.
///
/// Panics if anything goes wrong.
pub fn pre_migration<T: crate::Config, N: AsRef<str>>(new: N) {
let new = new.as_ref();
log::info!("pre-migration elections-phragmen test with new = {}", new);
// the next key must exist, and start with the hash of `OLD_PREFIX`.
let next_key = sp_io::storage::next_key(OLD_PREFIX).unwrap();
assert!(next_key.starts_with(&sp_io::hashing::twox_128(OLD_PREFIX)));
// ensure nothing is stored in the new prefix.
assert!(
sp_io::storage::next_key(new.as_bytes()).map_or(
// either nothing is there
true,
// or we ensure that it has no common prefix with twox_128(new).
|next_key| !next_key.starts_with(&sp_io::hashing::twox_128(new.as_bytes()))
),
"unexpected next_key({}) = {:?}",
new,
sp_core::hexdisplay::HexDisplay::from(&sp_io::storage::next_key(new.as_bytes()).unwrap())
);
// ensure storage version is 3.
assert_eq!(StorageVersion::get::<crate::Pallet<T>>(), 3);
}
/// Some checks for after migration. This can be linked to
/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing.
///
/// Panics if anything goes wrong.
pub fn post_migration<T: crate::Config>() {
log::info!("post-migration elections-phragmen");
// ensure we've been updated to v4 by the automatic write of crate version -> storage version.
assert_eq!(StorageVersion::get::<crate::Pallet<T>>(), 4);
}
@@ -1,70 +0,0 @@
use super::super::*;
/// Migrate the locks and vote stake on accounts (as specified with param `to_migrate`) that have
/// more than their free balance locked.
///
/// This migration addresses a bug were a voter could lock up to their reserved balance + free
/// balance. Since locks are only designed to operate on free balance, this put those affected in a
/// situation where they could increase their free balance but still not be able to use their funds
/// because they were less than the lock.
pub fn migrate<T: Config>(to_migrate: Vec<T::AccountId>) -> Weight {
let mut weight = Weight::zero();
for who in to_migrate.iter() {
if let Ok(mut voter) = Voting::<T>::try_get(who) {
let free_balance = T::Currency::free_balance(who);
weight = weight.saturating_add(T::DbWeight::get().reads(2));
if voter.stake > free_balance {
voter.stake = free_balance;
Voting::<T>::insert(&who, voter);
let pallet_id = T::PalletId::get();
T::Currency::set_lock(pallet_id, who, free_balance, WithdrawReasons::all());
weight = weight.saturating_add(T::DbWeight::get().writes(2));
}
}
}
weight
}
/// Given the list of voters to migrate return a function that does some checks and information
/// prior to migration. This can be linked to [`frame_support::traits::OnRuntimeUpgrade::
/// pre_upgrade`] for further testing.
pub fn pre_migrate_fn<T: Config>(to_migrate: Vec<T::AccountId>) -> Box<dyn Fn() -> ()> {
Box::new(move || {
for who in to_migrate.iter() {
if let Ok(voter) = Voting::<T>::try_get(who) {
let free_balance = T::Currency::free_balance(who);
if voter.stake > free_balance {
// all good
} else {
log::warn!("pre-migrate elections-phragmen: voter={:?} has less stake then free balance", who);
}
} else {
log::warn!("pre-migrate elections-phragmen: cannot find voter={:?}", who);
}
}
log::info!("pre-migrate elections-phragmen complete");
})
}
/// Some checks for after migration. This can be linked to
/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing.
///
/// Panics if anything goes wrong.
pub fn post_migrate<T: crate::Config>() {
for (who, voter) in Voting::<T>::iter() {
let free_balance = T::Currency::free_balance(&who);
assert!(voter.stake <= free_balance, "migration should have made locked <= free_balance");
// Ideally we would also check that the locks and AccountData.misc_frozen where correctly
// updated, but since both of those are generic we can't do that without further bounding T.
}
log::info!("post-migrate elections-phragmen complete");
}