Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove the most recent staking migration #765

Merged
merged 4 commits into from
Aug 31, 2021
Merged

Remove the most recent staking migration #765

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4479548
remove staking migration to reset state consistency
4meta5 Aug 31, 2021
925bac7
kill storage item instead
4meta5 Aug 31, 2021
10a1f44
fix
4meta5 Aug 31, 2021
42e0bc6
remove storage item without killing what it stores, need to do this t…
4meta5 Aug 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 2 additions & 128 deletions pallets/parachain-staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,118 +861,13 @@ pub mod pallet {
Perbill,
Perbill,
),
/// Account, Amount Unreserved by Democracy
HotfixUnreservedNomination(T::AccountId, BalanceOf<T>),
}

pub(crate) fn correct_bond_less_removes_bottom_nomination_inconsistencies<T: Config>(
) -> (u64, u64) {
let (mut reads, mut writes) = (0u64, 0u64);
let mut map: BTreeMap<(T::AccountId, T::AccountId), ()> = BTreeMap::new();
let top_n = T::MaxNominatorsPerCollator::get() as usize;
// 1. for collator state, check if there is a nominator not in top or bottom
for (account, state) in <CollatorState2<T>>::iter() {
reads += 1u64;
// remove all accounts not in self.top_nominators && self.bottom_nominators
let mut nominator_set = Vec::new();
for Bond { owner, .. } in &state.top_nominators {
nominator_set.push(owner.clone());
}
for Bond { owner, .. } in &state.bottom_nominators {
nominator_set.push(owner.clone());
}
for nominator in &state.nominators.0 {
if !nominator_set.contains(nominator) {
// these accounts were removed without being unreserved so we track it
// with this map which will hold the due amount
map.insert((account.clone(), nominator.clone()), ());
}
}
// RESET incorrect storage state
let mut all_nominators = state.top_nominators.clone();
let mut starting_bottom_nominators = state.bottom_nominators.clone();
all_nominators.append(&mut starting_bottom_nominators);
// sort all nominators from greatest to least
all_nominators.sort_unstable_by(|a, b| b.amount.cmp(&a.amount));
// 2. split them into top and bottom using the T::MaxNominatorsPerCollator
let top_nominators: Vec<Bond<T::AccountId, BalanceOf<T>>> =
all_nominators.clone().into_iter().take(top_n).collect();
let bottom_nominators = if all_nominators.len() > top_n {
let rest = all_nominators.len() - top_n;
let bottom: Vec<Bond<T::AccountId, BalanceOf<T>>> = all_nominators
.clone()
.into_iter()
.rev()
.take(rest)
.collect();
bottom
} else {
// empty, all nominations are in top
Vec::new()
};
let (mut total_counted, mut total_backing): (BalanceOf<T>, BalanceOf<T>) =
(state.bond, state.bond);
for Bond { amount, .. } in &top_nominators {
total_counted += *amount;
total_backing += *amount;
}
for Bond { amount, .. } in &bottom_nominators {
total_backing += *amount;
}
// update candidate pool with new total counted if it changed
if state.total_counted != total_counted && state.is_active() {
reads += 1u64;
writes += 1u64;
<Pallet<T>>::update_active(account.clone(), total_counted);
}
let new_state = Collator2 {
nominators: OrderedSet::from(nominator_set),
top_nominators,
bottom_nominators,
total_counted,
total_backing,
..state
};
<CollatorState2<T>>::insert(&account, new_state);
writes += 1u64;
log::warn!("CORRECTED INCONSISTENT COLLATOR STATE FOR {:?}", account);
}
// 2. for nominator state, check if there are nominations that were inadvertently bumped
// -> this allows us to recover the due unreserved balances for cases of (1) that
// did not have the nominator state removed (it does not account for when it was removed)
for (account, mut state) in <NominatorState2<T>>::iter() {
reads += 1u64;
for Bond { owner, .. } in state.nominations.0.clone() {
if map.get(&(owner.clone(), account.clone())).is_some() {
if state.rm_nomination(owner).is_some() {
if state.nominations.0.is_empty() {
<NominatorState2<T>>::remove(&account);
} else {
<NominatorState2<T>>::insert(&account, state.clone());
}
writes += 1u64;
}
}
}
}
(reads, writes)
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_runtime_upgrade() -> Weight {
let weight = T::DbWeight::get();
if !<FixBondLessMigrationExecuted<T>>::get() {
let (mut reads, mut writes) =
correct_bond_less_removes_bottom_nomination_inconsistencies::<T>();
reads += 1u64;
writes += 1u64;
<FixBondLessMigrationExecuted<T>>::put(true);
// 50% of the max block weight as safety margin for computation
weight.reads(reads) + weight.writes(writes) + 250_000_000_000
} else {
weight.reads(1u64)
}
<FixBondLessMigrationExecuted<T>>::kill();
T::DbWeight::get().writes(1u64)
}
fn on_initialize(n: T::BlockNumber) -> Weight {
let mut round = <Round<T>>::get();
Expand Down Expand Up @@ -1223,27 +1118,6 @@ pub mod pallet {

#[pallet::call]
impl<T: Config> Pallet<T> {
#[pallet::weight(1_000_000_000)]
/// Temporary root function to return nominations
/// - charges uniform 0.2% of block weight as fee per call
pub fn hotfix_unreserve_nomination(
origin: OriginFor<T>,
stakers: Vec<(T::AccountId, BalanceOf<T>)>,
) -> DispatchResultWithPostInfo {
frame_system::ensure_root(origin)?;
let mut sum_unreserved: BalanceOf<T> = 0u32.into();
for (due_account, due_unreserve) in stakers {
T::Currency::unreserve(&due_account, due_unreserve);
Self::deposit_event(Event::HotfixUnreservedNomination(
due_account,
due_unreserve,
));
sum_unreserved += due_unreserve;
}
let new_total = <Total<T>>::get().saturating_sub(sum_unreserved);
<Total<T>>::put(new_total);
Ok(().into())
}
/// Set the expectations for total staked. These expectations determine the issuance for
/// the round according to logic in `fn compute_issuance`
#[pallet::weight(<T as Config>::WeightInfo::set_staking_expectations())]
Expand Down
113 changes: 0 additions & 113 deletions pallets/parachain-staking/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3831,116 +3831,3 @@ fn nomination_events_convey_correct_position() {
);
});
}

#[test]
fn migration_corrects_storage_corrupted_by_bond_less_bug() {
ExtBuilder::default()
.with_balances(vec![
(1, 100),
(2, 100),
(3, 100),
(4, 100),
(5, 100),
(6, 100),
])
.with_candidates(vec![(1, 20)])
.with_nominations(vec![
(2, 1, 19),
(3, 1, 20),
(4, 1, 21),
(5, 1, 22),
(6, 1, 23),
])
.build()
.execute_with(|| {
// start by corrupting collator state like the bug -- basically every `nominator_bond_less`
// call would bump the highest nomination in the bottom nominations without replacing it,
// thereby preventing proper exit for these nominators (unreserve)
let mut candidate_state =
<CollatorState2<Test>>::get(&1).expect("set up 1 as candidate");
candidate_state.bottom_nominators = Vec::new();
// corrupt storage like the bug instance
<CollatorState2<Test>>::insert(1, candidate_state);
// function called in executed nomination/nominator exits
assert_noop!(
Stake::nominator_leaves_collator(2, 1),
Error::<Test>::NominatorDNEinTopNorBottom
);
assert_eq!(Stake::total(), 125);
let candidate_state = <CollatorState2<Test>>::get(&1).expect("still exists");
// was removed from bottom nominators and not the `nominators` set
assert_eq!(
candidate_state.nominators.0.len() - 1,
candidate_state.top_nominators.len() + candidate_state.bottom_nominators.len()
);
// nominator state still has the nomination if the collator hasn't left and the nominator
// didn't exit entirely (this wasn't their last nomination)
assert!(Stake::is_nominator(&2));
// make storage consistent at least
crate::correct_bond_less_removes_bottom_nomination_inconsistencies::<Test>();
// check storage is consistent
let candidate_state = <CollatorState2<Test>>::get(&1).expect("still exists");
assert_eq!(
candidate_state.nominators.0.len(),
candidate_state.top_nominators.len() + candidate_state.bottom_nominators.len()
);
// no longer a nominator
assert!(!Stake::is_nominator(&2));
// check that the balance is not unreserved
assert_eq!(Balances::reserved_balance(&2), 19);
assert_eq!(Balances::free_balance(&2), 81);
assert_eq!(Stake::total(), 125);
// return due unreserved balance (NOTE: doesn't reset this storage item)
assert_ok!(Stake::hotfix_unreserve_nomination(
Origin::root(),
vec![(2, 19)]
));
assert_eq!(Balances::reserved_balance(&2), 0);
assert_eq!(Balances::free_balance(&2), 100);
assert_eq!(Stake::total(), 106);
});
}

#[test]
fn migration_corrects_storage_corrupted_by_max_nominators_upgrade_bug() {
ExtBuilder::default()
.with_balances(vec![
(1, 100),
(2, 100),
(3, 100),
(4, 100),
(5, 100),
(6, 100),
(7, 100),
(8, 100),
])
.with_candidates(vec![(1, 20)])
.with_nominations(vec![
(2, 1, 19),
(3, 1, 20),
(4, 1, 21),
(5, 1, 22),
(6, 1, 23),
(7, 1, 24),
(8, 1, 25),
])
.build()
.execute_with(|| {
// start by corrupting collator state like the bug -- have some in bottom with open
// slots in the top
let mut candidate_state =
<CollatorState2<Test>>::get(&1).expect("set up 1 as candidate");
// corrupt storage via unhandled pop
candidate_state.top_nominators.pop();
candidate_state.top_nominators.pop();
assert_eq!(candidate_state.top_nominators.len(), 2); // < MaxNominatorsPerCollator = 4
assert_eq!(candidate_state.bottom_nominators.len(), 3);
<CollatorState2<Test>>::insert(&1, candidate_state);
// full migration, first cleans nominator set and second cleans other items
crate::correct_bond_less_removes_bottom_nomination_inconsistencies::<Test>();
let post_candidate_state =
<CollatorState2<Test>>::get(&1).expect("set up 1 as candidate");
assert_eq!(post_candidate_state.top_nominators.len(), 4);
assert_eq!(post_candidate_state.bottom_nominators.len(), 1);
});
}