Skip to content

Commit

Permalink
refactoring and improvements (still failing)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gpestana
gpestana committed May 27, 2024
1 parent 45439b8 commit 0139b15
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 86 deletions.
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2714,6 +2714,8 @@ mod remote_tests {
.unwrap();

ext.execute_with(|| {
//let mut meter = WeightMeter::with_limit(Weight::from_parts(100_779_206_000,
// 10477090));
let mut meter = WeightMeter::new();
let mut cursor = None;
loop {
Expand Down
148 changes: 95 additions & 53 deletions substrate/frame/staking/src/migrations/v13_stake_tracker/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
//! sorted list of targets with a bags-list.

use super::PALLET_MIGRATIONS_ID;
use crate::{log, weights, Config, Nominators, Pallet, StakerStatus};
use crate::{log, weights, Config, Nominators, Pallet, StakerStatus, Validators};
use core::marker::PhantomData;
use frame_election_provider_support::SortedListProvider;
use frame_election_provider_support::{SortedListProvider, VoteWeight};
use frame_support::{
ensure,
migrations::{MigrationId, SteppedMigration, SteppedMigrationError},
Expand All @@ -34,9 +34,6 @@ use sp_std::prelude::*;
#[cfg(test)]
mod tests;

#[cfg(test)]
const TRY_STATE_INTERVAL: usize = 10_000_000;

/// V13 Multi-block migration to introduce the stake-tracker pallet.
///
/// A step of the migration consists of processing one nominator in the [`Nominators`] list. All
Expand All @@ -50,8 +47,6 @@ const TRY_STATE_INTERVAL: usize = 10_000_000;
/// - Ensure the new targets in the list are sorted per total stake (as per the underlying
/// [`SortedListProvider`]).
pub struct MigrationV13<T: Config, W: weights::WeightInfo>(PhantomData<(T, W)>);

// TODO: check bonds again.
impl<T: Config<CurrencyBalance = u128>, W: weights::WeightInfo> SteppedMigration
for MigrationV13<T, W>
{
Expand All @@ -74,11 +69,12 @@ impl<T: Config<CurrencyBalance = u128>, W: weights::WeightInfo> SteppedMigration
return Err(SteppedMigrationError::InsufficientWeight { required });
}

#[cfg(test)]
let mut counter = 0;
let mut chilled = 0;

// do as much progress as possible per step.
while meter.try_consume(required).is_ok() {
//TODO: only bench a sub-step function, e.g. Self::do_migrate_nominatior(who);

// fetch the next nominator to migrate.
let mut iter = if let Some(ref last_nom) = cursor {
Nominators::<T>::iter_from(Nominators::<T>::hashed_key_for(last_nom))
Expand All @@ -87,65 +83,54 @@ impl<T: Config<CurrencyBalance = u128>, W: weights::WeightInfo> SteppedMigration
};

if let Some((nominator, _)) = iter.next() {
// try chill nominator. If chilled, skip migration of this nominator.
// try chill nominator. If chilled, skip migration of this nominator. The nominator
// is force-chilled because it may need to re-nominate with a different set of
// nominations (see below). Thus it is better to just chill the nominator and move
// on.
if Self::try_chill_nominator(&nominator) {
log!(info, "nominator {:?} chilled, skip it.", nominator);
//log!(info, "nominator {:?} chilled, skip it.", nominator);
chilled += 1;
cursor = Some(nominator);
continue;
}
// clean the nominations before migrating. This will ensure that the voter is not
// nominating duplicate and/or dangling targets.
let nominations = Self::clean_nominations(&nominator)?;

let nominator_stake = Pallet::<T>::weight_of(&nominator);

log!(
info,
"mmb: processing nominator {:?} with {} nominations. remaining {} nominators to migrate.",
nominator,
nominations.len(),
iter.count()
);

// iter over up to `MaxNominationsOf<T>` targets of `nominator`.
// iter over up to `MaxNominationsOf<T>` targets of `nominator` and insert or
// update the target's approval's score.
for target in nominations.into_iter() {
if let Ok(current_stake) = <T as Config>::TargetList::get_score(&target) {
// target is in the target list. update with nominator's stake.
if let Some(total_stake) = current_stake.checked_add(nominator_stake.into())
{
let _ = <T as Config>::TargetList::on_update(&target, total_stake)
.defensive();
} else {
log!(error, "target stake overflow. exit.");
return Err(SteppedMigrationError::Failed)
}
if <T as Config>::TargetList::contains(&target) {
Self::update_target(&target, nominator_stake)?;
} else {
// target is not in the target list, insert new node and consider self
// stake.
let self_stake = Pallet::<T>::weight_of(&target);
if let Some(total_stake) = self_stake.checked_add(nominator_stake) {
let _ =
<T as Config>::TargetList::on_insert(target, total_stake.into())
.defensive();
} else {
log!(error, "target stake overflow. exit.");
return Err(SteppedMigrationError::Failed)
}
}
}

// enable partial checks for testing purposes only.
#[cfg(test)]
{
counter += 1;
if counter % TRY_STATE_INTERVAL == 0 {
Pallet::<T>::do_try_state_approvals(Some(nominator.clone())).unwrap();
Self::insert_target(&target, nominator_stake)?;
}
}

// progress cursor.
cursor = Some(nominator)
} else {
// done, return earlier.

// TODO: do this as the migration is performed -- not a large step at the end.

let mut a = 0;
// but before, add active validators without any nominations.
for (validator, _) in Validators::<T>::iter() {
if !<T as Config>::TargetList::contains(&validator) &&
<Pallet<T> as StakingInterface>::status(&validator) ==
Ok(StakerStatus::Validator)
{
a += 1;
let self_stake = Pallet::<T>::weight_of(&validator);
<T as Config>::TargetList::on_insert(validator, self_stake.into())
.expect("node does not exist, checked above; qed.");
}
}

log!(info, "Added validators with self stake: {:?}", a);
log!(info, "Chilled nominators: {:?}", chilled);
return Ok(None)
}
}
Expand All @@ -154,7 +139,59 @@ impl<T: Config<CurrencyBalance = u128>, W: weights::WeightInfo> SteppedMigration
}
}

impl<T: Config, W: weights::WeightInfo> MigrationV13<T, W> {
impl<T: Config<CurrencyBalance = u128>, W: weights::WeightInfo> MigrationV13<T, W> {
/// Inserts a new target in the list.
///
/// Note: the caller must ensure that the target node does not exist in the list yet.
/// Oterhwise, use [`Self::update_target`].
fn insert_target(
who: &T::AccountId,
nomination_stake: VoteWeight,
) -> Result<(), SteppedMigrationError> {
let init_stake = match <Pallet<T> as StakingInterface>::status(&who) {
Ok(StakerStatus::Validator) => {
let self_stake = Pallet::<T>::weight_of(&who);
if let Some(total_stake) = self_stake.checked_add(nomination_stake) {
total_stake
} else {
log!(error, "target stake overflow. exit.");
return Err(SteppedMigrationError::Failed)
}
},
_ => nomination_stake,
};

match <T as Config>::TargetList::on_insert(who.clone(), init_stake.into()) {
Err(e) => {
log!(error, "inserting {:?} in TL: {:?}", who, e);
Err(SteppedMigrationError::Failed)
},
Ok(_) => Ok(()),
}
}

/// Updates the target score in the target list.
///
/// Note: the caller must ensure that the target node already exists in the list. Otherwise,
/// use [`Self::insert_target`].
fn update_target(
who: &T::AccountId,
nomination_stake: VoteWeight,
) -> Result<(), SteppedMigrationError> {
let current_stake =
<T as Config>::TargetList::get_score(&who).expect("node is in the list");

if let Some(total_stake) = current_stake.checked_add(nomination_stake.into()) {
let _ = <T as Config>::TargetList::on_update(&who, total_stake)
.map_err(|e| log!(error, "updating TL score of {:?}: {:?}", who, e))
.defensive();
Ok(())
} else {
log!(error, "target stake overflow. exit.");
Err(SteppedMigrationError::Failed)
}
}

/// Chills a nominator if their active stake is below the minimum bond.
pub(crate) fn try_chill_nominator(who: &T::AccountId) -> bool {
if Pallet::<T>::active_stake(&who).unwrap_or_default() <
Expand Down Expand Up @@ -206,7 +243,12 @@ impl<T: Config, W: weights::WeightInfo> MigrationV13<T, W> {

// update `who`'s nominations in staking or chill voter, if necessary.
if nominations.len() == 0 {
let _ = <Pallet<T> as StakingInterface>::chill(&who).defensive();
let _ = <Pallet<T> as StakingInterface>::chill(&who)
.map_err(|e| {
log!(error, "ERROR when chilling {:?}", who);
e
})
.defensive();
} else if count_before > nominations.len() {
<Pallet<T> as StakingInterface>::nominate(who, nominations.clone()).map_err(|e| {
log!(error, "failed to migrate nominations {:?}.", e);
Expand Down
91 changes: 58 additions & 33 deletions substrate/frame/staking/src/pallet/impls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2110,6 +2110,7 @@ impl<T: Config> Pallet<T> {
"VoterList contains non-staker"
);

/*
Self::check_ledgers()?;
Self::check_bonded_consistency()?;
Self::check_payees()?;
Expand All @@ -2118,7 +2119,8 @@ impl<T: Config> Pallet<T> {
Self::check_paged_exposures()?;
Self::check_count()?;
Self::ensure_disabled_validators_sorted()?;
Self::do_try_state_approvals(None)?;
*/
Self::do_try_state_approvals()?;
Self::do_try_state_target_sorting()
}

Expand Down Expand Up @@ -2471,10 +2473,8 @@ impl<T: Config> Pallet<T> {
/// score.
/// * The number of target nodes in the target list matches the number of
/// (active_validators + idle_validators + dangling_targets_score_with_score).
pub fn do_try_state_approvals(
maybe_last: Option<T::AccountId>,
) -> Result<(), sp_runtime::TryRuntimeError> {
use sp_std::collections::btree_map::BTreeMap;
pub fn do_try_state_approvals() -> Result<(), sp_runtime::TryRuntimeError> {
use sp_std::collections::{btree_map::BTreeMap, btree_set::BTreeSet};
let mut approvals_map: BTreeMap<T::AccountId, T::CurrencyBalance> = BTreeMap::new();

// build map of approvals stakes from the `Nominators` storage map POV.
Expand All @@ -2484,7 +2484,6 @@ impl<T: Config> Pallet<T> {

// fail if nominator has duplicate nominations, unexpected in the context of
// keeping track of the stake approvals.
use sp_std::collections::btree_set::BTreeSet;
let count_before = nominations.len();
let dedup_noms: Vec<T::AccountId> = nominations
.clone()
Expand All @@ -2499,9 +2498,28 @@ impl<T: Config> Pallet<T> {
);

for target in nominations.into_iter() {
// skip if for some reason there is a nominator being nominated.
match Self::status(&target) {
Ok(StakerStatus::Nominator(_)) => {
log!(
warn,
"nominated stakers {:?} is a nominator, not a validator.",
target
);
continue;
},
_ => (),
}

if let Some(approvals) = approvals_map.get_mut(&target) {
*approvals += vote.into();
} else {
// TODO: simplify
let self_stake = Pallet::<T>::weight_of(&target);
let init_stake = self_stake.saturating_add(vote);
approvals_map.insert(target, init_stake.into());

/*
// new addition to the map. add self-stake if validator is active.
let _ = match Self::status(&target) {
Ok(StakerStatus::Validator) => {
Expand All @@ -2510,63 +2528,70 @@ impl<T: Config> Pallet<T> {
},
_ => approvals_map.insert(target, vote.into()),
};
*/
}
}

// stop builing the approvals map at the cursor, if it is set.
match maybe_last {
Some(last) if voter == last => break,
_ => (),
}
}

let mut a = 0;
// add active validators without any nominations.
for (validator, _) in Validators::<T>::iter() {
// do not add validator if it is not in a good state.
match Self::inspect_bond_state(&validator) {
Ok(LedgerIntegrityState::Ok) | Err(_) => (),
_ => continue,
}

if !approvals_map.contains_key(&validator) {
let self_stake = Pallet::<T>::weight_of(&validator);
approvals_map.insert(validator, self_stake.into());
// skip if for some reason there is a nominator being nominated.
match Self::status(&validator) {
Ok(StakerStatus::Nominator(_)) => {
log!(
warn,
"nominated staker {:?} is a nominator, not a validator.",
validator
);
},
_ => {
a += 1;
let self_stake = Pallet::<T>::weight_of(&validator);
approvals_map.insert(validator, self_stake.into());
},
}
}
}

let mut bad_count = 0;
let mut mismatch_approvals = 0;

// compare calculated approvals per target with target list state.
for (target, calculated_stake) in approvals_map.iter() {
let stake_in_list = if let Ok(stake) = T::TargetList::get_score(target) {
stake
} else {
continue;
};
let stake_in_list = T::TargetList::get_score(target).unwrap();

if *calculated_stake != stake_in_list {
log!(
error,
"try-runtime: score of {:?} in `TargetList` list: {:?}, calculated sum of all stake: {:?}",
target,
stake_in_list,
calculated_stake,
);
mismatch_approvals += 1;

log!(
error,
"{:?} score in the target list {:?} is different than the expected {:?}",
"try-runtime: score of {:?} in `TargetList` list: {:?}, calculated sum of all stake: {:?} -- status: {:?}",
target,
stake_in_list,
calculated_stake,
Self::status(&target),
);
bad_count += 1;
//return Err("target score in the target list is different than the
// expected".into())
}
}

log!(error, "BAD COUNT: {:?}", bad_count);

frame_support::ensure!(
approvals_map.keys().count() == T::TargetList::iter().count(),
"calculated approvals count is different from total of target list.",
);

if !mismatch_approvals.is_zero() {
log!(error, "{} targets with unexpected score in list", mismatch_approvals);

return Err("final calculated approvals != target list scores".into());
}

Ok(())
}

Expand Down

0 comments on commit 0139b15

Please sign in to comment.