Skip to content

Commit

Permalink
Migration for over locked accounts in phgragmen elections (paritytech…
Browse files Browse the repository at this point in the history
…#10649)

* use free balance rather than total balance

* Docs

* Migration for over-locked phrag voters

* New line

* comment

* Update frame/elections-phragmen/src/migrations/v5.rs

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

* Only set lock, don't remove it

* delete commented out

* docs

* Update migration to just take a set of accounts

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
  • Loading branch information
3 people authored and grishasobol committed Mar 28, 2022
1 parent 175a8fb commit 7778501
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
2 changes: 2 additions & 0 deletions frame/elections-phragmen/src/migrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@
pub mod v3;
/// Version 4.
pub mod v4;
/// Version 5.
pub mod v5;
70 changes: 70 additions & 0 deletions frame/elections-phragmen/src/migrations/v5.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
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 = 0;

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");
}

0 comments on commit 7778501

Please sign in to comment.