Skip to content

Commit

Permalink
[Staking] Patch delegations total mismatch (#1291)
Browse files Browse the repository at this point in the history 
* patch init but will need migration to correct incorrect state

* init migration

* unit test migration logic

* move updating candidate pool up one function to reset top data

* improve try runtime check and add comment to reset top data explaining why we have conditional candidate pool update

* uncomment xcm migrations

* fix try runtime build
  • Loading branch information
@4meta5
4meta5 committed Feb 17, 2022
1 parent 31015aa commit 30b9553
Show file tree
Hide file tree
Showing 4 changed files with 283 additions and 59 deletions.
54 changes: 25 additions & 29 deletions pallets/parachain-staking/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,13 +527,21 @@ pub mod pallet {
/// Reset top delegations metadata
pub fn reset_top_data<T: Config>(
&mut self,
candidate: T::AccountId,
top_delegations: &Delegations<T::AccountId, BalanceOf<T>>,
) where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
self.lowest_top_delegation_amount = top_delegations.lowest_delegation_amount().into();
self.top_capacity = top_delegations.top_capacity::<T>();
let old_total_counted = self.total_counted;
self.total_counted = self.bond + top_delegations.total.into();
// CandidatePool value for candidate always changes if top delegations total changes
// so we moved the update into this function to deduplicate code and patch a bug that
// forgot to apply the update when increasing top delegation
if old_total_counted != self.total_counted && self.is_active() {
Pallet::<T>::update_active(candidate, self.total_counted.into());
}
}
/// Reset bottom delegations metadata
pub fn reset_bottom_data<T: Config>(
Expand All @@ -558,7 +566,7 @@ pub mod pallet {
delegation: Bond<T::AccountId, BalanceOf<T>>,
) -> Result<(DelegatorAdded<Balance>, Option<Balance>), DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let mut less_total_staked = None;
let delegator_added = match self.top_capacity {
Expand Down Expand Up @@ -605,7 +613,7 @@ pub mod pallet {
delegation: Bond<T::AccountId, BalanceOf<T>>,
) -> Option<Balance>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let mut less_total_staked = None;
let mut top_delegations = <TopDelegations<T>>::get(candidate)
Expand All @@ -625,7 +633,7 @@ pub mod pallet {
// insert into top
top_delegations.insert_sorted_greatest_to_least(delegation);
// update candidate info
self.reset_top_data::<T>(&top_delegations);
self.reset_top_data::<T>(candidate.clone(), &top_delegations);
if less_total_staked.is_none() {
// only increment delegation count if we are not kicking a bottom delegation
self.delegation_count += 1u32;
Expand All @@ -643,7 +651,7 @@ pub mod pallet {
candidate: &T::AccountId,
delegation: Bond<T::AccountId, BalanceOf<T>>,
) where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let mut bottom_delegations = <BottomDelegations<T>>::get(candidate)
.expect("CandidateInfo existence => BottomDelegations existence");
Expand Down Expand Up @@ -740,7 +748,7 @@ pub mod pallet {
delegator: T::AccountId,
) -> Result<bool, DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let old_total_counted = self.total_counted;
// remove top delegation
Expand Down Expand Up @@ -777,7 +785,7 @@ pub mod pallet {
top_delegations.insert_sorted_greatest_to_least(highest_bottom_delegation);
}
// update candidate info
self.reset_top_data::<T>(&top_delegations);
self.reset_top_data::<T>(candidate.clone(), &top_delegations);
self.delegation_count -= 1u32;
<TopDelegations<T>>::insert(candidate, top_delegations);
// return whether total counted changed
Expand Down Expand Up @@ -827,7 +835,7 @@ pub mod pallet {
more: BalanceOf<T>,
) -> Result<bool, DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let lowest_top_eq_highest_bottom =
self.lowest_top_delegation_amount == self.highest_bottom_delegation_amount;
Expand Down Expand Up @@ -857,7 +865,7 @@ pub mod pallet {
more: BalanceOf<T>,
) -> Result<bool, DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let mut top_delegations = <TopDelegations<T>>::get(candidate)
.expect("CandidateInfo exists => TopDelegations exists");
Expand All @@ -880,8 +888,9 @@ pub mod pallet {
})
.collect();
ensure!(in_top, Error::<T>::DelegationDNE);
top_delegations.total = top_delegations.total.saturating_add(more);
top_delegations.sort_greatest_to_least();
self.reset_top_data::<T>(&top_delegations);
self.reset_top_data::<T>(candidate.clone(), &top_delegations);
<TopDelegations<T>>::insert(candidate, top_delegations);
Ok(true)
}
Expand All @@ -894,7 +903,7 @@ pub mod pallet {
more: BalanceOf<T>,
) -> Result<bool, DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
let mut bottom_delegations =
<BottomDelegations<T>>::get(candidate).ok_or(Error::<T>::CandidateDNE)?;
Expand Down Expand Up @@ -937,7 +946,7 @@ pub mod pallet {
}
// insert into top
top_delegations.insert_sorted_greatest_to_least(delegation);
self.reset_top_data::<T>(&top_delegations);
self.reset_top_data::<T>(candidate.clone(), &top_delegations);
<TopDelegations<T>>::insert(candidate, top_delegations);
true
} else {
Expand All @@ -960,6 +969,7 @@ pub mod pallet {
})
.collect();
ensure!(in_bottom, Error::<T>::DelegationDNE);
bottom_delegations.total = bottom_delegations.total.saturating_add(more);
bottom_delegations.sort_greatest_to_least();
false
};
Expand Down Expand Up @@ -1012,7 +1022,7 @@ pub mod pallet {
less: BalanceOf<T>,
) -> Result<bool, DispatchError>
where
BalanceOf<T>: Into<Balance>,
BalanceOf<T>: Into<Balance> + From<Balance>,
{
// The delegation after the `decrease-delegation` will be strictly less than the
// highest bottom delegation
Expand Down Expand Up @@ -1084,7 +1094,7 @@ pub mod pallet {
top_delegations.sort_greatest_to_least();
true
};
self.reset_top_data::<T>(&top_delegations);
self.reset_top_data::<T>(candidate.clone(), &top_delegations);
<TopDelegations<T>>::insert(candidate, top_delegations);
Ok(in_top_after)
}
Expand Down Expand Up @@ -1574,18 +1584,13 @@ pub mod pallet {
let mut collator = <CandidateInfo<T>>::get(&candidate_id)
.ok_or(Error::<T>::CandidateDNE)?;
T::Currency::unreserve(&delegator_id, balance_amt);
let before = collator.total_counted;
// need to go into decrease_delegation
let in_top = collator.decrease_delegation::<T>(
&candidate_id,
delegator_id.clone(),
amount_before,
balance_amt,
)?;
let after = collator.total_counted;
if collator.is_active() && (before != after) {
Pallet::<T>::update_active(candidate_id.clone(), after);
}
<CandidateInfo<T>>::insert(&candidate_id, collator);
let new_total_staked =
<Total<T>>::get().saturating_sub(balance_amt);
Expand Down Expand Up @@ -2858,11 +2863,6 @@ pub mod pallet {
)?;
T::Currency::reserve(&delegator, amount)
.expect("verified can reserve at top of this extrinsic body");
if let DelegatorAdded::AddedToTop { new_total } = delegator_position {
if state.is_active() {
Self::update_active(candidate.clone(), new_total);
}
}
// only is_some if kicked the lowest bottom as a consequence of this new delegation
let net_total_increase = if let Some(less) = less_total_staked {
amount - less
Expand Down Expand Up @@ -3047,12 +3047,8 @@ pub mod pallet {
amount: BalanceOf<T>,
) -> DispatchResult {
let mut state = <CandidateInfo<T>>::get(&candidate).ok_or(Error::<T>::CandidateDNE)?;
let total_changed =
state.rm_delegation_if_exists::<T>(&candidate, delegator.clone(), amount)?;
state.rm_delegation_if_exists::<T>(&candidate, delegator.clone(), amount)?;
T::Currency::unreserve(&delegator, amount);
if state.is_active() && total_changed {
Self::update_active(candidate.clone(), state.total_counted);
}
let new_total_locked = <Total<T>>::get().saturating_sub(amount);
<Total<T>>::put(new_total_locked);
let new_total = state.total_counted;
Expand Down
114 changes: 112 additions & 2 deletions pallets/parachain-staking/src/migrations.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,116 @@ use frame_support::{
use sp_runtime::traits::Zero;
use sp_std::{convert::TryInto, vec::Vec};

/// Migration to patch the incorrect delegations sums for all candidates
pub struct PatchIncorrectDelegationSums<T>(PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for PatchIncorrectDelegationSums<T> {
fn on_runtime_upgrade() -> Weight {
log::info!(
target: "PatchIncorrectDelegationSums",
"running migration to patch incorrect delegation sums"
);
let pallet_prefix: &[u8] = b"ParachainStaking";
let top_delegations_prefix: &[u8] = b"TopDelegations";
let bottom_delegations_prefix: &[u8] = b"BottomDelegations";
// Read all the data into memory.
// https://crates.parity.io/frame_support/storage/migration/fn.storage_key_iter.html
let stored_top_delegations: Vec<_> = storage_key_iter::<
T::AccountId,
Delegations<T::AccountId, BalanceOf<T>>,
Twox64Concat,
>(pallet_prefix, top_delegations_prefix)
.collect();
let migrated_candidates_top_count: Weight = stored_top_delegations
.len()
.try_into()
.expect("There are between 0 and 2**64 mappings stored.");
let stored_bottom_delegations: Vec<_> = storage_key_iter::<
T::AccountId,
Delegations<T::AccountId, BalanceOf<T>>,
Twox64Concat,
>(pallet_prefix, bottom_delegations_prefix)
.collect();
let migrated_candidates_bottom_count: Weight = stored_bottom_delegations
.len()
.try_into()
.expect("There are between 0 and 2**64 mappings stored.");
fn fix_delegations<T: Config>(
delegations: Delegations<T::AccountId, BalanceOf<T>>,
) -> Delegations<T::AccountId, BalanceOf<T>> {
let correct_total = delegations
.delegations
.iter()
.fold(BalanceOf::<T>::zero(), |acc, b| acc + b.amount);
log::info!(
target: "PatchIncorrectDelegationSums",
"Correcting total from {:?} to {:?}",
delegations.total, correct_total
);
Delegations {
delegations: delegations.delegations,
total: correct_total,
}
}
for (account, old_top_delegations) in stored_top_delegations {
let new_top_delegations = fix_delegations::<T>(old_top_delegations);
let mut candidate_info = <CandidateInfo<T>>::get(&account)
.expect("TopDelegations exists => CandidateInfo exists");
candidate_info.total_counted = candidate_info.bond + new_top_delegations.total;
if candidate_info.is_active() {
Pallet::<T>::update_active(account.clone(), candidate_info.total_counted);
}
<CandidateInfo<T>>::insert(&account, candidate_info);
<TopDelegations<T>>::insert(&account, new_top_delegations);
}
for (account, old_bottom_delegations) in stored_bottom_delegations {
let new_bottom_delegations = fix_delegations::<T>(old_bottom_delegations);
<BottomDelegations<T>>::insert(&account, new_bottom_delegations);
}
let weight = T::DbWeight::get();
let top = migrated_candidates_top_count.saturating_mul(3 * weight.write + 3 * weight.read);
let bottom = migrated_candidates_bottom_count.saturating_mul(weight.write + weight.read);
// 20% max block weight as margin for error
top + bottom + 100_000_000_000
}
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
// get total counted for all candidates
for (account, state) in <CandidateInfo<T>>::iter() {
Self::set_temp_storage(
state.total_counted,
&format!("Candidate{:?}TotalCounted", account)[..],
);
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
// ensure new total counted = top_delegations.sum() + collator self bond
for (account, state) in <CandidateInfo<T>>::iter() {
let old_count =
Self::get_temp_storage(&format!("Candidate{:?}TotalCounted", account)[..])
.expect("qed");
let new_count = state.total_counted;
let top_delegations_sum = <TopDelegations<T>>::get(account)
.expect("CandidateInfo exists => TopDelegations exists")
.delegations
.iter()
.fold(BalanceOf::<T>::zero(), |acc, b| acc + b.amount);
let correct_total_counted = top_delegations_sum + state.bond;
assert_eq!(new_count, correct_total_counted);
if new_count != old_count {
log::info!(
target: "PatchIncorrectDelegationSums",
"Corrected total from {:?} to {:?}",
old_count, new_count
);
}
}
Ok(())
}
}

/// Migration to split CandidateState and minimize unnecessary storage reads
/// for PoV optimization
/// This assumes Config::MaxTopDelegationsPerCandidate == OldConfig::MaxDelegatorsPerCandidate
Expand Down Expand Up @@ -185,7 +295,7 @@ impl<T: Config> OnRuntimeUpgrade for SplitCandidateStateToDecreasePoV<T> {
state.top_delegations.len() as u32 + state.bottom_delegations.len() as u32;
Self::set_temp_storage(
total_delegation_count,
&format!("Candidate{}DelegationCount", account)[..],
&format!("Candidate{:?}DelegationCount", account)[..],
);
}
Ok(())
Expand All @@ -196,7 +306,7 @@ impl<T: Config> OnRuntimeUpgrade for SplitCandidateStateToDecreasePoV<T> {
// check that top + bottom are the same as the expected (stored in temp)
for (account, state) in <CandidateInfo<T>>::iter() {
let expected_count: u32 =
Self::get_temp_storage(&format!("Candidate{}DelegationCount", account)[..])
Self::get_temp_storage(&format!("Candidate{:?}DelegationCount", account)[..])
.expect("qed");
let actual_count = state.delegation_count;
assert_eq!(expected_count, actual_count);
Expand Down

0 comments on commit 30b9553

Please sign in to comment.