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

[Staking] Patch delegations total mismatch #1291

Merged
merged 7 commits into from
Feb 17, 2022
+283 −59
Merged

[Staking] Patch delegations total mismatch #1291

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
68cda11
patch init but will need migration to correct incorrect state
4meta5 Feb 16, 2022
c2e5c0a
init migration
4meta5 Feb 16, 2022
2e6b663
unit test migration logic
4meta5 Feb 16, 2022
efad012
move updating candidate pool up one function to reset top data
4meta5 Feb 16, 2022
7e080c0
improve try runtime check and add comment to reset top data explainin…
4meta5 Feb 17, 2022
4197e04
uncomment xcm migrations
4meta5 Feb 17, 2022
dfe98aa
fix try runtime build
4meta5 Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 22 additions & 29 deletions pallets/parachain-staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,13 +527,18 @@ 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();
if old_total_counted != self.total_counted && self.is_active() {
girazoki marked this conversation as resolved.
Show resolved Hide resolved
Pallet::<T>::update_active(candidate, self.total_counted.into());
}
}
/// Reset bottom delegations metadata
pub fn reset_bottom_data<T: Config>(
Expand All @@ -558,7 +563,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 +610,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 +630,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 +648,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 +745,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 +782,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 +832,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 +862,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 +885,9 @@ pub mod pallet {
})
.collect();
ensure!(in_top, Error::<T>::DelegationDNE);
top_delegations.total = top_delegations.total.saturating_add(more);
girazoki marked this conversation as resolved.
Show resolved Hide resolved
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 +900,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 +943,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 +966,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 +1019,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 +1091,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 +1581,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;
girazoki marked this conversation as resolved.
Show resolved Hide resolved
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 +2860,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 +3044,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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here as before, what happens if its bottom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if its just bottom then total_changed == false so it would never enter this if branch

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
98 changes: 98 additions & 0 deletions pallets/parachain-staking/src/migrations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,104 @@ 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 delegation count for all candidates
for (account, state) in <CandidateInfo<T>>::iter() {
let total_delegation_count = state;
Self::set_temp_storage(
state.delegation_count,
&format!("Candidate{:?}DelegationCount", account)[..],
);
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
// ensure delegation count was unchanged by migration
for (account, state) in <CandidateInfo<T>>::iter() {
let expected_count: u32 =
Self::get_temp_storage(&format!("Candidate{:?}DelegationCount", account)[..])
girazoki marked this conversation as resolved.
Show resolved Hide resolved
.expect("qed");
let actual_count = state.delegation_count;
assert_eq!(expected_count, actual_count);
}
Ok(())
}
}

/// Migration to split CandidateState and minimize unnecessary storage reads
/// for PoV optimization
/// This assumes Config::MaxTopDelegationsPerCandidate == OldConfig::MaxDelegatorsPerCandidate
Expand Down