Skip to content

Commit

Permalink
refactor: Replace StakesInfo per era stakes with latest staked amount (
Browse files Browse the repository at this point in the history
…#254)

* Modify StakesInfo to store last staked value

* Refactor runtime to work with last staked value

* Add migration for creator-staking pallet

* Refactor tests for creator-staking pallet

* Update spec_version to 40

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
F3Joule and github-actions[bot] authored Feb 21, 2024
1 parent c525607 commit 01fb722
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 221 deletions.
74 changes: 18 additions & 56 deletions pallets/creator-staking/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,24 +101,18 @@ impl<T: Config> Pallet<T> {
creator_stake_info: &mut CreatorStakeInfo<BalanceOf<T>>,
amount: BalanceOf<T>,
) -> Result<(), DispatchError> {
let current_era = Self::current_era();
let staked_before = backer_stakes.current_stake();
// let current_era = Self::current_era();
let staked_before = backer_stakes.staked;

// FIXME: this check is not needed if we ensure that backer_stakes is always empty
ensure!(
!staked_before.is_zero() ||
creator_stake_info.backers_count < T::MaxNumberOfBackersPerCreator::get(),
Error::<T>::MaxNumberOfBackersExceeded
);
if staked_before.is_zero() {
creator_stake_info.backers_count = creator_stake_info.backers_count.saturating_add(1);
creator_stake_info.backers_count.saturating_inc();
}

backer_stakes
.increase_stake(current_era, amount)
.map_err(|_| Error::<T>::CannotChangeStakeInPastEra)?;
// backer_stakes
// .increase_stake(current_era, amount)
// .map_err(|_| Error::<T>::CannotChangeStakeInPastEra)?;

Self::ensure_can_add_stake_item(backer_stakes)?;
backer_stakes.staked.saturating_accrue(amount);

let total_stake = Self::total_staked_amount(&backer).saturating_add(amount);
ensure!(total_stake >= T::MinimumTotalStake::get(), Error::<T>::InsufficientStakingAmount);
Expand Down Expand Up @@ -154,8 +148,8 @@ impl<T: Config> Pallet<T> {
creator_stake_info: &mut CreatorStakeInfo<BalanceOf<T>>,
desired_amount: BalanceOf<T>,
) -> Result<BalanceOf<T>, DispatchError> {
let current_era = Self::current_era();
let staked_value = backer_stakes.current_stake();
// let current_era = Self::current_era();
let staked_value = backer_stakes.staked;
ensure!(staked_value > Zero::zero(), Error::<T>::NotStakedCreator);

// If the remaining amount equals to zero, unstake the entire amount by this creator.
Expand All @@ -172,11 +166,11 @@ impl<T: Config> Pallet<T> {
// Modify data
creator_stake_info.total_staked = creator_stake_info.total_staked.saturating_sub(amount_to_decrease);

backer_stakes
.decrease_stake(current_era, amount_to_decrease)
.map_err(|_| Error::<T>::CannotChangeStakeInPastEra)?;
// backer_stakes
// .decrease_stake(current_era, amount_to_decrease)
// .map_err(|_| Error::<T>::CannotChangeStakeInPastEra)?;

Self::ensure_can_add_stake_item(backer_stakes)?;
backer_stakes.staked.saturating_reduce(amount_to_decrease);

Ok(amount_to_decrease)
}
Expand All @@ -200,7 +194,7 @@ impl<T: Config> Pallet<T> {
creator_id: CreatorId,
backer_stakes: StakesInfoOf<T>,
) {
if backer_stakes.is_empty() {
if backer_stakes.is_empty() && backer_stakes.staked.is_zero() {
BackerStakesByCreator::<T>::remove(backer, creator_id)
} else {
BackerStakesByCreator::<T>::insert(backer, creator_id, backer_stakes)
Expand Down Expand Up @@ -291,45 +285,13 @@ impl<T: Config> Pallet<T> {
RegisteredCreators::<T>::get(creator_id).ok_or(Error::<T>::CreatorNotFound.into())
}

pub(crate) fn ensure_can_add_stake_item(
backer_stakes: &StakesInfoOf<T>,
) -> DispatchResult {
ensure!(
backer_stakes.len() < T::MaxEraStakeItems::get(),
Error::<T>::TooManyEraStakeValues,
);
Ok(())
}

pub(crate) fn ensure_can_restake_reward(
pub(crate) fn can_restake_reward(
restake: bool,
creator_status: CreatorStatus,
backer_stakes: &mut StakesInfoOf<T>,
current_era: EraIndex,
backer_reward: BalanceOf<T>,
) -> Result<bool, DispatchError> {
staked: BalanceOf<T>,
) -> bool {
// Can restake only if the backer is already staking on the active creator
// and all the other conditions are met:
let can_restake = restake
&& creator_status == CreatorStatus::Active
&& backer_stakes.current_stake() > Zero::zero();

return if can_restake {
backer_stakes
.increase_stake(current_era, backer_reward)
.map_err(|_| Error::<T>::CannotChangeStakeInPastEra)?;

// Restaking will, in the worst case, remove one record and add another one,
// so it's fine if the vector is full
ensure!(
backer_stakes.len() <= T::MaxEraStakeItems::get(),
Error::<T>::TooManyEraStakeValues,
);

Ok(true)
} else {
Ok(false)
}
restake && creator_status == CreatorStatus::Active && staked > Zero::zero()
}

pub(crate) fn do_restake_reward(
Expand Down
17 changes: 8 additions & 9 deletions pallets/creator-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod functions;
pub mod inflation;
#[cfg(test)]
mod tests;
pub mod migration;

// #[cfg(feature = "runtime-benchmarks")]
// mod benchmarking;
Expand Down Expand Up @@ -126,7 +127,11 @@ pub mod pallet {
type TreasuryAccount: Get<Self::AccountId>;
}

/// The current storage version
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(_);

#[pallet::type_value]
Expand Down Expand Up @@ -258,9 +263,7 @@ pub mod pallet {
InactiveCreator,
CannotStakeZero,
CannotUnstakeZero,
MaxNumberOfBackersExceeded,
CannotChangeStakeInPastEra,
TooManyEraStakeValues,
InsufficientStakingAmount,
NotStakedCreator,
TooManyUnbondingChunks,
Expand Down Expand Up @@ -554,7 +557,7 @@ pub mod pallet {

// There should be some leftover staked amount
let mut backer_stakes = Self::backer_stakes(&backer, creator_id);
let staked_value = backer_stakes.current_stake();
let staked_value = backer_stakes.staked;
ensure!(staked_value > Zero::zero(), Error::<T>::NotStakedCreator);

// Don't allow withdrawal until all rewards have been claimed.
Expand Down Expand Up @@ -689,11 +692,6 @@ pub mod pallet {
let backer_reward =
Perbill::from_rational(backer_staked, reward_and_stake.staked) * reward_and_stake.rewards.backers;

// FIXME: we mustn't modify `backer_stakes` here!
let can_restake_reward = Self::ensure_can_restake_reward(
restake, creator_info.status, &mut backer_stakes, current_era, backer_reward
)?;

// Withdraw reward funds from the rewards holding account
let reward_imbalance = T::Currency::withdraw(
&Self::rewards_pot_account(),
Expand All @@ -704,8 +702,9 @@ pub mod pallet {

T::Currency::resolve_creating(&backer, reward_imbalance);

if can_restake_reward {
if Self::can_restake_reward(restake, creator_info.status, backer_stakes.staked) {
Self::do_restake_reward(&backer, backer_reward, creator_id, current_era);
backer_stakes.staked.saturating_accrue(backer_reward);
}

Self::update_backer_stakes(&backer, creator_id, backer_stakes);
Expand Down
146 changes: 146 additions & 0 deletions pallets/creator-staking/src/migration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// Copyright (C) DAPPFORCE PTE. LTD.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0.
//
// Full notice is available at https://github.com/dappforce/subsocial-parachain/blob/main/COPYRIGHT
// Full license is available at https://github.com/dappforce/subsocial-parachain/blob/main/LICENSE

use codec::{Decode, Encode};
use frame_support::{
ensure, log,
pallet_prelude::{Get, GetStorageVersion},
traits::OnRuntimeUpgrade,
weights::Weight,
};
use sp_runtime::traits::Zero;
use sp_std::vec::Vec;

use crate::{migration::old::OldStakesInfo, types::BalanceOf, Config, Pallet, StakesInfo};

const LOG_TARGET: &'static str = "runtime::creator-staking";

mod old {
use codec::MaxEncodedLen;
use frame_support::{
dispatch::TypeInfo, pallet_prelude::ValueQuery, storage_alias, Blake2_128Concat,
BoundedVec, RuntimeDebug,
};
use sp_arithmetic::traits::AtLeast32BitUnsigned;

use crate::{CreatorId, EraStake};

use super::*;

pub(super) type OldStakesInfoOf<T> =
OldStakesInfo<BalanceOf<T>, <T as Config>::MaxEraStakeItems>;

#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
#[scale_info(skip_type_params(MaxEraStakeItems))]
pub(super) struct OldStakesInfo<
Balance: AtLeast32BitUnsigned + Copy + MaxEncodedLen,
MaxEraStakeItems: Get<u32>,
> {
pub(crate) stakes: BoundedVec<EraStake<Balance>, MaxEraStakeItems>,
}

impl<Balance, MaxEraStakeItems> Default for OldStakesInfo<Balance, MaxEraStakeItems>
where
Balance: AtLeast32BitUnsigned + Copy + MaxEncodedLen,
MaxEraStakeItems: Get<u32>,
{
fn default() -> Self {
Self { stakes: BoundedVec::<EraStake<Balance>, MaxEraStakeItems>::default() }
}
}

#[storage_alias]
pub(super) type BackerStakesByCreator<T: Config> = StorageDoubleMap<
Pallet<T>,
Blake2_128Concat,
<T as frame_system::Config>::AccountId,
Blake2_128Concat,
CreatorId,
OldStakesInfoOf<T>,
ValueQuery,
>;
}

pub struct MigrateToV1<T>(sp_std::marker::PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
let current_version = Pallet::<T>::current_storage_version();
let onchain_version = Pallet::<T>::on_chain_storage_version();

log::info!(
target: LOG_TARGET,
"Running migration with current storage version {:?} / onchain {:?}",
current_version,
onchain_version
);

if onchain_version == 0 && current_version == 1 {
let mut translated: usize = 0;

crate::BackerStakesByCreator::<T>::translate_values(
|old: OldStakesInfo<BalanceOf<T>, <T as Config>::MaxEraStakeItems>| {
let last_staked = old.stakes.last().map_or(Zero::zero(), |s| s.staked);

let new_stakes = StakesInfo { stakes: old.stakes, staked: last_staked };
translated += 1;
Some(new_stakes)
},
);

current_version.put::<Pallet<T>>();

log::info!(
target: LOG_TARGET,
"Upgraded {} BackerStakesByCreator records, storage updated to version {:?}",
translated,
current_version
);
T::DbWeight::get().reads_writes((translated + 1) as u64, (translated + 1) as u64)
} else {
log::info!(
target: LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let current_version = Pallet::<T>::current_storage_version();
let onchain_version = Pallet::<T>::on_chain_storage_version();
ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1.");
let prev_count = old::BackerStakesByCreator::<T>::iter().count();
Ok((prev_count as u32).encode())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(prev_count: Vec<u8>) -> Result<(), &'static str> {
let prev_count: u32 = Decode::decode(&mut prev_count.as_slice())
.expect("the state parameter should be something that was generated by pre_upgrade");
let post_count = crate::BackerStakesByCreator::<T>::iter().count() as u32;
ensure!(
prev_count == post_count,
"the records count before and after the migration should be the same"
);

let conflicts_count = crate::BackerStakesByCreator::<T>::iter()
.filter_map(|(_, _, backer_stakes)| {
if backer_stakes.stakes.last().map_or(Zero::zero(), |s| s.staked) == backer_stakes.staked {
None
} else {
Some(())
}
}).count();

ensure!(conflicts_count.is_zero(), "latest staked value was migrated incorrectly");

ensure!(Pallet::<T>::on_chain_storage_version() == 1, "wrong storage version");

Ok(())
}
}
Loading

0 comments on commit 01fb722

Please sign in to comment.