From 01fb7222a23f5bcebad77f539bf1b967976fb618 Mon Sep 17 00:00:00 2001 From: Vlad Proshchavaiev <32250097+F3Joule@users.noreply.github.com> Date: Wed, 21 Feb 2024 16:47:06 +0200 Subject: [PATCH] refactor: Replace StakesInfo per era stakes with latest staked amount (#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] --- pallets/creator-staking/src/functions.rs | 74 +++------ pallets/creator-staking/src/lib.rs | 17 +- pallets/creator-staking/src/migration.rs | 146 ++++++++++++++++++ .../src/tests/testing_utils.rs | 55 +++---- pallets/creator-staking/src/tests/tests.rs | 140 +++-------------- .../creator-staking/src/tests/tests_lib.rs | 2 +- pallets/creator-staking/src/types/impls.rs | 3 + pallets/creator-staking/src/types/mod.rs | 2 + runtime/src/lib.rs | 4 +- 9 files changed, 222 insertions(+), 221 deletions(-) create mode 100644 pallets/creator-staking/src/migration.rs diff --git a/pallets/creator-staking/src/functions.rs b/pallets/creator-staking/src/functions.rs index a2e2f9ee..88fdcc85 100644 --- a/pallets/creator-staking/src/functions.rs +++ b/pallets/creator-staking/src/functions.rs @@ -101,24 +101,18 @@ impl Pallet { creator_stake_info: &mut CreatorStakeInfo>, amount: BalanceOf, ) -> 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::::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::::CannotChangeStakeInPastEra)?; + // backer_stakes + // .increase_stake(current_era, amount) + // .map_err(|_| Error::::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::::InsufficientStakingAmount); @@ -154,8 +148,8 @@ impl Pallet { creator_stake_info: &mut CreatorStakeInfo>, desired_amount: BalanceOf, ) -> Result, 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::::NotStakedCreator); // If the remaining amount equals to zero, unstake the entire amount by this creator. @@ -172,11 +166,11 @@ impl Pallet { // 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::::CannotChangeStakeInPastEra)?; + // backer_stakes + // .decrease_stake(current_era, amount_to_decrease) + // .map_err(|_| Error::::CannotChangeStakeInPastEra)?; - Self::ensure_can_add_stake_item(backer_stakes)?; + backer_stakes.staked.saturating_reduce(amount_to_decrease); Ok(amount_to_decrease) } @@ -200,7 +194,7 @@ impl Pallet { creator_id: CreatorId, backer_stakes: StakesInfoOf, ) { - if backer_stakes.is_empty() { + if backer_stakes.is_empty() && backer_stakes.staked.is_zero() { BackerStakesByCreator::::remove(backer, creator_id) } else { BackerStakesByCreator::::insert(backer, creator_id, backer_stakes) @@ -291,45 +285,13 @@ impl Pallet { RegisteredCreators::::get(creator_id).ok_or(Error::::CreatorNotFound.into()) } - pub(crate) fn ensure_can_add_stake_item( - backer_stakes: &StakesInfoOf, - ) -> DispatchResult { - ensure!( - backer_stakes.len() < T::MaxEraStakeItems::get(), - Error::::TooManyEraStakeValues, - ); - Ok(()) - } - - pub(crate) fn ensure_can_restake_reward( + pub(crate) fn can_restake_reward( restake: bool, creator_status: CreatorStatus, - backer_stakes: &mut StakesInfoOf, - current_era: EraIndex, - backer_reward: BalanceOf, - ) -> Result { + staked: BalanceOf, + ) -> 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::::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::::TooManyEraStakeValues, - ); - - Ok(true) - } else { - Ok(false) - } + restake && creator_status == CreatorStatus::Active && staked > Zero::zero() } pub(crate) fn do_restake_reward( diff --git a/pallets/creator-staking/src/lib.rs b/pallets/creator-staking/src/lib.rs index d36124f2..d22f3abc 100644 --- a/pallets/creator-staking/src/lib.rs +++ b/pallets/creator-staking/src/lib.rs @@ -13,6 +13,7 @@ pub mod functions; pub mod inflation; #[cfg(test)] mod tests; +pub mod migration; // #[cfg(feature = "runtime-benchmarks")] // mod benchmarking; @@ -126,7 +127,11 @@ pub mod pallet { type TreasuryAccount: Get; } + /// The current storage version + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::type_value] @@ -258,9 +263,7 @@ pub mod pallet { InactiveCreator, CannotStakeZero, CannotUnstakeZero, - MaxNumberOfBackersExceeded, CannotChangeStakeInPastEra, - TooManyEraStakeValues, InsufficientStakingAmount, NotStakedCreator, TooManyUnbondingChunks, @@ -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::::NotStakedCreator); // Don't allow withdrawal until all rewards have been claimed. @@ -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(), @@ -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); diff --git a/pallets/creator-staking/src/migration.rs b/pallets/creator-staking/src/migration.rs new file mode 100644 index 00000000..df03be36 --- /dev/null +++ b/pallets/creator-staking/src/migration.rs @@ -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 = + OldStakesInfo, ::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, + > { + pub(crate) stakes: BoundedVec, MaxEraStakeItems>, + } + + impl Default for OldStakesInfo + where + Balance: AtLeast32BitUnsigned + Copy + MaxEncodedLen, + MaxEraStakeItems: Get, + { + fn default() -> Self { + Self { stakes: BoundedVec::, MaxEraStakeItems>::default() } + } + } + + #[storage_alias] + pub(super) type BackerStakesByCreator = StorageDoubleMap< + Pallet, + Blake2_128Concat, + ::AccountId, + Blake2_128Concat, + CreatorId, + OldStakesInfoOf, + ValueQuery, + >; +} + +pub struct MigrateToV1(sp_std::marker::PhantomData); + +impl OnRuntimeUpgrade for MigrateToV1 { + fn on_runtime_upgrade() -> Weight { + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::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::::translate_values( + |old: OldStakesInfo, ::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::>(); + + 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, &'static str> { + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::on_chain_storage_version(); + ensure!(onchain_version == 0 && current_version == 1, "migration from version 0 to 1."); + let prev_count = old::BackerStakesByCreator::::iter().count(); + Ok((prev_count as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> 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::::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::::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::::on_chain_storage_version() == 1, "wrong storage version"); + + Ok(()) + } +} diff --git a/pallets/creator-staking/src/tests/testing_utils.rs b/pallets/creator-staking/src/tests/testing_utils.rs index fb915332..55e4a83a 100644 --- a/pallets/creator-staking/src/tests/testing_utils.rs +++ b/pallets/creator-staking/src/tests/testing_utils.rs @@ -157,7 +157,7 @@ pub(super) fn assert_withdraw_from_inactive_creator( panic!("Creator should be unregistered.") }; - let staked_value = init_state.backer_stakes.current_stake(); + let staked_value = init_state.backer_stakes.staked; assert!(staked_value > 0); // Op with verification @@ -194,7 +194,7 @@ pub(super) fn assert_withdraw_from_inactive_creator( final_state.backer_locks.unbonding_info.vec(), ); - assert!(final_state.backer_stakes.current_stake().is_zero()); + assert!(final_state.backer_stakes.staked.is_zero()); assert!(!BackerStakesByCreator::::contains_key( &backer, creator_id @@ -232,7 +232,7 @@ pub(super) fn assert_stake( let final_state = MemorySnapshot::all(current_era, creator_id, backer); // In case backer hasn't been staking this creator until now - if init_state.backer_stakes.current_stake() == 0 { + if init_state.backer_stakes.staked == 0 { assert!(BackerStakesByCreator::::contains_key( &backer, creator_id @@ -257,8 +257,8 @@ pub(super) fn assert_stake( init_state.creator_stakes_info.total_staked + staking_value ); assert_eq!( - final_state.backer_stakes.current_stake(), - init_state.backer_stakes.current_stake() + staking_value + final_state.backer_stakes.staked, + init_state.backer_stakes.staked + staking_value ); assert_eq!( final_state.backer_locks.total_locked, @@ -279,14 +279,14 @@ pub(super) fn assert_unstake( // Calculate the expected resulting unbonding amount let remaining_staked = init_state .backer_stakes - .current_stake() + .staked .saturating_sub(value); let expected_unbond_amount = if remaining_staked < MINIMUM_TOTAL_STAKE { - init_state.backer_stakes.current_stake() + init_state.backer_stakes.staked } else { value }; - let remaining_staked = init_state.backer_stakes.current_stake() - expected_unbond_amount; + let remaining_staked = init_state.backer_stakes.staked.saturating_sub(expected_unbond_amount); // Ensure call executed successfully and event is emitted assert_ok!(CreatorStaking::unstake( @@ -345,8 +345,8 @@ pub(super) fn assert_unstake( final_state.creator_stakes_info.total_staked ); assert_eq!( - init_state.backer_stakes.current_stake() - expected_unbond_amount, - final_state.backer_stakes.current_stake() + init_state.backer_stakes.staked.saturating_sub(expected_unbond_amount), + final_state.backer_stakes.staked ); // Ensure that the number of backers is as expected @@ -414,7 +414,7 @@ pub(super) fn assert_claim_backer(claimer: AccountId, creator_id: SpaceId, resta System::reset_events(); let init_state_claim_era = MemorySnapshot::all(claim_era, creator_id, claimer); - let mut init_state_current_era = MemorySnapshot::all(current_era, creator_id, claimer); + let init_state_current_era = MemorySnapshot::all(current_era, creator_id, claimer); // Calculate backer's portion of the reward let (claim_era, staked) = init_state_claim_era.backer_stakes.clone().claim(); @@ -442,20 +442,17 @@ pub(super) fn assert_claim_backer(claimer: AccountId, creator_id: SpaceId, resta // assert staked and free balances depending on restake check, assert_restake_reward( restake, - current_era, &init_state_current_era, &final_state_current_era, calculated_reward, ); // check for stake event if restaking is performed - if CreatorStaking::ensure_can_restake_reward( + if CreatorStaking::can_restake_reward( restake, init_state_current_era.creator_info.status, - &mut init_state_current_era.backer_stakes, - current_era, - calculated_reward, - ).map_or(false, |should_restake| should_restake) { + init_state_current_era.backer_stakes.staked, + ) { // There should be at least 2 events, Reward and BondAndStake. // if there's less, panic is acceptable let events = creator_staking_events(); @@ -506,23 +503,19 @@ pub(super) fn assert_claim_backer(claimer: AccountId, creator_id: SpaceId, resta // returns should_restake_reward result so further checks can be made fn assert_restake_reward( restake: bool, - current_era: EraIndex, init_state_current_era: &MemorySnapshot, final_state_current_era: &MemorySnapshot, reward: Balance, ) { - let mut init_backer_stakes = init_state_current_era.backer_stakes.clone(); - if CreatorStaking::ensure_can_restake_reward( + if CreatorStaking::can_restake_reward( restake, init_state_current_era.clone().creator_info.status, - &mut init_backer_stakes, - current_era, - reward, - ).map_or(false, |should_restake| should_restake) { + init_state_current_era.backer_stakes.staked, + ) { // staked values should increase assert_eq!( - init_state_current_era.backer_stakes.current_stake() + reward, - final_state_current_era.backer_stakes.current_stake() + init_state_current_era.backer_stakes.staked + reward, + final_state_current_era.backer_stakes.staked ); assert_eq!( init_state_current_era.era_info.staked + reward, @@ -611,7 +604,7 @@ pub(crate) fn assert_move_stake( let target_creator_init_state = MemorySnapshot::all(current_era, to_creator_id, backer); // Calculate value which will actually be transferred - let source_creator_init_stake_amount = source_creator_init_state.backer_stakes.current_stake(); + let source_creator_init_stake_amount = source_creator_init_state.backer_stakes.staked; let expected_amount_to_move = if amount < source_creator_init_stake_amount { amount } else { @@ -637,12 +630,12 @@ pub(crate) fn assert_move_stake( // Ensure backer stakes has increased/decreased staked amount assert_eq!( - source_creator_final_state.backer_stakes.current_stake(), + source_creator_final_state.backer_stakes.staked, source_creator_init_stake_amount - expected_amount_to_move ); assert_eq!( - target_creator_final_state.backer_stakes.current_stake(), - target_creator_init_state.backer_stakes.current_stake() + expected_amount_to_move + target_creator_final_state.backer_stakes.staked, + target_creator_init_state.backer_stakes.staked + expected_amount_to_move ); // Ensure total value staked on creators has appropriately increased/decreased @@ -668,7 +661,7 @@ pub(crate) fn assert_move_stake( // if it is first stake by the backer let no_init_stake_on_target_creator = target_creator_init_state .backer_stakes - .current_stake() + .staked .is_zero(); if no_init_stake_on_target_creator { assert_eq!( diff --git a/pallets/creator-staking/src/tests/tests.rs b/pallets/creator-staking/src/tests/tests.rs index ca3207e3..da2007b5 100644 --- a/pallets/creator-staking/src/tests/tests.rs +++ b/pallets/creator-staking/src/tests/tests.rs @@ -8,10 +8,7 @@ use crate::{pallet::Error, pallet::Event, *}; use super::*; use frame_support::{assert_noop, assert_ok, traits::{Currency, OnInitialize, OnTimestampSet}, weights::Weight}; use mock::{Balances, *}; -use sp_runtime::{ - traits::{BadOrigin, Zero}, - Perbill, RuntimeDebug, -}; +use sp_runtime::{traits::{BadOrigin, Zero}, Perbill, RuntimeDebug, DispatchError}; use testing_utils::*; @@ -275,6 +272,8 @@ fn new_era_forcing() { }) } +#[deprecated] +#[ignore] #[test] fn general_backer_stakes_is_ok() { ExternalityBuilder::build().execute_with(|| { @@ -602,6 +601,8 @@ fn withdraw_from_inactive_creator_when_nothing_is_staked() { }) } +#[deprecated] +#[ignore] #[test] fn withdraw_from_inactive_creator_when_unclaimed_rewards_remaining() { ExternalityBuilder::build().execute_with(|| { @@ -799,33 +800,8 @@ fn stake_insufficient_value() { }) } -#[test] -fn stake_too_many_backers_per_creator() { - ExternalityBuilder::build().execute_with(|| { - initialize_first_block(); - - let stakeholder = 10; - let creator_id = 1; - // Insert a creator under registered creators. - assert_register(stakeholder, creator_id); - - // Stake with MAX_NUMBER_OF_BACKERS on the same creator. It must work. - for backer_id in 1..=MAX_NUMBER_OF_BACKERS { - assert_stake(backer_id.into(), creator_id, 100); - } - - // Now try to stake with an additional backer and expect an error. - assert_noop!( - CreatorStaking::stake( - RuntimeOrigin::signed((1 + MAX_NUMBER_OF_BACKERS).into()), - creator_id.clone(), - 100 - ), - Error::::MaxNumberOfBackersExceeded - ); - }) -} - +#[deprecated] +#[ignore] #[test] fn stake_too_many_era_stakes() { ExternalityBuilder::build().execute_with(|| { @@ -847,7 +823,7 @@ fn stake_too_many_era_stakes() { // Now try to stake with an additional backer and expect an error. assert_noop!( CreatorStaking::stake(RuntimeOrigin::signed(backer_id), creator_id, 100), - Error::::TooManyEraStakeValues + DispatchError::Other("Too many era stakes") ); }) } @@ -1029,6 +1005,8 @@ fn unstake_on_not_staked_creator_is_not_ok() { }) } +#[deprecated] +#[ignore] #[test] fn unstake_should_fail_when_too_many_era_stakes() { ExternalityBuilder::build().execute_with(|| { @@ -1049,7 +1027,7 @@ fn unstake_should_fail_when_too_many_era_stakes() { // an additional one. assert_noop!( CreatorStaking::unstake(RuntimeOrigin::signed(backer_id), creator_id, 10), - Error::::TooManyEraStakeValues + DispatchError::Other("Too many era stakes") ); }) } @@ -1523,41 +1501,6 @@ fn claim_with_zero_staked_is_ok() { }) } -// FIXME: how to be here? -#[ignore] -#[test] -fn claiming_when_stakes_full_without_compounding_is_ok() { - ExternalityBuilder::build().execute_with(|| { - initialize_first_block(); - - let stakeholder = 10; - let backer_id = 1; - let creator_id = 1; - // Insert a creator under registered creators. - assert_register(stakeholder, creator_id); - - // Stake with MAX_ERA_STAKE_VALUES - 1 on the same creator. It must work. - let start_era = CreatorStaking::current_era(); - for offset in 1..MAX_ERA_STAKE_ITEMS { - assert_stake(backer_id, creator_id, 100); - advance_to_era(start_era + offset * 5); - } - - // claim and restake once, so there's a claim record for the current era in the stakes vec - assert_claim_backer(backer_id, creator_id, true); - - // making another gap in eras and trying to claim and restake would exceed MAX_ERA_STAKE_VALUES - advance_to_era(CreatorStaking::current_era() + 1); - assert_noop!( - CreatorStaking::claim_backer_reward(RuntimeOrigin::signed(backer_id), creator_id, true), - Error::::TooManyEraStakeValues - ); - - // claiming should work again - assert_claim_backer(backer_id, creator_id, false); - }) -} - #[test] fn claim_creator_with_zero_stake_periods_is_ok() { ExternalityBuilder::build().execute_with(|| { @@ -1856,7 +1799,7 @@ fn move_stake_is_ok() { ); assert!( !BackerStakesByCreator::::get(&backer, &from_creator_id) - .current_stake() + .staked .is_zero() ); @@ -1869,7 +1812,7 @@ fn move_stake_is_ok() { ); assert!( BackerStakesByCreator::::get(&backer, &from_creator_id) - .current_stake() + .staked .is_zero() ); }) @@ -2000,6 +1943,8 @@ fn move_zero_stake_should_fail() { }) } +#[deprecated] +#[ignore] #[test] fn move_stake_with_max_era_stake_items_exceeded_should_fail() { ExternalityBuilder::build().execute_with(|| { @@ -2025,7 +1970,7 @@ fn move_stake_with_max_era_stake_items_exceeded_should_fail() { source_creator_id, 15 ), - Error::::TooManyEraStakeValues + DispatchError::Other("Too many era stakes") ); // Ensure it's not possible to transfer from source creator since it's era stake items are at max @@ -2036,7 +1981,7 @@ fn move_stake_with_max_era_stake_items_exceeded_should_fail() { target_creator_id, 15, ), - Error::::TooManyEraStakeValues + DispatchError::Other("Too many era stakes") ); // Swap source and target to verify that same is true if target creator era stake imtes is maxed out @@ -2049,56 +1994,7 @@ fn move_stake_with_max_era_stake_items_exceeded_should_fail() { target_creator_id, 15, ), - Error::::TooManyEraStakeValues - ); - }) -} - -#[test] -fn move_stake_with_max_number_of_backers_exceeded_should_fail() { - ExternalityBuilder::build().execute_with(|| { - initialize_first_block(); - - let stakeholder = 1; - // This one will only stake on source creator - let first_backer = 3; - // This one will stake on both origin and target contracts - let second_backer = 4; - let source_creator_id = 1; - let target_creator_id = 2; - - // Register creators and stake them with both backers - assert_register(stakeholder, source_creator_id); - assert_register(stakeholder, target_creator_id); - - assert_stake(first_backer, source_creator_id, 23); - assert_stake(second_backer, target_creator_id, 37); - assert_stake(second_backer, target_creator_id, 41); - - // Fill up the second creator with backers until max number of backers limit has been reached - for temp_backer in (second_backer + 1)..(MAX_NUMBER_OF_BACKERS as u64 + second_backer) { - Balances::resolve_creating(&temp_backer, Balances::issue(100)); - assert_stake(temp_backer, target_creator_id, 13); - } - // Sanity check + assurance that first_backer isn't staking on target creator - assert_noop!( - CreatorStaking::stake( - RuntimeOrigin::signed(first_backer), - target_creator_id, - 19 - ), - Error::::MaxNumberOfBackersExceeded - ); - - // Now attempt move stake and expect an error - assert_noop!( - CreatorStaking::move_stake( - RuntimeOrigin::signed(first_backer), - source_creator_id, - target_creator_id, - 19, - ), - Error::::MaxNumberOfBackersExceeded + DispatchError::Other("Too many era stakes") ); }) } diff --git a/pallets/creator-staking/src/tests/tests_lib.rs b/pallets/creator-staking/src/tests/tests_lib.rs index 8fa11c6d..0253a0bd 100644 --- a/pallets/creator-staking/src/tests/tests_lib.rs +++ b/pallets/creator-staking/src/tests/tests_lib.rs @@ -171,7 +171,7 @@ fn stakes_info_unstake_ops() { // Save this for later let stakes = backer_stakes.stakes.clone(); - let temp_backer_stakes = StakesInfo { stakes }; + let temp_backer_stakes = StakesInfo { stakes, staked: backer_stakes.staked }; // Fully unstake existing EraStake assert_ok!(backer_stakes.decrease_stake(second_unstake_era, total_staked)); diff --git a/pallets/creator-staking/src/types/impls.rs b/pallets/creator-staking/src/types/impls.rs index cf0e9149..2041810d 100644 --- a/pallets/creator-staking/src/types/impls.rs +++ b/pallets/creator-staking/src/types/impls.rs @@ -57,6 +57,7 @@ impl Default for StakesInfo Self { Self { stakes: BoundedVec::, MaxEraStakeItems>::default(), + staked: Zero::zero(), } } } @@ -72,6 +73,7 @@ impl StakesInfo } /// number of `EraStake` chunks + #[cfg(test)] pub(crate) fn len(&self) -> u32 { self.stakes.len() as u32 } @@ -80,6 +82,7 @@ impl StakesInfo pub(crate) fn clone(&self) -> Self { Self { stakes: self.stakes.clone(), + staked: self.staked, } } diff --git a/pallets/creator-staking/src/types/mod.rs b/pallets/creator-staking/src/types/mod.rs index 7b8bc5fb..43b74e67 100644 --- a/pallets/creator-staking/src/types/mod.rs +++ b/pallets/creator-staking/src/types/mod.rs @@ -130,7 +130,9 @@ pub struct StakesInfo< Balance: AtLeast32BitUnsigned + Copy + MaxEncodedLen, MaxEraStakeItems: Get, > { + #[deprecated(note = "This field is deprecated and will be removed in the future.")] pub(crate) stakes: BoundedVec, MaxEraStakeItems>, + pub(crate) staked: Balance, } /// Represents a balance amount that is currently unbonding. diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index b690b844..c8c849b8 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -121,7 +121,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, - (), + pallet_creator_staking::migration::MigrateToV1, >; /// Handles converting a weight scalar to a fee value, based on the scale and granularity of the @@ -178,7 +178,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("subsocial-parachain"), impl_name: create_runtime_str!("subsocial-parachain"), authoring_version: 1, - spec_version: 39, + spec_version: 40, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 8,